linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH,TRIVIAL] AF_UNIX, accept() and addrlen
@ 2008-03-08  2:23 Samuel Thibault
  2008-03-24  4:56 ` David Miller
  0 siblings, 1 reply; 17+ messages in thread
From: Samuel Thibault @ 2008-03-08  2:23 UTC (permalink / raw)
  To: linux-kernel, akpm

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 <samuel.thibault@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;
 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH,TRIVIAL] AF_UNIX, accept() and addrlen
  2008-03-08  2:23 [PATCH,TRIVIAL] AF_UNIX, accept() and addrlen Samuel Thibault
@ 2008-03-24  4:56 ` David Miller
  2008-03-24 10:43   ` Samuel Thibault
  0 siblings, 1 reply; 17+ messages in thread
From: David Miller @ 2008-03-24  4:56 UTC (permalink / raw)
  To: samuel.thibault; +Cc: linux-kernel, akpm

From: Samuel Thibault <samuel.thibault@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.thibault@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.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH,TRIVIAL] AF_UNIX, accept() and addrlen
  2008-03-24  4:56 ` David Miller
@ 2008-03-24 10:43   ` Samuel Thibault
  2008-03-24 11:50     ` Andi Kleen
  2008-03-24 20:23     ` David Miller
  0 siblings, 2 replies; 17+ messages in thread
From: Samuel Thibault @ 2008-03-24 10:43 UTC (permalink / raw)
  To: David Miller; +Cc: linux-kernel, akpm

David Miller, le Sun 23 Mar 2008 21:56:41 -0700, a écrit :
> From: Samuel Thibault <samuel.thibault@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.thibault@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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH,TRIVIAL] AF_UNIX, accept() and addrlen
  2008-03-24 10:43   ` Samuel Thibault
@ 2008-03-24 11:50     ` Andi Kleen
  2008-03-24 12:17       ` Samuel Thibault
  2008-03-24 20:23     ` David Miller
  1 sibling, 1 reply; 17+ messages in thread
From: Andi Kleen @ 2008-03-24 11:50 UTC (permalink / raw)
  To: Samuel Thibault; +Cc: David Miller, linux-kernel, akpm

Samuel Thibault <samuel.thibault@ens-lyon.org> writes:

> David Miller, le Sun 23 Mar 2008 21:56:41 -0700, a écrit :
> > From: Samuel Thibault <samuel.thibault@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.thibault@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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH,TRIVIAL] AF_UNIX, accept() and addrlen
  2008-03-24 11:50     ` Andi Kleen
@ 2008-03-24 12:17       ` Samuel Thibault
  2008-03-24 12:27         ` Samuel Thibault
  0 siblings, 1 reply; 17+ messages in thread
From: Samuel Thibault @ 2008-03-24 12:17 UTC (permalink / raw)
  To: Andi Kleen; +Cc: David Miller, linux-kernel, akpm

Andi Kleen, le Mon 24 Mar 2008 12:50:10 +0100, a écrit :
> Samuel Thibault <samuel.thibault@ens-lyon.org> writes:
> 
> > David Miller, le Sun 23 Mar 2008 21:56:41 -0700, a écrit :
> > > From: Samuel Thibault <samuel.thibault@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.thibault@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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH,TRIVIAL] AF_UNIX, accept() and addrlen
  2008-03-24 12:17       ` Samuel Thibault
@ 2008-03-24 12:27         ` Samuel Thibault
  2008-03-31  4:00           ` Michael Kerrisk
  0 siblings, 1 reply; 17+ messages in thread
From: Samuel Thibault @ 2008-03-24 12:27 UTC (permalink / raw)
  To: Andi Kleen, David Miller, linux-kernel

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 <samuel.thibault@ens-lyon.org> writes:
> > > David Miller, le Sun 23 Mar 2008 21:56:41 -0700, a écrit :
> > > > From: Samuel Thibault <samuel.thibault@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.thibault@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.

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH,TRIVIAL] AF_UNIX, accept() and addrlen
  2008-03-24 10:43   ` Samuel Thibault
  2008-03-24 11:50     ` Andi Kleen
@ 2008-03-24 20:23     ` David Miller
  1 sibling, 0 replies; 17+ messages in thread
From: David Miller @ 2008-03-24 20:23 UTC (permalink / raw)
  To: samuel.thibault; +Cc: linux-kernel, akpm

From: Samuel Thibault <samuel.thibault@ens-lyon.org>
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.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH,TRIVIAL] AF_UNIX, accept() and addrlen
  2008-03-24 12:27         ` Samuel Thibault
@ 2008-03-31  4:00           ` Michael Kerrisk
  2008-03-31  9:44             ` Samuel Thibault
  2008-04-18 16:52             ` Michael Kerrisk
  0 siblings, 2 replies; 17+ messages in thread
From: Michael Kerrisk @ 2008-03-31  4:00 UTC (permalink / raw)
  To: Samuel Thibault, Andi Kleen, David Miller, linux-kernel, mtk.manpages

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 <samuel.thibault@ens-lyon.org> 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 <samuel.thibault@ens-lyon.org> writes:
>  > > > David Miller, le Sun 23 Mar 2008 21:56:41 -0700, a écrit :
>  > > > > From: Samuel Thibault <samuel.thibault@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.thibault@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.
>
>
> 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 majordomo@vger.kernel.org
>  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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH,TRIVIAL] AF_UNIX, accept() and addrlen
  2008-03-31  4:00           ` Michael Kerrisk
@ 2008-03-31  9:44             ` Samuel Thibault
  2008-03-31 18:51               ` Michael Kerrisk
  2008-04-18 16:52             ` Michael Kerrisk
  1 sibling, 1 reply; 17+ messages in thread
From: Samuel Thibault @ 2008-03-31  9:44 UTC (permalink / raw)
  To: Michael Kerrisk; +Cc: Andi Kleen, David Miller, linux-kernel

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH,TRIVIAL] AF_UNIX, accept() and addrlen
  2008-03-31  9:44             ` Samuel Thibault
@ 2008-03-31 18:51               ` Michael Kerrisk
  0 siblings, 0 replies; 17+ messages in thread
From: Michael Kerrisk @ 2008-03-31 18:51 UTC (permalink / raw)
  To: Samuel Thibault, Michael Kerrisk, Andi Kleen, David Miller, linux-kernel

On 3/31/08, Samuel Thibault <samuel.thibault@ens-lyon.org> 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...

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH,TRIVIAL] AF_UNIX, accept() and addrlen
  2008-03-31  4:00           ` Michael Kerrisk
  2008-03-31  9:44             ` Samuel Thibault
@ 2008-04-18 16:52             ` Michael Kerrisk
  2008-04-24  0:16               ` Samuel Thibault
  1 sibling, 1 reply; 17+ messages in thread
From: Michael Kerrisk @ 2008-04-18 16:52 UTC (permalink / raw)
  To: Samuel Thibault; +Cc: Andi Kleen, David Miller, linux-kernel, mtk.manpages

Samuel,


Michael Kerrisk wrote:
> On 3/24/08, Samuel Thibault <samuel.thibault@ens-lyon.org> 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 <samuel.thibault@ens-lyon.org> writes:
>>  > > > David Miller, le Sun 23 Mar 2008 21:56:41 -0700, a écrit :
>>  > > > > From: Samuel Thibault <samuel.thibault@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.thibault@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.
>>
>>
>> How about this?

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


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH,TRIVIAL] AF_UNIX, accept() and addrlen
  2008-04-18 16:52             ` Michael Kerrisk
@ 2008-04-24  0:16               ` Samuel Thibault
  2008-04-24  8:31                 ` Michael Kerrisk
  0 siblings, 1 reply; 17+ messages in thread
From: Samuel Thibault @ 2008-04-24  0:16 UTC (permalink / raw)
  To: Michael Kerrisk; +Cc: Andi Kleen, David Miller, linux-kernel, mtk.manpages

Michael Kerrisk, le Fri 18 Apr 2008 18:52:21 +0200, a écrit :
> >>  +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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH,TRIVIAL] AF_UNIX, accept() and addrlen
  2008-04-24  0:16               ` Samuel Thibault
@ 2008-04-24  8:31                 ` Michael Kerrisk
  2008-04-26  1:44                   ` Samuel Thibault
  0 siblings, 1 reply; 17+ messages in thread
From: Michael Kerrisk @ 2008-04-24  8:31 UTC (permalink / raw)
  To: Samuel Thibault, Michael Kerrisk, Andi Kleen, David Miller,
	linux-kernel, mtk.manpages

>  > 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?

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH,TRIVIAL] AF_UNIX, accept() and addrlen
  2008-04-24  8:31                 ` Michael Kerrisk
@ 2008-04-26  1:44                   ` Samuel Thibault
  2008-04-27  5:54                     ` David Miller
  0 siblings, 1 reply; 17+ messages in thread
From: Samuel Thibault @ 2008-04-26  1:44 UTC (permalink / raw)
  To: Michael Kerrisk; +Cc: Michael Kerrisk, Andi Kleen, David Miller, linux-kernel

Michael Kerrisk, le Thu 24 Apr 2008 10:31:15 +0200, a écrit :
> >  > 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.thibault@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;
 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH,TRIVIAL] AF_UNIX, accept() and addrlen
  2008-04-26  1:44                   ` Samuel Thibault
@ 2008-04-27  5:54                     ` David Miller
  2008-05-12 13:10                       ` Michael Kerrisk
  0 siblings, 1 reply; 17+ messages in thread
From: David Miller @ 2008-04-27  5:54 UTC (permalink / raw)
  To: samuel.thibault; +Cc: mtk.manpages, mtk.manpages, andi, linux-kernel

From: Samuel Thibault <samuel.thibault@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.thibault@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.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH,TRIVIAL] AF_UNIX, accept() and addrlen
  2008-04-27  5:54                     ` David Miller
@ 2008-05-12 13:10                       ` Michael Kerrisk
  2008-05-12 13:20                         ` Samuel Thibault
  0 siblings, 1 reply; 17+ messages in thread
From: Michael Kerrisk @ 2008-05-12 13:10 UTC (permalink / raw)
  To: samuel.thibault; +Cc: David Miller, mtk.manpages, andi, linux-kernel

Samuel, et al.

David Miller wrote:
> From: Samuel Thibault <samuel.thibault@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.thibault@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 ==


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH,TRIVIAL] AF_UNIX, accept() and addrlen
  2008-05-12 13:10                       ` Michael Kerrisk
@ 2008-05-12 13:20                         ` Samuel Thibault
  0 siblings, 0 replies; 17+ messages in thread
From: Samuel Thibault @ 2008-05-12 13:20 UTC (permalink / raw)
  To: Michael Kerrisk; +Cc: David Miller, andi, linux-kernel

Hello,

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


^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2008-05-12 14:17 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-03-08  2:23 [PATCH,TRIVIAL] AF_UNIX, accept() and addrlen Samuel Thibault
2008-03-24  4:56 ` David Miller
2008-03-24 10:43   ` Samuel Thibault
2008-03-24 11:50     ` Andi Kleen
2008-03-24 12:17       ` Samuel Thibault
2008-03-24 12:27         ` Samuel Thibault
2008-03-31  4:00           ` Michael Kerrisk
2008-03-31  9:44             ` Samuel Thibault
2008-03-31 18:51               ` Michael Kerrisk
2008-04-18 16:52             ` Michael Kerrisk
2008-04-24  0:16               ` Samuel Thibault
2008-04-24  8:31                 ` Michael Kerrisk
2008-04-26  1:44                   ` Samuel Thibault
2008-04-27  5:54                     ` David Miller
2008-05-12 13:10                       ` Michael Kerrisk
2008-05-12 13:20                         ` Samuel Thibault
2008-03-24 20:23     ` David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).