netdev - Re: [PATCH] VSOCK: fix uapi/linux/vm

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <FDD1A319-6FF0-4CB2-B362-7B033C89F238@vmware.com>
Date:   Wed, 13 Sep 2017 07:55:20 +0000
From:   "Jorgen S. Hansen" <jhansen@...are.com>
To:     Stefan Hajnoczi <stefanha@...hat.com>
CC:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "David S . Miller" <davem@...emloft.net>
Subject: Re: [PATCH] VSOCK: fix uapi/linux/vm_sockets.h incomplete types


> On Sep 12, 2017, at 6:34 PM, Stefan Hajnoczi <stefanha@...hat.com> wrote:
> 
> This patch fixes the following compiler errors when userspace
> applications use the vm_sockets.h header:
> 
>  include/uapi/linux/vm_sockets.h:148:32: error: invalid application of ‘sizeof’ to incomplete type ‘struct sockaddr’
>    unsigned char svm_zero[sizeof(struct sockaddr) -
>                                  ^~~~~~
>  include/uapi/linux/vm_sockets.h:149:18: error: ‘sa_family_t’ undeclared here (not in a function)
>             sizeof(sa_family_t) -
>                    ^~~~~~~~~~~
> 
> Two issues:
> 1. In the kernel struct sockaddr comes in via <linux/socket.h> but in
>   userspace <sys/socket.h> is required.
> 2. struct sockaddr_vm has a __kernel_sa_family_t field so let's be
>   consistent and use the same type for the sizeof(sa_family_t)
>   calculation.
> 
> Currently userspace applications work around this broken header by first
> including <sys/socket.h>.  In the kernel there is no compiler error
> because <linux/socket.h> provides everything.  It's worth fixing the
> header file though.
> 
> Cc: Jorgen Hansen <jhansen@...are.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@...hat.com>
> ---
> include/uapi/linux/vm_sockets.h | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/include/uapi/linux/vm_sockets.h b/include/uapi/linux/vm_sockets.h
> index b4ed5d895699..4ae5c625ac56 100644
> --- a/include/uapi/linux/vm_sockets.h
> +++ b/include/uapi/linux/vm_sockets.h
> @@ -18,6 +18,10 @@
> 
> #include <linux/socket.h>
> 
> +#ifndef __KERNEL__
> +#include <sys/socket.h> /* struct sockaddr */
> +#endif
> +
> /* Option name for STREAM socket buffer size.  Use as the option name in
>  * setsockopt(3) or getsockopt(3) to set or get an unsigned long long that
>  * specifies the size of the buffer underlying a vSockets STREAM socket.
> @@ -146,7 +150,7 @@ struct sockaddr_vm {
> 	unsigned int svm_port;
> 	unsigned int svm_cid;
> 	unsigned char svm_zero[sizeof(struct sockaddr) -
> -			       sizeof(sa_family_t) -
> +			       sizeof(__kernel_sa_family_t) -
> 			       sizeof(unsigned short) -
> 			       sizeof(unsigned int) - sizeof(unsigned int)];
> };
> -- 
> 2.13.5
> 

Thanks for fixing this.

Reviewed-by: Jorgen Hansen <jhansen@...are.com>