linux-kernel - Re: [PATCH] uapi: fix linux/if.h userspace compilation errors

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: <20170221201914.GA28360@altlinux.org>
Date:   Tue, 21 Feb 2017 23:19:14 +0300
From:   "Dmitry V. Levin" <ldv@...linux.org>
To:     David Miller <davem@...emloft.net>
Cc:     netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] uapi: fix linux/if.h userspace compilation errors

On Tue, Feb 21, 2017 at 12:10:22PM -0500, David Miller wrote:
> From: "Dmitry V. Levin" <ldv@...linux.org>
> Date: Mon, 20 Feb 2017 14:58:41 +0300
> 
> > Include <sys/socket.h> (guarded by ifndef __KERNEL__) to fix
> > the following linux/if.h userspace compilation errors:
> 
> Wouldn't it be so much better to do this in include/uapi/linux/socket.h?

Yes, it would be nicer if we could afford it.  However, changing
uapi/linux/socket.h to include <sys/socket.h> is less conservative than
changing every uapi header that fails to compile because of its use
of struct sockaddr.  It's risky because <sys/socket.h> pulls in other
types that might conflict with definitions provided by uapi headers.

I've checked all uapi headers that have no compilation errors whether
inclusion of <sys/socket.h> before them causes any issues, and found
that linux/time.h and linux/uio.h (and their users) stop compiling
because of type conflicts:

$ gcc -S -o/dev/null -xc /dev/null -include /usr/include/sys/socket.h -include /usr/include/linux/time.h
In file included from <command-line>:32:0:
/usr/include/linux/time.h:9:8: error: redefinition of 'struct timespec'
 struct timespec {
        ^~~~~~~~
In file included from /usr/include/sys/select.h:45:0,
                 from /usr/include/sys/types.h:219,
                 from /usr/include/sys/uio.h:23,
                 from /usr/include/sys/socket.h:26,
                 from <command-line>:32:
/usr/include/time.h:120:8: note: originally defined here
 struct timespec
        ^~~~~~~~
In file included from <command-line>:32:0:
/usr/include/linux/time.h:15:8: error: redefinition of 'struct timeval'
 struct timeval {
        ^~~~~~~
In file included from /usr/include/sys/select.h:47:0,
                 from /usr/include/sys/types.h:219,
                 from /usr/include/sys/uio.h:23,
                 from /usr/include/sys/socket.h:26,
                 from <command-line>:32:
/usr/include/bits/time.h:30:8: note: originally defined here
 struct timeval
        ^~~~~~~
$ gcc -S -o/dev/null -xc /dev/null -include /usr/include/sys/socket.h -include /usr/include/linux/uio.h
In file included from <command-line>:32:0:
/usr/include/linux/uio.h:16:8: error: redefinition of 'struct iovec'
 struct iovec
        ^~~~~
In file included from /usr/include/sys/uio.h:28:0,
                 from /usr/include/sys/socket.h:26,
                 from <command-line>:32:
/usr/include/bits/uio.h:43:8: note: originally defined here
 struct iovec
        ^~~~~

We can try to workaround these ancient <time.h> against <linux/time.h>
and <sys/uio.h> against <linux/uio.h> conflicts on uapi side by
introducing __UAPI_DEF_TIMESPEC, __UAPI_DEF_TIMEVAL, and __UAPI_DEF_IOVEC
guards, but these workarounds will work only one way (when libc headers
are included before uapi ones) until glibc catches up, and the latter
may take a lot of time.

> I mean, look at the comment right by the change you are making:
> 
> > @@ -24,6 +24,10 @@
> >  #include <linux/socket.h>		/* for "struct sockaddr" et al	*/
> 
> Anyone including linux/socket.h is trying to obtain that type.

I think this comment is true for #ifdef __KERNEL__ code only.

-- 
ldv

Content of type "application/pgp-signature" skipped