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

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

[Dmitry V. Levin]

Include <sys/socket.h> (guarded by ifndef __KERNEL__) to fix
the following linux/if.h userspace compilation errors:

/usr/include/linux/if.h:234:19: error: field 'ifru_addr' has incomplete type
   struct sockaddr ifru_addr;
/usr/include/linux/if.h:235:19: error: field 'ifru_dstaddr' has incomplete type
   struct sockaddr ifru_dstaddr;
/usr/include/linux/if.h:236:19: error: field 'ifru_broadaddr' has incomplete type
   struct sockaddr ifru_broadaddr;
/usr/include/linux/if.h:237:19: error: field 'ifru_netmask' has incomplete type
   struct sockaddr ifru_netmask;
/usr/include/linux/if.h:238:20: error: field 'ifru_hwaddr' has incomplete type
   struct  sockaddr ifru_hwaddr;

This also fixes userspace compilation of the following uapi headers:
  linux/atmbr2684.h
  linux/gsmmux.h
  linux/if_arp.h
  linux/if_bonding.h
  linux/if_frad.h
  linux/if_pppox.h
  linux/if_tunnel.h
  linux/netdevice.h
  linux/route.h
  linux/wireless.h

As no uapi header provides a definition of struct sockaddr, inclusion
of <sys/socket.h> seems to be the most conservative and the only safe
fix available.

All current users of <linux/if.h> are very likely to be including
<sys/socket.h> already because the latter is the sole provider
of struct sockaddr definition in libc, so adding a uapi header
with a definition of struct sockaddr would create a potential
conflict with <sys/socket.h>.

Replacing struct sockaddr in the definition of struct ifreq with
a different type would create a potential incompatibility with current
users of struct ifreq who might rely on ifru_addr et al members being
of type struct sockaddr.

Signed-off-by: Dmitry V. Levin <ldv@altlinux.org>
---
 include/uapi/linux/if.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/uapi/linux/if.h b/include/uapi/linux/if.h
index 1158a04..259617a 100644
--- a/include/uapi/linux/if.h
+++ b/include/uapi/linux/if.h
@@ -24,6 +24,10 @@
 #include <linux/socket.h>		/* for "struct sockaddr" et al	*/
 #include <linux/compiler.h>		/* for "__user" et al           */
 
+#ifndef __KERNEL__
+#include <sys/socket.h>			/* for struct sockaddr.		*/
+#endif
+
 #if __UAPI_DEF_IF_IFNAMSIZ
 #define	IFNAMSIZ	16
 #endif /* __UAPI_DEF_IF_IFNAMSIZ */
-- 
ldv

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

[David Miller]

From: "Dmitry V. Levin" <ldv@altlinux.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?

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.

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

[Dmitry V. Levin]

[-- Attachment #1: Type: text/plain, Size: 3245 bytes --]

On Tue, Feb 21, 2017 at 12:10:22PM -0500, David Miller wrote:
> From: "Dmitry V. Levin" <ldv@altlinux.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

[-- Attachment #2: Type: application/pgp-signature, Size: 801 bytes --]

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

[Dmitry V. Levin]

[-- Attachment #1: Type: text/plain, Size: 1016 bytes --]

On Tue, Feb 21, 2017 at 11:19:14PM +0300, Dmitry V. Levin wrote:
> On Tue, Feb 21, 2017 at 12:10:22PM -0500, David Miller wrote:
> > From: "Dmitry V. Levin" <ldv@altlinux.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?
[...]
> 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.

OK, these conflicts need to be fixed regardless of the way how
struct sockaddr related uapi compilation errors are fixed.
I'll submit these fixes shortly.


-- 
ldv

[-- Attachment #2: Type: application/pgp-signature, Size: 801 bytes --]

[PATCH 1/2] uapi: add a compatibility layer between linux/time.h and glibc

[Dmitry V. Levin]

Do not define struct itimerspec, struct timespec, struct timeval,
and TIMER_ABSTIME in linux/time.h when <time.h> is already included
and provides these definitions.

This fixes the following compilation errors and warnings when <time.h>
is included before <linux/time.h>:

/usr/include/linux/time.h:9:8: error: redefinition of 'struct timespec'
/usr/include/linux/time.h:15:8: error: redefinition of 'struct timeval'
/usr/include/linux/time.h:34:8: error: redefinition of 'struct itimerspec'
/usr/include/linux/time.h:67:0: warning: "TIMER_ABSTIME" redefined

Do not define struct itimerval, struct timespec, struct timeval, struct
timezone, ITIMER_REAL, ITIMER_VIRTUAL, and ITIMER_PROF in linux/time.h
when <sys/time.h> is already included and provides these definitions.

This fixes the following compilation errors and warnings when <sys/time.h>
is included before <linux/time.h>:

/usr/include/linux/time.h:9:8: error: redefinition of 'struct timespec'
/usr/include/linux/time.h:15:8: error: redefinition of 'struct timeval'
/usr/include/linux/time.h:20:8: error: redefinition of 'struct timezone'
/usr/include/linux/time.h:30:0: warning: "ITIMER_REAL" redefined
/usr/include/linux/time.h:31:0: warning: "ITIMER_VIRTUAL" redefined
/usr/include/linux/time.h:32:0: warning: "ITIMER_PROF" redefined
/usr/include/linux/time.h:39:8: error: redefinition of 'struct itimerval'

Signed-off-by: Dmitry V. Levin <ldv@altlinux.org>
---
 include/uapi/linux/libc-compat.h | 56 ++++++++++++++++++++++++++++++++++++++++
 include/uapi/linux/time.h        | 31 ++++++++++++++++++++--
 2 files changed, 85 insertions(+), 2 deletions(-)

diff --git a/include/uapi/linux/libc-compat.h b/include/uapi/linux/libc-compat.h
index 44b8a6b..481e3b1 100644
--- a/include/uapi/linux/libc-compat.h
+++ b/include/uapi/linux/libc-compat.h
@@ -158,6 +158,53 @@
 
 #endif /* defined(__NETIPX_IPX_H) */
 
+/* Coordinate with glibc time.h header. */
+#if (defined(__itimerspec_defined) && __itimerspec_defined) || \
+    (defined(_TIME_H) && defined(__USE_POSIX199309))
+/* __itimerspec_defined was introduced in glibc-2.25 */
+#define __UAPI_DEF_ITIMERSPEC		0
+#else
+#define __UAPI_DEF_ITIMERSPEC		1
+#endif
+
+#if (defined(__timespec_defined) && __timespec_defined) || \
+    (defined(_STRUCT_TIMESPEC) && _STRUCT_TIMESPEC)
+/* __timespec_defined was introduced in glibc-2.3.2 */
+#define __UAPI_DEF_TIMESPEC		0
+#else
+#define __UAPI_DEF_TIMESPEC		1
+#endif
+
+#if (defined(__timeval_defined) && __timeval_defined) || \
+    (defined(_STRUCT_TIMEVAL) && _STRUCT_TIMEVAL)
+/* __timeval_defined was introduced in glibc-2.25 */
+#define __UAPI_DEF_TIMEVAL		0
+#else
+#define __UAPI_DEF_TIMEVAL		1
+#endif
+
+/* Coordinate with glibc bits/time.h header. */
+#if defined(_BITS_TIME_H) && defined(__USE_POSIX199309)
+#define __UAPI_DEF_TIMER_ABSTIME	0
+#else
+#define __UAPI_DEF_TIMER_ABSTIME	1
+#endif
+
+/* Coordinate with glibc sys/time.h header. */
+#if defined(_SYS_TIME_H)
+#define __UAPI_DEF_ITIMERVAL			0
+#define __UAPI_DEF_ITIMER_REAL_VIRTUAL_PROF	0
+#else
+#define __UAPI_DEF_ITIMERVAL			1
+#define __UAPI_DEF_ITIMER_REAL_VIRTUAL_PROF	1
+#endif
+
+#if defined(_SYS_TIME_H) && defined(__USE_MISC)
+#define __UAPI_DEF_TIMEZONE		0
+#else
+#define __UAPI_DEF_TIMEZONE		1
+#endif
+
 /* Definitions for xattr.h */
 #if defined(_SYS_XATTR_H)
 #define __UAPI_DEF_XATTR		0
@@ -205,6 +252,15 @@
 #define __UAPI_DEF_IPX_CONFIG_DATA		1
 #define __UAPI_DEF_IPX_ROUTE_DEF		1
 
+/* Definitions for time.h */
+#define __UAPI_DEF_ITIMERSPEC			1
+#define __UAPI_DEF_ITIMERVAL			1
+#define __UAPI_DEF_ITIMER_REAL_VIRTUAL_PROF	1
+#define __UAPI_DEF_TIMER_ABSTIME		1
+#define __UAPI_DEF_TIMESPEC			1
+#define __UAPI_DEF_TIMEVAL			1
+#define __UAPI_DEF_TIMEZONE			1
+
 /* Definitions for xattr.h */
 #define __UAPI_DEF_XATTR		1
 
diff --git a/include/uapi/linux/time.h b/include/uapi/linux/time.h
index e75e1b6..9d03ca4 100644
--- a/include/uapi/linux/time.h
+++ b/include/uapi/linux/time.h
@@ -1,45 +1,70 @@
 #ifndef _UAPI_LINUX_TIME_H
 #define _UAPI_LINUX_TIME_H
 
+#include <linux/libc-compat.h>
 #include <linux/types.h>
 
 
+#if __UAPI_DEF_TIMESPEC
+#ifndef __timespec_defined
+#define __timespec_defined	1
+#endif
 #ifndef _STRUCT_TIMESPEC
-#define _STRUCT_TIMESPEC
+#define _STRUCT_TIMESPEC	1
+#endif
 struct timespec {
 	__kernel_time_t	tv_sec;			/* seconds */
 	long		tv_nsec;		/* nanoseconds */
 };
-#endif
+#endif /* __UAPI_DEF_TIMESPEC */
 
+#if __UAPI_DEF_TIMEVAL
+#ifndef __timeval_defined
+#define __timeval_defined	1
+#endif
+#ifndef _STRUCT_TIMEVAL
+#define _STRUCT_TIMEVAL		1
+#endif
 struct timeval {
 	__kernel_time_t		tv_sec;		/* seconds */
 	__kernel_suseconds_t	tv_usec;	/* microseconds */
 };
+#endif /* __UAPI_DEF_TIMEVAL */
 
+#if __UAPI_DEF_TIMEZONE
 struct timezone {
 	int	tz_minuteswest;	/* minutes west of Greenwich */
 	int	tz_dsttime;	/* type of dst correction */
 };
+#endif /* __UAPI_DEF_TIMEZONE */
 
 
 /*
  * Names of the interval timers, and structure
  * defining a timer setting:
  */
+#if __UAPI_DEF_ITIMER_REAL_VIRTUAL_PROF
 #define	ITIMER_REAL		0
 #define	ITIMER_VIRTUAL		1
 #define	ITIMER_PROF		2
+#endif /* __UAPI_DEF_ITIMER_REAL_VIRTUAL_PROF */
 
+#if __UAPI_DEF_ITIMERSPEC
+#ifndef __itimerspec_defined
+#define __itimerspec_defined	1
+#endif
 struct itimerspec {
 	struct timespec it_interval;	/* timer period */
 	struct timespec it_value;	/* timer expiration */
 };
+#endif /* __UAPI_DEF_ITIMERSPEC */
 
+#if __UAPI_DEF_ITIMERVAL
 struct itimerval {
 	struct timeval it_interval;	/* timer interval */
 	struct timeval it_value;	/* current value */
 };
+#endif /* __UAPI_DEF_ITIMERVAL */
 
 /*
  * The IDs of the various system clocks (for POSIX.1b interval timers):
@@ -64,6 +89,8 @@ struct itimerval {
 /*
  * The various flags for setting POSIX.1b interval timers:
  */
+#if __UAPI_DEF_TIMER_ABSTIME
 #define TIMER_ABSTIME			0x01
+#endif
 
 #endif /* _UAPI_LINUX_TIME_H */
-- 
ldv

[PATCH 2/2] uapi: add a compatibility layer between linux/uio.h and glibc

[Dmitry V. Levin]

Do not define struct iovec in linux/uio.h when <sys/uio.h> or <fcntl.h>
is already included and provides these definitions.

This fixes the following compilation error when <sys/uio.h> or <fcntl.h>
is included before <linux/uio.h>:

/usr/include/linux/uio.h:16:8: error: redefinition of 'struct iovec'

Signed-off-by: Dmitry V. Levin <ldv@altlinux.org>
---
 include/uapi/linux/libc-compat.h | 10 ++++++++++
 include/uapi/linux/uio.h         |  3 +++
 2 files changed, 13 insertions(+)

diff --git a/include/uapi/linux/libc-compat.h b/include/uapi/linux/libc-compat.h
index 481e3b1..9b88586 100644
--- a/include/uapi/linux/libc-compat.h
+++ b/include/uapi/linux/libc-compat.h
@@ -205,6 +205,13 @@
 #define __UAPI_DEF_TIMEZONE		1
 #endif
 
+/* Coordinate with glibc bits/uio.h header. */
+#if defined(_SYS_UIO_H) || defined(_FCNTL_H)
+#define __UAPI_DEF_IOVEC		0
+#else
+#define __UAPI_DEF_IOVEC		1
+#endif
+
 /* Definitions for xattr.h */
 #if defined(_SYS_XATTR_H)
 #define __UAPI_DEF_XATTR		0
@@ -261,6 +268,9 @@
 #define __UAPI_DEF_TIMEVAL			1
 #define __UAPI_DEF_TIMEZONE			1
 
+/* Definitions for uio.h */
+#define __UAPI_DEF_IOVEC		1
+
 /* Definitions for xattr.h */
 #define __UAPI_DEF_XATTR		1
 
diff --git a/include/uapi/linux/uio.h b/include/uapi/linux/uio.h
index 2731d56..e6e12cf 100644
--- a/include/uapi/linux/uio.h
+++ b/include/uapi/linux/uio.h
@@ -9,15 +9,18 @@
 #ifndef _UAPI__LINUX_UIO_H
 #define _UAPI__LINUX_UIO_H
 
+#include <linux/libc-compat.h>
 #include <linux/compiler.h>
 #include <linux/types.h>
 
 
+#if __UAPI_DEF_IOVEC
 struct iovec
 {
 	void __user *iov_base;	/* BSD uses caddr_t (1003.1g requires void *) */
 	__kernel_size_t iov_len; /* Must be size_t (1003.1g) */
 };
+#endif /* __UAPI_DEF_IOVEC */
 
 /*
  *	UIO_MAXIOV shall be at least 16 1003.1g (5.4.1.1)
-- 
ldv

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

[David Miller]

From: "Dmitry V. Levin" <ldv@altlinux.org>
Date: Tue, 21 Feb 2017 23:19:14 +0300

> On Tue, Feb 21, 2017 at 12:10:22PM -0500, David Miller wrote:
>> From: "Dmitry V. Levin" <ldv@altlinux.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.

Ok, I'll apply this for now.

Re: [PATCH 2/2] uapi: add a compatibility layer between linux/uio.h and glibc

[Lee Duncan]

Ping? I never saw any response to this.

On Wed, 22 Feb 2017 05:29:47 +0300, Dmitry V Levin wrote:
> Do not define struct iovec in linux/uio.h when <sys/uio.h> or <fcntl.h>
> is already included and provides these definitions.
> 
> This fixes the following compilation error when <sys/uio.h> or <fcntl.h>
> is included before <linux/uio.h>:
> 
> /usr/include/linux/uio.h:16:8: error: redefinition of 'struct iovec'
> 
> Signed-off-by: Dmitry V. Levin <ldv@...linux.org>
> ---
>  include/uapi/linux/libc-compat.h | 10 ++++++++++
>  include/uapi/linux/uio.h         |  3 +++
>  2 files changed, 13 insertions(+)
> 
> diff --git a/include/uapi/linux/libc-compat.h b/include/uapi/linux/libc-compat.h
> index 481e3b1..9b88586 100644
> --- a/include/uapi/linux/libc-compat.h
> +++ b/include/uapi/linux/libc-compat.h
> @@ -205,6 +205,13 @@
>  #define __UAPI_DEF_TIMEZONE		1
>  #endif
>  
> +/* Coordinate with glibc bits/uio.h header. */
> +#if defined(_SYS_UIO_H) || defined(_FCNTL_H)
> +#define __UAPI_DEF_IOVEC		0
> +#else
> +#define __UAPI_DEF_IOVEC		1
> +#endif
> +
>  /* Definitions for xattr.h */
>  #if defined(_SYS_XATTR_H)
>  #define __UAPI_DEF_XATTR		0
> @@ -261,6 +268,9 @@
>  #define __UAPI_DEF_TIMEVAL			1
>  #define __UAPI_DEF_TIMEZONE			1
>  
> +/* Definitions for uio.h */
> +#define __UAPI_DEF_IOVEC		1
> +
>  /* Definitions for xattr.h */
>  #define __UAPI_DEF_XATTR		1
>  
> diff --git a/include/uapi/linux/uio.h b/include/uapi/linux/uio.h
> index 2731d56..e6e12cf 100644
> --- a/include/uapi/linux/uio.h
> +++ b/include/uapi/linux/uio.h
> @@ -9,15 +9,18 @@
>  #ifndef _UAPI__LINUX_UIO_H
>  #define _UAPI__LINUX_UIO_H
>  
> +#include <linux/libc-compat.h>
>  #include <linux/compiler.h>
>  #include <linux/types.h>
>  
>  
> +#if __UAPI_DEF_IOVEC
>  struct iovec
>  {
>  	void __user *iov_base;	/* BSD uses caddr_t (1003.1g requires void *) */
>  	__kernel_size_t iov_len; /* Must be size_t (1003.1g) */
>  };
> +#endif /* __UAPI_DEF_IOVEC */
>  
>  /*
>   *	UIO_MAXIOV shall be at least 16 1003.1g (5.4.1.1)
> -- 
> ldv

-- 
Lee Duncan
SUSE Labs