[PATCH,TRIVIAL] AF_UNIX, accept() and addrlen
Samuel Thibault
Accept and getpeername are supposed to return the amount of bytes
written in the returned address. However, on unnamed sockets, only
sizeof(short) is returned, while a 0 is put in the sun_path member.
This patch adds 1 for that additional byte.
Signed-off-by: Samuel Thibault <samuel....@ens-lyon.org>
--- linux/net/unix/af_unix.c.orig 2008-03-08 02:17:40.000000000 +0000
+++ linux/net/unix/af_unix.c 2008-03-08 02:17:54.000000000 +0000
@@ -1274,7 +1274,7 @@ static int unix_getname(struct socket *s
if (!u->addr) {
sunaddr->sun_family = AF_UNIX;
sunaddr->sun_path[0] = 0;
- *uaddr_len = sizeof(short);
+ *uaddr_len = sizeof(short) + 1;
} else {
struct unix_address *addr = u->addr;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
David Miller
Date: Sat, 8 Mar 2008 02:23:21 +0000
> Accept and getpeername are supposed to return the amount of bytes
> written in the returned address. However, on unnamed sockets, only
> sizeof(short) is returned, while a 0 is put in the sun_path member.
> This patch adds 1 for that additional byte.
>
> Signed-off-by: Samuel Thibault <samuel....@ens-lyon.org>
This change isn't correct. It's the fact that the
length returned is sizeof(short) that tells the caller
that the unix socket is unnamed.
We zero out the sun_path[0] member just to be polite
and tidy.
You would break applications if you changed this, so
marking this patch as "trivial" is extremely premature.
Samuel Thibault
> From: Samuel Thibault <samuel....@ens-lyon.org>
> Date: Sat, 8 Mar 2008 02:23:21 +0000
>
> > Accept and getpeername are supposed to return the amount of bytes
> > written in the returned address. However, on unnamed sockets, only
> > sizeof(short) is returned, while a 0 is put in the sun_path member.
> > This patch adds 1 for that additional byte.
> >
> > Signed-off-by: Samuel Thibault <samuel....@ens-lyon.org>
>
> This change isn't correct. It's the fact that the
> length returned is sizeof(short) that tells the caller
> that the unix socket is unnamed.
Mmm, where that is documented?
I can't find any details about that in SUS, and man 7 unix says
`If sun_path starts with a null byte ('' '), then it refers to the
abstract namespace main- tained by the Unix protocol module.'
It doesn't talk about the size being only sizeof(short) (which I guess
you meant sizeof(sa_family_t) actually).
> We zero out the sun_path[0] member just to be polite and tidy.
>
> You would break applications if you changed this, so
> marking this patch as "trivial" is extremely premature.
See documentation above. If applications don't follow documentation,
then they deserve breaking :)
Note also that on some (BSD-ish) systems, sockaddr_un contains a sun_len
field, containing the length of the data, and thus on them accept and
getpeername return more that sizeof(sa_family_t) as length (it actually
returns 16). So such applications are really broken.
Samuel
Andi Kleen
> David Miller, le Sun 23 Mar 2008 21:56:41 -0700, a écrit :
> > From: Samuel Thibault <samuel....@ens-lyon.org>
> > Date: Sat, 8 Mar 2008 02:23:21 +0000
> >
> > > Accept and getpeername are supposed to return the amount of bytes
> > > written in the returned address. However, on unnamed sockets, only
> > > sizeof(short) is returned, while a 0 is put in the sun_path member.
> > > This patch adds 1 for that additional byte.
> > >
> > > Signed-off-by: Samuel Thibault <samuel....@ens-lyon.org>
> >
> > This change isn't correct. It's the fact that the
> > length returned is sizeof(short) that tells the caller
> > that the unix socket is unnamed.
>
> Mmm, where that is documented?
>
> I can't find any details about that in SUS, and man 7 unix says
>
> `If sun_path starts with a null byte ('' '), then it refers to the
> abstract namespace main- tained by the Unix protocol module.'
[I wrote unix(7) originally]. The abstract name space is a Linux
extension and there is no written standard and whatever the kernel
implements is the de-facto standard. If unix(7) differs in anything
from what the code does please send patches to the manpages
maintainer.
-Andi
Samuel Thibault
> Samuel Thibault <samuel....@ens-lyon.org> writes:
>
> > David Miller, le Sun 23 Mar 2008 21:56:41 -0700, a écrit :
> > > From: Samuel Thibault <samuel....@ens-lyon.org>
> > > Date: Sat, 8 Mar 2008 02:23:21 +0000
> > >
> > > > Accept and getpeername are supposed to return the amount of bytes
> > > > written in the returned address. However, on unnamed sockets, only
> > > > sizeof(short) is returned, while a 0 is put in the sun_path member.
> > > > This patch adds 1 for that additional byte.
> > > >
> > > > Signed-off-by: Samuel Thibault <samuel....@ens-lyon.org>
> > >
> > > This change isn't correct. It's the fact that the
> > > length returned is sizeof(short) that tells the caller
> > > that the unix socket is unnamed.
> >
> > Mmm, where that is documented?
> >
> > I can't find any details about that in SUS, and man 7 unix says
> >
> > `If sun_path starts with a null byte ('' '), then it refers to the
> > abstract namespace main- tained by the Unix protocol module.'
>
> [I wrote unix(7) originally]. The abstract name space is a Linux
> extension and there is no written standard and whatever the kernel
> implements is the de-facto standard. If unix(7) differs in anything
> from what the code does please send patches to the manpages
> maintainer.
Oops, sorry, we are not talking about abstract namespace actually (their
sockaddr length are necessarily bigger than sizeof(sa_family_t) since
they need some data), but unamed sockets. So the Address Format
paragraph just misses description of unnamed sockets.
Samuel
Samuel Thibault
How about this?
--- unix.7.orig 2008-03-24 12:24:37.000000000 +0000
+++ unix.7 2008-03-24 12:24:56.000000000 +0000
@@ -87,6 +87,15 @@
bytes in
.IR sun_path .
Note that names in the abstract namespace are not zero-terminated.
+If the size returned by
+.BR accept
+or
+.BR getpeername
+is
+.IR sizeof(sa_family_t) ,
+then it refers to a unnamed socket and
+.I sun_path
+should not be read.
.SS Socket Options
For historical reasons these socket options are specified with a
.B SOL_SOCKET
David Miller
Date: Mon, 24 Mar 2008 10:43:30 +0000
> See documentation above. If applications don't follow documentation,
> then they deserve breaking :)
Not when we've been reporting the existing value for more
than 10 years.
Michael Kerrisk
It would really be much more useful if you CCed me, rather than hoping
that I'd find this patch by trawling LKML...
David, Andi,
My understanding about abstract namespace sockets (what Samuel calls
unnamed sockets) is that the indicator that the address is for an
unnamed socket is that the sun_path starts with a zero byte -- and the
*entire* remainder of the sun_path constitutes the name of the socket.
As such, information about the size returned by accept() etc. is
redundant. (I've happily written programs that use abstract namespace
sockets without even knowing what is returned by a succesful
accept().)
I agree with Samuel that there should be some documentation of the
return value of accept() etc, for abstract sockets but my inclination
would be to document that the indicator that this is an abstract
socket is the initial null byte in sun_path, and mention the returned
length as an after word. Does this seem reasonable?
Cheers,
Michael
--
I'll likely only see replies if they are CCed to mtk.manpages at gmail dot com
Samuel Thibault
> My understanding about abstract namespace sockets (what Samuel calls
> unnamed sockets)
No, unnamed sockets are not in the abstract namespace sockets. They
really have _no_ name.
Michael Kerrisk
> Michael Kerrisk, le Mon 31 Mar 2008 06:00:50 +0200, a écrit :
>
> > My understanding about abstract namespace sockets (what Samuel calls
> > unnamed sockets)
>
>
> No, unnamed sockets are not in the abstract namespace sockets. They
> really have _no_ name.
Ahhh -- sorry -- I didn't read the thread closely enough...
Michael Kerrisk
The idea of this patch seems okay. But one minor question below.
>> --- unix.7.orig 2008-03-24 12:24:37.000000000 +0000
>> +++ unix.7 2008-03-24 12:24:56.000000000 +0000
>> @@ -87,6 +87,15 @@
>> bytes in
>> .IR sun_path .
>> Note that names in the abstract namespace are not zero-terminated.
>> +If the size returned by
>> +.BR accept
>> +or
>> +.BR getpeername
or getsockname()
>> +is
>> +.IR sizeof(sa_family_t) ,
Why did you write sa_family_t here? Dave M already said sizeof(short), which is
the same thing, and I see that in net/unix/af_unix.c::unix_getname() there is:
u = unix_sk(sk);
unix_state_lock(sk);
if (!u->addr) {
sunaddr->sun_family = AF_UNIX;
sunaddr->sun_path[0] = 0;
*uaddr_len = sizeof(short);
} else {
>> +then it refers to a unnamed socket and
>> +.I sun_path
>> +should not be read.
>> .SS Socket Options
>> For historical reasons these socket options are specified with a
>> .B SOL_SOCKET
Cheers,
Michael
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Want to report a man-pages bug? Look here:
http://www.kernel.org/doc/man-pages/reporting_bugs.html
Samuel Thibault
> >> +is
> >> +.IR sizeof(sa_family_t) ,
>
> Why did you write sa_family_t here?
Because to me it made more sense.
> Dave M already said sizeof(short), which is the same thing,
Ok, but that's exposing implementation.
> and I see that in net/unix/af_unix.c::unix_getname() there is:
>
> *uaddr_len = sizeof(short);
I'd say that code should be fixed into using sa_family_t.
Samuel
Michael Kerrisk
> >
>
> > *uaddr_len = sizeof(short);
>
> I'd say that code should be fixed into using sa_family_t.
Is there a patch for this?
Samuel Thibault
> > > and I see that in net/unix/af_unix.c::unix_getname() there is:
> > >
> >
> > > *uaddr_len = sizeof(short);
> >
> > I'd say that code should be fixed into using sa_family_t.
>
> Is there a patch for this?
Here it is
AF_UNIX: make unix_getname use sizeof(sunaddr->sun_family) instead of
sizeof(short).
Signed-off-by: Samuel Thibault <samuel....@ens-lyon.org>
--- linux/net/unix/af_unix.c.orig 2008-04-26 02:41:45.000000000 +0100
+++ linux/net/unix/af_unix.c 2008-04-26 02:42:07.000000000 +0100
@@ -1256,7 +1256,7 @@ static int unix_getname(struct socket *s
if (!u->addr) {
sunaddr->sun_family = AF_UNIX;
sunaddr->sun_path[0] = 0;
- *uaddr_len = sizeof(short);
+ *uaddr_len = sizeof(sunaddr->sun_family);
} else {
struct unix_address *addr = u->addr;
David Miller
Date: Sat, 26 Apr 2008 02:44:45 +0100
> AF_UNIX: make unix_getname use sizeof(sunaddr->sun_family) instead of
> sizeof(short).
>
> Signed-off-by: Samuel Thibault <samuel....@ens-lyon.org>
This is just syntactic masterbation, sa_family_t is typedef'd
"unsigned short".
No system on planet earth providing the BSD sockets API uses
anything other than uint16_t or unsigned short for this.
Sorry, I'm not applying this.
Michael Kerrisk
David Miller wrote:
> From: Samuel Thibault <samuel....@ens-lyon.org>
> Date: Sat, 26 Apr 2008 02:44:45 +0100
>
>> AF_UNIX: make unix_getname use sizeof(sunaddr->sun_family) instead of
>> sizeof(short).
>>
>> Signed-off-by: Samuel Thibault <samuel....@ens-lyon.org>
>
> This is just syntactic masterbation, sa_family_t is typedef'd
> "unsigned short".
>
> No system on planet earth providing the BSD sockets API uses
> anything other than uint16_t or unsigned short for this.
>
> Sorry, I'm not applying this.
I finally got round to testing on FreeBSD. Linux and BSD are unfortunately not
compatible. For the unnamed sockets, FreeBSD says the socket address length is
16 bytes (whereas Linux says it's 2 bytes). On the other hand, there doesn't
seem to be much consistency across implementations: HP-UX's get{peer,sock}name()
says that unnamed sockets have a zero-length address.
Anyway, the situation we have is three address formats in the Unix domain on Linux:
Named sockets (socket was given an string name with bind())
length >= 4 (i.e., sizeof(unsigned short) + at least one character for a
pathname + 1 for the NUL
sun_path is a null-terminated string
Abstract sockets (socket was bound to a sun_path whose initial byte is 0)
Length == sizeof(struct sockaddr_un) (i.e., 110)
sun_path is an initial null byte, followed by 107 other bytes that make the name
unique
Unnamed sockets (created by socketpair(), or when we connect() a socket that was
not bound to a name; the current unix.7 page suggests that when we connect() a
socket that was not bound, then it gets a name in the abstract name space --
that's not true)
Length = 2
sun_path is 108 null bytes .
I have drafted a revision to section of the unix.7 page describing the address
format to try and cover all of the above. Samuel (and David?) could you review
please?
Cheers,
Michael
Address Format
A Unix domain socket address is represented in the following
structure:
#define UNIX_PATH_MAX 108
struct sockaddr_un {
sa_family_t sun_family; /* AF_UNIX */
char sun_path[UNIX_PATH_MAX]; /* pathname */
};
sun_family always contains AF_UNIX.
Three types of address are distinguished in this structure:
* pathname: a Unix domain socket can be bound to a null-ter-
minated file system pathname using bind(2). When the
address of the socket is returned by getsockname(2), get-
peername(2), and accept(2), its length is sizeof(sa_fam-
ily_t) + strlen(sun_path) + 1, and sun_path contains the
null-terminated pathname.
* anonymous: A stream socket that is connect(2)ed to another
socket without first being bound to an address is anony-
mous. Likewise, the two sockets created by socketpair(2)
are anonymous. When the address of an anonymous socket is
returned by getsockname(2), getpeername(2), and accept(2),
its length is sizeof(sa_family_t), and sun_path should not
be inspected.
* abstract: an abstract socket address is distinguished by
the fact that sun_path[0] is a null byte (`\0'). All of
the remaining bytes in sun_path define the "name" of the
socket. (Null bytes in the name have no special signifi-
cance.) The name has no connection with file system path-
names. The socket's address in this namespace is given by
the rest of the bytes in sun_path. When the address of an
abstract socket is returned by getsockname(2), getpeer-
name(2), and accept(2), its length is sizeof(struct sock-
addr_un), and sun_path contains the abstract name. The
abstract socket namespace is a non-portable Linux exten-
sion.
== END ==
Samuel Thibault
Michael Kerrisk, le Mon 12 May 2008 15:10:04 +0200, a écrit :
> I have drafted a revision to section of the unix.7 page describing the address
> format to try and cover all of the above.
Looks much more clear.
Samuel