Fix bits/socket.h IOC* namespace issues (bug 21267)

Fix bits/socket.h IOC* namespace issues (bug 21267)

[Joseph Myers]

sysdeps/unix/sysv/linux/bits/socket.h includes asm/socket.h.  That
includes asm/sockios.h, which on MIPS includes asm/ioctl.h, resulting
in namespace violations from IOC* macros.

bits/socket.h already has code to handle asm/socket.h unconditionally
defining macros that are only wanted for __USE_MISC.  This patch
extends it to handle the IOC* macros as well (always undefining them
if not defined when bits/socket.h was included, as I don't think they
are part of the intended API even for __USE_MISC).

It's possible there should also be a kernel fix - it's not clear to me
that IOC* belong in the uapi headers, and even if they do they might
best be split out into another header to avoid getting defined by this
particular path.  But since glibc needs to deal with existing kernel
headers, it also seems appropriate to extend the existing workaround
to these macros.

Tested (compilation only) with build-many-glibcs.py.

2017-03-18  Joseph Myers  <joseph@codesourcery.com>

	[BZ #21267]
	* sysdeps/unix/sysv/linux/bits/socket.h (IOCSIZE_MASK): Undefine
	if defined by <asm/socket.h> and not previously defined.
	(IOCSIZE_SHIFT): Likewise.
	(IOC_IN): Likewise.
	(IOC_INOUT): Likewise.
	(IOC_OUT): Likewise.

diff --git a/sysdeps/unix/sysv/linux/bits/socket.h b/sysdeps/unix/sysv/linux/bits/socket.h
index 0f1b786..6d6d56e 100644
--- a/sysdeps/unix/sysv/linux/bits/socket.h
+++ b/sysdeps/unix/sysv/linux/bits/socket.h
@@ -365,6 +365,21 @@ struct ucred
 #  define __SYS_SOCKET_H_undef_SIOCSPGRP
 # endif
 #endif
+#ifndef IOCSIZE_MASK
+# define __SYS_SOCKET_H_undef_IOCSIZE_MASK
+#endif
+#ifndef IOCSIZE_SHIFT
+# define __SYS_SOCKET_H_undef_IOCSIZE_SHIFT
+#endif
+#ifndef IOC_IN
+# define __SYS_SOCKET_H_undef_IOC_IN
+#endif
+#ifndef IOC_INOUT
+# define __SYS_SOCKET_H_undef_IOC_INOUT
+#endif
+#ifndef IOC_OUT
+# define __SYS_SOCKET_H_undef_IOC_OUT
+#endif
 
 /* Get socket manipulation related informations from kernel headers.  */
 #include <asm/socket.h>
@@ -399,6 +414,26 @@ struct ucred
 #  undef SIOCSPGRP
 # endif
 #endif
+#ifdef __SYS_SOCKET_H_undef_IOCSIZE_MASK
+# undef __SYS_SOCKET_H_undef_IOCSIZE_MASK
+# undef IOCSIZE_MASK
+#endif
+#ifdef __SYS_SOCKET_H_undef_IOCSIZE_SHIFT
+# undef __SYS_SOCKET_H_undef_IOCSIZE_SHIFT
+# undef IOCSIZE_SHIFT
+#endif
+#ifdef __SYS_SOCKET_H_undef_IOC_IN
+# undef __SYS_SOCKET_H_undef_IOC_IN
+# undef IOC_IN
+#endif
+#ifdef __SYS_SOCKET_H_undef_IOC_INOUT
+# undef __SYS_SOCKET_H_undef_IOC_INOUT
+# undef IOC_INOUT
+#endif
+#ifdef __SYS_SOCKET_H_undef_IOC_OUT
+# undef __SYS_SOCKET_H_undef_IOC_OUT
+# undef IOC_OUT
+#endif
 
 /* Structure used to manipulate the SO_LINGER option.  */
 struct linger

-- 
Joseph S. Myers
joseph@codesourcery.com

Ping Re: Fix bits/socket.h IOC* namespace issues (bug 21267)

[Joseph Myers]

Ping.  This patch 
<https://sourceware.org/ml/libc-alpha/2017-03/msg00400.html> is pending 
review.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: Ping Re: Fix bits/socket.h IOC* namespace issues (bug 21267)

[Arnd Bergmann]

On Thu, Mar 23, 2017 at 6:59 PM, Joseph Myers <joseph@codesourcery.com> wrote:
> Ping.  This patch
> <https://sourceware.org/ml/libc-alpha/2017-03/msg00400.html> is pending
> review.

I have no idea about the glibc side, but regarding your comment for the kernel
header:

> It's possible there should also be a kernel fix - it's not clear to me
> that IOC* belong in the uapi headers, and even if they do they might
> best be split out into another header to avoid getting defined by this
> particular path.  But since glibc needs to deal with existing kernel
> headers, it also seems appropriate to extend the existing workaround
> to these macros.

I wonder what the best fix is. Removing the five macros from
linux/ioctl.h risks regressions since they have been there since
ancient history (Linux-0.98.5), and there are at least two users,
one in kernel sources and one outside (we could still work
around that in the kernel):

/usr/include/linux/soundcard.h:#define SIOC_IN IOC_IN
/usr/include/linux/soundcard.h:#define SIOC_INOUT IOC_INOUT
/usr/include/linux/soundcard.h:#define SIOC_OUT IOC_OUT
/usr/include/xf86drm.h:#define DRM_IOC_READ            IOC_OUT
/usr/include/xf86drm.h:#define DRM_IOC_READWRITE       IOC_INOUT
/usr/include/xf86drm.h:#define DRM_IOC_WRITE           IOC_IN

There are many exported or user-space provided headers that include
linux/ioctl.h or asm/ioctl.h in order to get the _IOC/IOW/IOR/IOWR
macros:

linux/android/binder.h:#include <linux/ioctl.h>
linux/apm_bios.h:#include <linux/ioctl.h>
linux/aspeed-lpc-ctrl.h:#include <linux/ioctl.h>
linux/atmioc.h:#include <asm/ioctl.h>
linux/blkpg.h:#include <linux/ioctl.h>
linux/blkzoned.h:#include <linux/ioctl.h>
linux/bt-bmc.h:#include <linux/ioctl.h>
linux/btrfs.h:#include <linux/ioctl.h>
linux/capi.h:#include <linux/ioctl.h>
linux/cciss_ioctl.h:#include <linux/ioctl.h>
linux/cm4000_cs.h:#include <linux/ioctl.h>
linux/dn.h:#include <linux/ioctl.h>
linux/fd.h:#include <linux/ioctl.h>
linux/firewire-cdev.h:#include <linux/ioctl.h>
linux/fs.h:#include <linux/ioctl.h>
linux/genwqe/genwqe_card.h:#include <linux/ioctl.h>
linux/gigaset_dev.h:#include <linux/ioctl.h>
linux/gpio.h:#include <linux/ioctl.h>
linux/gsmmux.h:#include <linux/ioctl.h>
linux/hsi/cs-protocol.h:#include <linux/ioctl.h>
linux/i2o-dev.h:#include <linux/ioctl.h>
linux/iio/events.h:#include <linux/ioctl.h>
linux/ioctl.h:#include <asm/ioctl.h>
linux/isdn.h:#include <linux/ioctl.h>
linux/kfd_ioctl.h:#include <linux/ioctl.h>
linux/kvm.h:#include <linux/ioctl.h>
linux/lightnvm.h:#include <linux/ioctl.h>
linux/lirc.h:#include <linux/ioctl.h>
linux/matroxfb.h:#include <asm/ioctl.h>
linux/media.h:#include <linux/ioctl.h>
linux/mtio.h:#include <linux/ioctl.h>
linux/nilfs2_api.h:#include <linux/ioctl.h>
linux/nsfs.h:#include <linux/ioctl.h>
linux/nvram.h:#include <linux/ioctl.h>
linux/omapfb.h:#include <linux/ioctl.h>
linux/perf_event.h:#include <linux/ioctl.h>
linux/pmu.h:#include <linux/ioctl.h>
linux/pps.h:#include <linux/ioctl.h>
linux/ptp_clock.h:#include <linux/ioctl.h>
linux/radeonfb.h:#include <asm/ioctl.h>
linux/random.h:#include <linux/ioctl.h>
linux/rio_mport_cdev.h:#include <linux/ioctl.h>
linux/rpmsg.h:#include <linux/ioctl.h>
linux/serio.h:#include <linux/ioctl.h>
linux/soundcard.h:#include <linux/ioctl.h>
linux/sync_file.h:#include <linux/ioctl.h>
linux/timerfd.h:#include <linux/ioctl.h>
linux/usb/functionfs.h:#include <linux/ioctl.h>
linux/usb/gadgetfs.h:#include <linux/ioctl.h>
linux/uvcvideo.h:#include <linux/ioctl.h>
linux/v4l2-subdev.h:#include <linux/ioctl.h>
linux/vfio.h:#include <linux/ioctl.h>
linux/vhost.h:#include <linux/ioctl.h>
linux/videodev2.h:#include <linux/ioctl.h>
linux/videodev2.h.orig:#include <linux/ioctl.h>
linux/vtpm_proxy.h:#include <linux/ioctl.h>
linux/watchdog.h:#include <linux/ioctl.h>

*/asm/ioctls.h:#include <linux/ioctl.h>
cris/asm/sync_serial.h:#include <linux/ioctl.h>
mips/asm/sockios.h:#include <asm/ioctl.h>
misc/cxl.h:#include <linux/ioctl.h>
powerpc/asm/ps3fb.h:#include <linux/ioctl.h>
s390/asm/chsc.h:#include <linux/ioctl.h>
s390/asm/clp.h:#include <linux/ioctl.h>
s390/asm/dasd.h:#include <linux/ioctl.h>
s390/asm/pkey.h:#include <linux/ioctl.h>
s390/asm/qeth.h:#include <linux/ioctl.h>
s390/asm/zcrypt.h:#include <linux/ioctl.h>
sparc/asm/apc.h:#include <linux/ioctl.h>
sparc/asm/envctrl.h:#include <linux/ioctl.h>
sparc/asm/openpromio.h:#include <linux/ioctl.h>
tile/asm/hardwall.h:#include <linux/ioctl.h>
x86/asm/kvm.h:#include <linux/ioctl.h>
x86/asm/mce.h:#include <linux/ioctl.h>
x86/asm/msr.h:#include <linux/ioctl.h>
x86/asm/mtrr.h:#include <linux/ioctl.h>
xtensa/asm/sockios.h:#include <asm/ioctl.h>

btrfs/ioctl.h:#include <linux/ioctl.h>
libdrm/drm.h:#include <asm/ioctl.h>
drm/drm.h:#include <asm/ioctl.h>
rdma/ib_user_mad.h:#include <linux/ioctl.h>
rdma/rdma_user_ioctl.h:#include <linux/ioctl.h>
sound/firewire.h:#include <linux/ioctl.h>
video/sisfb.h:#include <asm/ioctl.h>
xen/sys/xenbus_dev.h:#include <linux/ioctl.h>

so any of them will also pull in IOC_IN/INOUT/OUT.  Is mips/asm/sockios.h
the only one in that list that i  problematic for glibc, or should they
all avoid the  macros if possible?

We can probably move everything other than the six
macros from asm/ioctl.h into a new header that is
included from both asm/ioctl.h and from any other
file that wants to avoid using asm/ioctl.h.

      Arnd

Re: Ping Re: Fix bits/socket.h IOC* namespace issues (bug 21267)

[Joseph Myers]

On Fri, 24 Mar 2017, Arnd Bergmann wrote:

> There are many exported or user-space provided headers that include
> linux/ioctl.h or asm/ioctl.h in order to get the _IOC/IOW/IOR/IOWR
> macros:

That's what I expect them to be used for.

> so any of them will also pull in IOC_IN/INOUT/OUT.  Is mips/asm/sockios.h
> the only one in that list that i  problematic for glibc, or should they
> all avoid the  macros if possible?

Only mips/asm/sockios.h is currently observed to cause namespace problems.  
alpha's asm/sockios.h also uses _IOR/_IOW macros, but without including 
<asm/ioctl.h> (so I suppose the macros defined using _IOR/_IOW might not 
actually work unless a source of those macro definitions gets included 
explicitly).

-- 
Joseph S. Myers
joseph@codesourcery.com

Ping^2 Re: Fix bits/socket.h IOC* namespace issues (bug 21267)

[Joseph Myers]

Ping^2.  This patch 
<https://sourceware.org/ml/libc-alpha/2017-03/msg00400.html> is still 
pending review.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: Ping^2 Re: Fix bits/socket.h IOC* namespace issues (bug 21267)

[Adhemerval Zanella]

On 05/04/2017 12:56, Joseph Myers wrote:
> Ping^2.  This patch 
> <https://sourceware.org/ml/libc-alpha/2017-03/msg00400.html> is still 
> pending review.
> 

The patch itself looks good.  However, I think the __SYS_SOCKET_H_undef*
macro usage is convoluted and complex, couldn't we just copy the kernel
definition from asm/socket.h and avoid include it on bits/socket.h?

Re: Ping^2 Re: Fix bits/socket.h IOC* namespace issues (bug 21267)

[Joseph Myers]

On Thu, 6 Apr 2017, Adhemerval Zanella wrote:

> On 05/04/2017 12:56, Joseph Myers wrote:
> > Ping^2.  This patch 
> > <https://sourceware.org/ml/libc-alpha/2017-03/msg00400.html> is still 
> > pending review.
> > 
> 
> The patch itself looks good.  However, I think the __SYS_SOCKET_H_undef*
> macro usage is convoluted and complex, couldn't we just copy the kernel
> definition from asm/socket.h and avoid include it on bits/socket.h?

In principle that might make sense.  The issues that would need addressing 
are:

* Different definitions for different architecture may well require a 
separate header, much like bits/socket_type.h.

* Presumably people need to be able to include the glibc headers together 
with the uapi headers, but the kernel headers define these macros 
unconditionally, meaning that any difference in the text of the definition 
(even keeping the same numerical value) would result in a conflict, and 
care would be needed to avoid such conflicts along the lines described at 
<https://sourceware.org/glibc/wiki/Synchronizing_Headers>.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: Ping^2 Re: Fix bits/socket.h IOC* namespace issues (bug 21267)

[Adhemerval Zanella]

On 18/04/2017 18:28, Joseph Myers wrote:
> On Thu, 6 Apr 2017, Adhemerval Zanella wrote:
> 
>> On 05/04/2017 12:56, Joseph Myers wrote:
>>> Ping^2.  This patch 
>>> <https://sourceware.org/ml/libc-alpha/2017-03/msg00400.html> is still 
>>> pending review.
>>>
>>
>> The patch itself looks good.  However, I think the __SYS_SOCKET_H_undef*
>> macro usage is convoluted and complex, couldn't we just copy the kernel
>> definition from asm/socket.h and avoid include it on bits/socket.h?
> 
> In principle that might make sense.  The issues that would need addressing 
> are:
> 
> * Different definitions for different architecture may well require a 
> separate header, much like bits/socket_type.h.

I think it is feasible, although it would require more work.

> 
> * Presumably people need to be able to include the glibc headers together 
> with the uapi headers, but the kernel headers define these macros 
> unconditionally, meaning that any difference in the text of the definition 
> (even keeping the same numerical value) would result in a conflict, and 
> care would be needed to avoid such conflicts along the lines described at 
> <https://sourceware.org/glibc/wiki/Synchronizing_Headers>.
> 

Thanks for the information. This would require more work than I would expect
to maintain compatibility and it would require some external information that
I would prefer to avoid (such as _UAPI_LINUX_IN6_H, but I also do not see
another way to fix it).