| Message ID | 201011232159.DFE78143.tSHMFQOLFVFJOO@I-love.SAKURA.ne.jp |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | David Miller |
| Headers | show |
Hello Tetsuao Handa [Thanks for the text program that you sent more recently] On Wed, Nov 24, 2010 at 1:59 AM, Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp> wrote: > From f388eedbdc0b099bb9f36ab007f9370432abb300 Mon Sep 17 00:00:00 2001 > From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> > Date: Tue, 23 Nov 2010 21:34:25 +0900 > Subject: [PATCH] unix.7: Fix description of "pathname" sockets > > Since unix_mkname() in net/unix/af_unix.c does > > ((char *)sunaddr)[len] = 0; > > rather than > > ((char *)sunaddr)[len - 1] = 0; > > , sunaddr->sun_path may not be terminated with a null byte if > len == sizeof(*sunaddr). > > Therefore, the caller of getsockname(), getpeername(), accept() must not assume > that sunaddr->sun_path contains a null-terminated pathname even if the returned > addrlen is greater than sizeof(sa_family_t) and sun_path[0] != '\0'. Thanks. I see what you mean. However, I'm wondering, is the kernel behavior simply a bug that should be fixed, so that a null terminator is always placed in sun_path? I realize that's an ABI change, but: a) I suspect most sane applications would never create a sun_path that didn't contain a null terminator within sizeof(sun_path) bytes. b) Considering these two sets: 1. [applications that would break if the assumption that there is no null terminator inside sizeof(sun_path) bytes doesn't hold true] 2. [applications that would break if the kernel behavior changed] I suspect that set 1 is much larger than set 2. Your thoughts? Thanks, Michael > Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> > --- > man7/unix.7 | 19 ++++++++++++++++--- > 1 files changed, 16 insertions(+), 3 deletions(-) > > diff --git a/man7/unix.7 b/man7/unix.7 > index b53328b..7b0b47c 100644 > --- a/man7/unix.7 > +++ b/man7/unix.7 > @@ -80,10 +80,23 @@ When the address of the socket is returned by > and > .BR accept (2), > its length is > -.IR "offsetof(struct sockaddr_un, sun_path) + strlen(sun_path) + 1" , > +.IR "offsetof(struct sockaddr_un, sun_path) + strlen(sun_path) + 1". > +Note that this length can be one byte larger than > +.IR "sizeof(struct sockaddr_un)" > +because > +.BR bind (2) > +accepts > +.IR sun_path > +which is not terminated with a null byte ('\\0'). > +Therefore, you must not use string manipulation functions (e.g. strlen(), > +printf("%s")) against > +.IR sun_path > +because > +.BR getsockname (2), > +.BR getpeername (2), > and > -.I sun_path > -contains the null-terminated pathname. > +.BR accept (2) > +may not have stored a null-terminated string. > .IP * > .IR unnamed : > A stream socket that has not been bound to a pathname using > -- > 1.6.1
On Mon, Apr 16, 2012 at 9:42 AM, Michael Kerrisk (man-pages) <mtk.manpages@gmail.com> wrote: > Hello Tetsuao Handa > > [Thanks for the text program that you sent more recently] > > On Wed, Nov 24, 2010 at 1:59 AM, Tetsuo Handa > <penguin-kernel@i-love.sakura.ne.jp> wrote: >> From f388eedbdc0b099bb9f36ab007f9370432abb300 Mon Sep 17 00:00:00 2001 >> From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> >> Date: Tue, 23 Nov 2010 21:34:25 +0900 >> Subject: [PATCH] unix.7: Fix description of "pathname" sockets >> >> Since unix_mkname() in net/unix/af_unix.c does >> >> ((char *)sunaddr)[len] = 0; >> >> rather than >> >> ((char *)sunaddr)[len - 1] = 0; >> >> , sunaddr->sun_path may not be terminated with a null byte if >> len == sizeof(*sunaddr). >> >> Therefore, the caller of getsockname(), getpeername(), accept() must not assume >> that sunaddr->sun_path contains a null-terminated pathname even if the returned >> addrlen is greater than sizeof(sa_family_t) and sun_path[0] != '\0'. > > Thanks. I see what you mean. However, I'm wondering, is the kernel > behavior simply a bug that should be fixed, so that a null terminator > is always placed in sun_path? > > I realize that's an ABI change, but: > a) I suspect most sane applications would never create a sun_path that > didn't contain a null terminator within sizeof(sun_path) bytes. > b) Considering these two sets: > 1. [applications that would break if the assumption that there > is no null terminator inside sizeof(sun_path) bytes doesn't Sorry! Small thinko in previous line: "is no null terminator" ==> "is a null terminator" Thanks, Michael > hold true] > 2. [applications that would break if the kernel behavior changed] > I suspect that set 1 is much larger than set 2. > > Your thoughts? > > Thanks, > > Michael > >> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> >> --- >> man7/unix.7 | 19 ++++++++++++++++--- >> 1 files changed, 16 insertions(+), 3 deletions(-) >> >> diff --git a/man7/unix.7 b/man7/unix.7 >> index b53328b..7b0b47c 100644 >> --- a/man7/unix.7 >> +++ b/man7/unix.7 >> @@ -80,10 +80,23 @@ When the address of the socket is returned by >> and >> .BR accept (2), >> its length is >> -.IR "offsetof(struct sockaddr_un, sun_path) + strlen(sun_path) + 1" , >> +.IR "offsetof(struct sockaddr_un, sun_path) + strlen(sun_path) + 1". >> +Note that this length can be one byte larger than >> +.IR "sizeof(struct sockaddr_un)" >> +because >> +.BR bind (2) >> +accepts >> +.IR sun_path >> +which is not terminated with a null byte ('\\0'). >> +Therefore, you must not use string manipulation functions (e.g. strlen(), >> +printf("%s")) against >> +.IR sun_path >> +because >> +.BR getsockname (2), >> +.BR getpeername (2), >> and >> -.I sun_path >> -contains the null-terminated pathname. >> +.BR accept (2) >> +may not have stored a null-terminated string. >> .IP * >> .IR unnamed : >> A stream socket that has not been bound to a pathname using >> -- >> 1.6.1 > > > > -- > Michael Kerrisk > Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/ > Author of "The Linux Programming Interface"; http://man7.org/tlpi/
Michael Kerrisk (man-pages) wrote: > >> Therefore, the caller of getsockname(), getpeername(), accept() must not assume > >> that sunaddr->sun_path contains a null-terminated pathname even if the returned > >> addrlen is greater than sizeof(sa_family_t) and sun_path[0] != '\0'. > > > > Thanks. I see what you mean. However, I'm wondering, is the kernel > > behavior simply a bug that should be fixed, so that a null terminator > > is always placed in sun_path? I thought it is a bug, but according to thread starting from http://lkml.indiana.edu/hypermail/linux/kernel/0503.3/0954.html , sun_path seems to be designed to accept pathname without null terminator. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/man7/unix.7 b/man7/unix.7 index b53328b..7b0b47c 100644 --- a/man7/unix.7 +++ b/man7/unix.7 @@ -80,10 +80,23 @@ When the address of the socket is returned by and .BR accept (2), its length is -.IR "offsetof(struct sockaddr_un, sun_path) + strlen(sun_path) + 1" , +.IR "offsetof(struct sockaddr_un, sun_path) + strlen(sun_path) + 1". +Note that this length can be one byte larger than +.IR "sizeof(struct sockaddr_un)" +because +.BR bind (2) +accepts +.IR sun_path +which is not terminated with a null byte ('\\0'). +Therefore, you must not use string manipulation functions (e.g. strlen(), +printf("%s")) against +.IR sun_path +because +.BR getsockname (2), +.BR getpeername (2), and -.I sun_path -contains the null-terminated pathname. +.BR accept (2) +may not have stored a null-terminated string. .IP * .IR unnamed : A stream socket that has not been bound to a pathname using