Hello,
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 <[email protected]>
--- 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;
From: Samuel Thibault <[email protected]>
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 <[email protected]>
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.
David Miller, le Sun 23 Mar 2008 21:56:41 -0700, a ?crit :
> From: Samuel Thibault <[email protected]>
> 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 <[email protected]>
>
> 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
Samuel Thibault <[email protected]> writes:
> David Miller, le Sun 23 Mar 2008 21:56:41 -0700, a écrit :
> > From: Samuel Thibault <[email protected]>
> > 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 <[email protected]>
> >
> > 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
Andi Kleen, le Mon 24 Mar 2008 12:50:10 +0100, a ?crit :
> Samuel Thibault <[email protected]> writes:
>
> > David Miller, le Sun 23 Mar 2008 21:56:41 -0700, a ?crit :
> > > From: Samuel Thibault <[email protected]>
> > > 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 <[email protected]>
> > >
> > > 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, le Mon 24 Mar 2008 12:17:19 +0000, a ?crit :
> Andi Kleen, le Mon 24 Mar 2008 12:50:10 +0100, a ?crit :
> > Samuel Thibault <[email protected]> writes:
> > > David Miller, le Sun 23 Mar 2008 21:56:41 -0700, a ?crit :
> > > > From: Samuel Thibault <[email protected]>
> > > > 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 <[email protected]>
> > > >
> > > > 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.
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
Samuel
From: Samuel Thibault <[email protected]>
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.
Samuel,
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
On 3/24/08, Samuel Thibault <[email protected]> wrote:
> Samuel Thibault, le Mon 24 Mar 2008 12:17:19 +0000, a ?crit :
>
> > Andi Kleen, le Mon 24 Mar 2008 12:50:10 +0100, a ?crit :
> > > Samuel Thibault <[email protected]> writes:
> > > > David Miller, le Sun 23 Mar 2008 21:56:41 -0700, a ?crit :
> > > > > From: Samuel Thibault <[email protected]>
> > > > > 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 <[email protected]>
> > > > >
> > > > > 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.
>
>
> 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
>
>
> Samuel
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
--
I'll likely only see replies if they are CCed to mtk.manpages at gmail dot com
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.
Samuel
On 3/31/08, Samuel Thibault <[email protected]> wrote:
> 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...