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]

Date:   Fri, 15 Sep 2017 14:14:32 -0700 (PDT)
From:   David Miller <davem@...emloft.net>
To:     stefanha@...hat.com
Cc:     netdev@...r.kernel.org, jhansen@...are.com
Subject: Re: [PATCH] VSOCK: fix uapi/linux/vm_sockets.h incomplete types

From: Stefan Hajnoczi <stefanha@...hat.com>
Date: Tue, 12 Sep 2017 17:34:35 +0100

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

There is no precedence whatsoever to include sys/socket.h in _any_ UAPI
header file provided by the kernel.

__kernel_sa_family_t is what should be used in UAPI headers, only
non-UAPI headers can use plain sa_family_t.

So that is the correct fix for this problem.

Thank you.