>> -static int xs_bind4(struct sock_xprt *transport, struct socket *sock)
>> +static int xs_bind(struct sock_xprt *transport, struct socket *sock)
>> {
>> - struct sockaddr_in myaddr = {
>> - .sin_family = AF_INET,
>> - };
>> - struct sockaddr_in *sa;
>> + struct sockaddr myaddr;
>
> You want "struct sockaddr_storage" here. A "struct sockaddr" is required by
> standard (iirc) to be only as large as a "struct sockaddr_in". So this can't
> possibly hold an IPv6 socket address without overflowing.
OK
>> int err, nloop = 0;
>> unsigned short port = xs_get_srcport(transport);
>> unsigned short last;
>>
>> - sa = (struct sockaddr_in *)&transport->srcaddr;
>> - myaddr.sin_addr = sa->sin_addr;
>> + memcpy(&myaddr, &transport->srcaddr, transport->xprt.addrlen);
>> do {
>> - myaddr.sin_port = htons(port);
>> - err = kernel_bind(sock, (struct sockaddr *) &myaddr,
>> - sizeof(myaddr));
>> + BUILD_BUG_ON(offsetof(struct sockaddr_in, sin_port) !=
>> + offsetof(struct sockaddr_in6, sin6_port));
>> + ((struct sockaddr_in *)&myaddr)->sin_port = htons(port);
>
> This series of patches looks mostly OK at first blush, but this little
> change is a bit offensive. :-)
>
> There should be some static inline helper functions that can get and set the
> port number in a socket address without a lot of magic, and I would prefer you
> use those instead. (I would point you directly to them, but I can't easily
> access kernel source at the moment -- hotel internet sucks).
>
> There should be one or two code examples in the RPC module or in headers that
> set and get ports in an automatic struct sockaddr_storage variable to give you
> an idea of what this needs to look like to avoid stack overruns and bad
> pointer aliasing.
I believe you mean the rpc_set_port() one. Will do!
Thanks,
Pavel
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
