Discussion:
Use of sun_len in AF_UNIX socket addresses
(too old to reply)
Martin Husemann
2006-10-08 14:33:39 UTC
Permalink
Looking at PR lib/34744, I was not sure if we expect userland to set
sun_len correctly. A quick grep through our tree shows about 10 out of about
20 source files doing it, the others seem to leave sun_len as zero.

The kernel cares about sun_len at very few places, and I fail to see the
big picture. Since apparently the userland parts that do not set sun_len
work just fine, I guess we could replace the few kernel usages of
sun_len with SUN_LEN()?

Maybe we could even get rid of the sun_len field completely? (But then it
might not be worth the compmatibility issues - and of course I might just
be missing something.)

Martin

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
der Mouse
2006-10-08 16:05:05 UTC
Permalink
Post by Martin Husemann
Looking at PR lib/34744, I was not sure if we expect userland to set
sun_len correctly. A quick grep through our tree shows about 10 out
of about 20 source files doing it, the others seem to leave sun_len
as zero.
The kernel cares about sun_len at very few places, and I fail to see
the big picture. Since apparently the userland parts that do not set
sun_len work just fine, I guess we could replace the few kernel
usages of sun_len with SUN_LEN()?
Since sun_len is the sockaddr_un pun of sockaddr.sa_len, I would very
much prefer to make sure everything sets sun_len correctly, and make
the kernel use it properly.

In particular, that's the only way I can see to make AF_LOCAL sockets
with path names longer than 103 octets work properly. Most of my code
that uses AF_LOCAL with non-null names malloc()s based on an expression
rather like SUN_LEN, rather than allocating a struct sockaddr_un and
trusting the path name to fit. It is also careful to set sun_len
correctly.

Really, the AF_LOCAL sockaddr interface needs to be better-specified.
Its interface contract is currently left unspecified in many important
respects, such as these....

/~\ The ASCII der Mouse
\ / Ribbon Campaign
X Against HTML ***@rodents.montreal.qc.ca
/ \ Email! 7D C8 61 52 5D E7 2D 39 4E F1 31 3E E8 B3 27 4B

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Christian Biere
2006-10-11 18:01:37 UTC
Permalink
Since sun_len is the sockaddr_un pun of sockaddr.sa_len, I would very much
prefer to make sure everything sets sun_len correctly, and make the kernel use
it properly.
With respect to userland I for one would prefer if sun_len disappeared. Many
other systems e.g., IRIX, Solaris don't have it and a lot software (maybe most)
ignores it. As NetBSD seems to ignore it and doesn't even document it, getting
rid of it seems an obvious choice to me. I don't mind what the kernel does. I
don't have a strong opinion on that though, if sun_len is there but ignored,
that's fine by me too.
In particular, that's the only way I can see to make AF_LOCAL sockets with path
names longer than 103 octets work properly. Most of my code that uses AF_LOCAL
with non-null names malloc()s based on an expression rather like SUN_LEN,
rather than allocating a struct sockaddr_un and trusting the path name to fit.
Few code does this and it's technically not portable i.e., it works only if
sun_path is last member and if the OS actually uses the trailing data. The
former can be checked at compile or runtime but it might just as well treat
sun_path as a char array that requires no NUL termination.

Actually it looks as if NetBSD doesn't require a terminating NUL if you use a
path longer than sun_path with a dynamically sized socket address as long as
you pass the correct size to bind() and connect(). Note, however, that you
mustn't use SUN_LEN() unless you allocate one additional byte and nullify it as
SUN_LEN() uses strlen().

This allows a maximum socket path of 253 characters which is still below the
minimum maximum pathname length (255) and filename length (1023) handled by
open() et al. That's because sun_len is uint8_t (u_char) like sun_family. So
you have a limit of 255 there and the two bytes are reserved for sun_len and
sun_family. Without sun_len (in userland) you could possibly easily support
longer paths.

I don't think not terminating the path is a good choice for portable code,
though as you obviously can't check for this requirement properly.

For what it's worth, I've checked Linux and apparently it's not possible to use
a path longer than sizeof(un->sun_path). bind() returns EINVAL for paths longer
than that. FreeBSD behaves the same as NetBSD. IRIX seems to have a rather
odd maximum of 221 characters.
It is also careful to set sun_len correctly.
Maybe but it's not documented how to set sun_len correctly. No, I don't accept
a pointer to Stevens as documentation. Also, I tried various flavours of
SUN_LEN() on IRIX and most of them were wrong causing truncation of the path.
sizeof() apparently works everywhere. It's used in NetBSD as well as
third-party code. So, I think this macro just causes unnecessary confusion or
bugs even. Every code I've seen so far uses

#ifndef SUN_LEN
#define SUN_LEN(x) ...
#endif

but as said, these are often wrong which is just never noticed because it works
on the most popular systems. As long as this is only used to set sun_len which
is seemingly ignored anyway, it's no problem.

Attached is test program that can be used to check the limit for the socket
path on a given system. Note, that this works without SUN_LEN() and sun_len. So
they are completely redundant (on NetBSD) in userland.
--
Christian
Antti Kantee
2006-10-11 19:41:56 UTC
Permalink
Post by Christian Biere
Since sun_len is the sockaddr_un pun of sockaddr.sa_len, I would very much
prefer to make sure everything sets sun_len correctly, and make the kernel use
it properly.
With respect to userland I for one would prefer if sun_len disappeared. Many
other systems e.g., IRIX, Solaris don't have it and a lot software (maybe most)
ignores it. As NetBSD seems to ignore it and doesn't even document it, getting
rid of it seems an obvious choice to me. I don't mind what the kernel does. I
don't have a strong opinion on that though, if sun_len is there but ignored,
that's fine by me too.
You can't get rid of it since the kernel uses the same memory layout for
the structure. Of course you could play games, but that's not really
a thing I want to see just to hide the field. You can't set it wrong,
so does it really matter?
--
Antti Kantee <***@iki.fi> Of course he runs NetBSD
http://www.iki.fi/pooka/ http://www.NetBSD.org/
"la qualité la plus indispensable du cuisinier est l'exactitude"

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Antti Kantee
2006-10-11 20:35:39 UTC
Permalink
Post by Antti Kantee
You can't get rid of it since the kernel uses the same memory layout for
the structure. Of course you could play games, but that's not really
a thing I want to see just to hide the field. You can't set it wrong,
so does it really matter?
A simple way to get rid of it would be renaming "sun_len".
Yes, see "games". Would we then obfuscate the kernel for the new name
or obfuscate the headers to have two definitions?
--
Antti Kantee <***@iki.fi> Of course he runs NetBSD
http://www.iki.fi/pooka/ http://www.NetBSD.org/
"la qualité la plus indispensable du cuisinier est l'exactitude"

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Christian Biere
2006-10-11 20:45:39 UTC
Permalink
Post by Antti Kantee
Post by Antti Kantee
You can't get rid of it since the kernel uses the same memory layout for
the structure. Of course you could play games, but that's not really
a thing I want to see just to hide the field. You can't set it wrong,
so does it really matter?
A simple way to get rid of it would be renaming "sun_len".
Yes, see "games". Would we then obfuscate the kernel for the new name
or obfuscate the headers to have two definitions?
I wouldn't call that a "game" nor obfuscated. There are already many
"#ifdef _KERNEL" in the header and I don't see anything wrong with that
except maybe that it would be cleaner to move all such stuff into
/usr/include/kernel/ whenever possible.
--
Christian

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Christian Biere
2006-10-11 20:13:47 UTC
Permalink
Post by Antti Kantee
Post by Christian Biere
With respect to userland I for one would prefer if sun_len disappeared. Many
other systems e.g., IRIX, Solaris don't have it and a lot software (maybe most)
ignores it.
You can't get rid of it since the kernel uses the same memory layout for
the structure. Of course you could play games, but that's not really
a thing I want to see just to hide the field. You can't set it wrong,
so does it really matter?
A simple way to get rid of it would be renaming "sun_len".

The primary issue is the missing documentation. Something like "The structure member
'sun_len' is historic and should be ignored. Neither 'sun_len' nor SUN_LEN() are
part of POSIX." would be useful. Likewise, the malloc() "trick" to extend the path
and the pathname length limit should be mentioned.

Off-topic: It would be great to have SO_REUSEADDR for AF_UNIX/AF_LOCAL, so that dead
sockets don't have be unlink()ed which is always subject to a race-condition. Even
if that were a NetBSD extension for now, I'm sure other's would pick it up fairly
soon.
--
Christian

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Antti Kantee
2006-10-11 21:05:48 UTC
Permalink
Post by Christian Biere
Post by Antti Kantee
Yes, see "games". Would we then obfuscate the kernel for the new name
or obfuscate the headers to have two definitions?
I wouldn't call that a "game" nor obfuscated. There are already many
"#ifdef _KERNEL" in the header and I don't see anything wrong with that
except maybe that it would be cleaner to move all such stuff into
/usr/include/kernel/ whenever possible.
Yes, but they aren't *inside* a structure, but rather choose not to
pollute the userspace with nameload for stuff that it can't use anyway.

I could find only one _KERNEL in a struct with a quick glance (sys/buf.h),
and amusing as it is, looks like a very nice place for an unexpected
bug some day.
--
Antti Kantee <***@iki.fi> Of course he runs NetBSD
http://www.iki.fi/pooka/ http://www.NetBSD.org/
"la qualité la plus indispensable du cuisinier est l'exactitude"

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Christian Biere
2006-10-11 21:24:12 UTC
Permalink
Post by Antti Kantee
Yes, but they aren't *inside* a structure, but rather choose not to
pollute the userspace with nameload for stuff that it can't use anyway.
I could find only one _KERNEL in a struct with a quick glance (sys/buf.h),
and amusing as it is, looks like a very nice place for an unexpected
bug some day.
Well, you don't need _KERNEL. Just renaming it e.g. __sun_len and fixing
the very few references in the kernel and you're done. I've really seen
uglier stuff than that on other systems but on NetBSD as well. For example
there are many macros s_len (on Solaris IIRC), sa_len on IRIX, or s6_addr
on NetBSD that pollute the namespace and can cause really arcane bugs. That's
far from what I proposed.

But again, the main issue was the documentation. If those who care at all
about this stuff prefer to keep sun_len and SUN_LEN(), so be it.
--
Christian

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
SODA Noriyuki
2006-10-17 23:12:09 UTC
Permalink
Post by Christian Biere
On Wed, 11 Oct 2006 23:24:12 +0200,
But again, the main issue was the documentation. If those who care at all
about this stuff prefer to keep sun_len and SUN_LEN(), so be it.
I believe we should keep both sun_len and SUN_LEN().

Is the latest version of the unix(4) man page OK for you?

Or, perhaps you want to add something like the following code fragment
in some section like PORTABILITY ISSUE?
On systems which don't have the SUN_LEN() macro,
the following definition is recommended,
since using strlen((su)->sun_path) like our native SUN_LEN()
implementation truncates the path on IRIX.

#ifndef SUN_LEN
#define SUN_LEN(su) sizeof(struct(sockaddr_un))
#endif
But this sort of description may be too much for a man page....

P.S.
Maybe we should change the strlcpy() line as you said in lib/34744,
though. e.g.
if (strlcpy(sunx.sun_path, THE_PATH, sizeof(sunx.sun_path)) >=
sizeof(sunx.sun_path))
errx(EXIT_FAILURE, "%s: too long socket name", THE_PATH);
--
soda

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Christian Biere
2006-10-18 16:47:12 UTC
Permalink
Post by SODA Noriyuki
On Wed, 11 Oct 2006 23:24:12 +0200,
Is the latest version of the unix(4) man page OK for you?
Yes, it's fine by me. It seems you want to ignore the case of
overlong paths but I don't mind since it's non-portable.
Post by SODA Noriyuki
Or, perhaps you want to add something like the following code fragment
in some section like PORTABILITY ISSUE?
On systems which don't have the SUN_LEN() macro,
the following definition is recommended,
since using strlen((su)->sun_path) like our native SUN_LEN()
implementation truncates the path on IRIX.
#ifndef SUN_LEN
#define SUN_LEN(su) sizeof(struct(sockaddr_un))
#endif
But this sort of description may be too much for a man page....
I'd appreciate it. I don't think there can be too much documentation
unless it starts repeating itself. I also appreciate the "EXAMPLE"
section of other manpages like strlcpy(3), strncpy(3) etc. because
these are truly essential.
Post by SODA Noriyuki
Maybe we should change the strlcpy() line as you said in lib/34744,
though. e.g.
if (strlcpy(sunx.sun_path, THE_PATH, sizeof(sunx.sun_path)) >=
sizeof(sunx.sun_path))
errx(EXIT_FAILURE, "%s: too long socket name", THE_PATH);
I'd prefer this as well. Albeit as said I'd use strlen() and strncpy()
because for some people (not me), copying or implementing strlcpy()
seems to be unthinkable.
--
Christian

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
SODA Noriyuki
2006-10-11 01:01:12 UTC
Permalink
Post by der Mouse
Post by Martin Husemann
On Sun, 8 Oct 2006 12:05:05 -0400 (EDT),
Looking at PR lib/34744, I was not sure if we expect userland to set
sun_len correctly. A quick grep through our tree shows about 10 out
of about 20 source files doing it, the others seem to leave sun_len
as zero.
The kernel cares about sun_len at very few places, and I fail to see
the big picture. Since apparently the userland parts that do not set
sun_len work just fine, I guess we could replace the few kernel
usages of sun_len with SUN_LEN()?
Since sun_len is the sockaddr_un pun of sockaddr.sa_len, I would very
much prefer to make sure everything sets sun_len correctly, and make
the kernel use it properly.
That's practically impossible, at least for third party packages,
because only 4.4BSD derived OSes have the sun_len member, and Solaris,
HP-UX and Linux don't have the sun_len member.
(Even 4.3BSD-tahoe didn't have it, either.)
--
soda

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Martin Husemann
2006-10-11 07:35:23 UTC
Permalink
Post by SODA Noriyuki
That's practically impossible, at least for third party packages,
because only 4.4BSD derived OSes have the sun_len member, and Solaris,
HP-UX and Linux don't have the sun_len member.
(Even 4.3BSD-tahoe didn't have it, either.)
Should we consider removing it then?

Otherwise, could we rely on it being either 0 or correctly set? We could
change the kernel then to consistently use a new version of SUN_LEN() that
checks for this and does the right thing.

Martin

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
SODA Noriyuki
2006-10-11 07:55:38 UTC
Permalink
Post by Martin Husemann
Post by SODA Noriyuki
On Wed, 11 Oct 2006 09:35:23 +0200,
That's practically impossible, at least for third party packages,
because only 4.4BSD derived OSes have the sun_len member, and Solaris,
HP-UX and Linux don't have the sun_len member.
(Even 4.3BSD-tahoe didn't have it, either.)
Should we consider removing it then?
IMHO, we should leave the sun_len member as is (from userland point of
view, I mean), to not create yet another variant of 4.4BSD,
unless most of other BSDs agree with removing the member.
Post by Martin Husemann
We could change the kernel then to consistently use a new version of
SUN_LEN() that checks for this and does the right thing.
We cannot change the definition of SUN_LEN(), because the macro is
already used by many applications to set the sun_len member. ;-)
(Try http://www.google.com/codesearch?q=SUN_LEN&btnG=Search+Code,
e.g. http://cvsweb.netbsd.org/bsdweb.cgi/src/dist/bind/lib/irs/Attic/irp.c?rev=1.1.8.2&content-type=text/x-cvsweb-markup
)

If you want such macro for the kernel, new macro name is necessary.
--
soda

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Martin Husemann
2006-10-11 08:01:54 UTC
Permalink
Post by SODA Noriyuki
If you want such macro for the kernel, new macro name is necessary.
Yes, that was my intention - the question is: can we rely on sun_len be
either zero intialized or correctly set to the path len?

Martin

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
SODA Noriyuki
2006-10-11 08:06:13 UTC
Permalink
Post by Martin Husemann
Post by SODA Noriyuki
On Wed, 11 Oct 2006 10:01:54 +0200,
If you want such macro for the kernel, new macro name is necessary.
Yes, that was my intention - the question is: can we rely on sun_len be
either zero intialized or correctly set to the path len?
Do you mean userland point of view?
If so, the answer is no.

Currently we are not using the value of sa_len provided by the
userland, but using the 3rd argument (namelen) of bind(2) and
connect(2), aren't we (as you see in "sa->sa_len = buflen" in
sockargs(9)))?
We should keep that code, shouldn't we?
--
soda

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Martin Husemann
2006-10-11 09:38:11 UTC
Permalink
Post by SODA Noriyuki
Currently we are not using the value of sa_len provided by the
userland,
Ah, this is the point I originally missed.

This should go into unix(4) then, in addition to the example recently
added (and maybe an explicit hint how to use it with longer path names).
Post by SODA Noriyuki
but using the 3rd argument (namelen) of bind(2) and
connect(2), aren't we (as you see in "sa->sa_len = buflen" in
sockargs(9)))?
That is great! It means we should remove all uses of SUN_LEN() from
the kernel, doesn't it?

Martin

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
SODA Noriyuki
2006-10-11 09:56:12 UTC
Permalink
Post by Martin Husemann
Post by SODA Noriyuki
On Wed, 11 Oct 2006 11:38:11 +0200,
Currently we are not using the value of sa_len provided by the
userland,
This should go into unix(4) then, in addition to the example recently
added (and maybe an explicit hint how to use it with longer path names).
Perhaps the author of the manual thought using memset(addr, 0, sizeof(addr)))
was a good idea for portability, so he made the sun_len initialization
ambiguous. Although I'm not really sure.
At least the Stevens' UNIX Network Programming is using such style.
Although RFC2553 (IPv6 API) doesn't use such style.
Post by Martin Husemann
Post by SODA Noriyuki
but using the 3rd argument (namelen) of bind(2) and
connect(2), aren't we (as you see in "sa->sa_len = buflen" in
sockargs(9)))?
That is great! It means we should remove all uses of SUN_LEN() from
the kernel, doesn't it?
Well, I cannot find any use of SUN_LEN() in the kernel... :)
--
soda

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Martin Husemann
2006-10-11 10:29:17 UTC
Permalink
Post by SODA Noriyuki
Well, I cannot find any use of SUN_LEN() in the kernel... :)
I must have fat fingered my grep. I just checked and my results agree
with you.

Martin

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Loading...