Robert Millan wrote:
> 2011/10/16 Jonathan Nieder <jrnie...@gmail.com>:
>> unsigned char sun_len;
>> unsigned char sun_family;
>> char sun_path[108]; /* Path name. */
>
> Is this 108 the actual length? ISTR it was just a placeholder.
It's the actual size of the sun_path field of struct sockaddr_un,
if that's what you mean.
> <sys/un.h> has a macro to determine actual length:
>
> /* Evaluate to actual length of the `sockaddr_un' structure. */
> # define SUN_LEN(ptr) ((size_t) (((struct sockaddr_un *) 0)->sun_path)
> \
> + strlen ((ptr)->sun_path))
Ah, thanks for this clarification. glibc's reference manual says:
You should compute the "length" parameter for a socket address in
the local namespace as the sum of the size of the "sun_family"
component and the string length (/not/ the allocation size!) of
the file name string. This can be done using the macro "SUN_LEN":
In other words, you can call bind() with SUN_LEN(ptr) as the addrlen
argument instead of sizeof(struct sockaddr_un) to tell the kernel not to
copy so much, and to avoid depending on the magic number chosen in the
definition of sockaddr_un.
However, the POSIX documentation for bind() gives an example of an AF_UNIX
socket with
if (bind(sfd, (struct sockaddr *) &my_addr,
sizeof(struct sockaddr_un)) == -1)
Similarly, the example in the bind(2) manpage from the man-pages project
uses sizeof(struct sockaddr_un) as the addrlen argument.
Regarding the magic number, POSIX provides some more insight:
The size of "sun_path" has intentionally been left undefined.
This is because different implementations use different sizes.
For example, 4.3 BSD uses a size of 108, and 4.4 BSD uses a
size of 104. Since most implementations originate from BSD
versions, the size is typically in the range 92 to 108.
[...]
>> I wonder whether there would be any downside to changing that 104 in
>> the kernel to 108.
>
> Breaking kernel ABI for everyone not using a patched kernel. Not a good
> thing.
If it's desirable, the change could be made upstream.
>> Separately from that, it would be helpful to know where the buffer
>> overflowed in #645377 is, since maybe it could be made bigger without
>> changing the layout of struct sockaddr_un.
>
> I don't think this matters, it's an instance of sockaddr_un.
If sockaddr_un is part of the ABI as an argument to some function,
it would matter.
> If you
> make sun_path[] bigger in the kernel, then instead of 160 you can
> overflow it with a larger value
[...]
> The kernel-side of things is now doing the right thing AFAICS.
It's breaking userspace apps that followed documentation to the letter
and worked before.
How about this (untested)?
diff --git i/sys/kern/uipc_syscalls.c w/sys/kern/uipc_syscalls.c
index 3b83e1c..7b4a11e 100644
--- i/sys/kern/uipc_syscalls.c
+++ w/sys/kern/uipc_syscalls.c
@@ -1703,11 +1703,18 @@ getsockaddr(namp, uaddr, len)
if (error) {
free(sa, M_SONAME);
} else {
+ const char *p;
+ size_t datalen;
+
#if defined(COMPAT_OLDSOCK) && BYTE_ORDER != BIG_ENDIAN
if (sa->sa_family == 0 && sa->sa_len < AF_MAX)
sa->sa_family = sa->sa_len;
#endif
sa->sa_len = len;
+ datalen = len - offsetof(struct sockaddr, sa_data[0]);
+ p = memchr(sa->sa_data, '\0', datalen);
+ if (p)
+ sa_len = p - (const char *)sa;
*namp = sa;
}
return (error);
--
To UNSUBSCRIBE, email to debian-bsd-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Archive: http://lists.debian.org/20111016113158.gb22...@elie.hsd1.il.comcast.net
