[PATCH 18/18] flag parameters: check magic constants

This patch adds test that ensure the boundary conditions for the various
constants introduced in the previous patches is met.  No code is generated.

 fs/eventfd.c      |    4 ++++
 fs/eventpoll.c    |    3 +++
 fs/inotify_user.c |    4 ++++
 fs/signalfd.c     |    4 ++++
 fs/timerfd.c      |    4 ++++
 net/socket.c      |    7 +++++++
 6 files changed, 26 insertions(+)


Signed-off-by: Ulrich Drepper <redacted>

diff --git a/fs/eventfd.c b/fs/eventfd.c
index 3ed4466..08bf558 100644
--- a/fs/eventfd.c
+++ b/fs/eventfd.c

@@ -203,6 +203,10 @@ asmlinkage long sys_eventfd2(unsigned int count, int flags)
 	int fd;
 	struct eventfd_ctx *ctx;
 
+	/* Check the EFD_* constants for consistency.  */
+	BUILD_BUG_ON(EFD_CLOEXEC != O_CLOEXEC);
+	BUILD_BUG_ON(EFD_NONBLOCK != O_NONBLOCK);
+
 	if (flags & ~(EFD_CLOEXEC | EFD_NONBLOCK))
 		return -EINVAL;
 

diff --git a/fs/eventpoll.c b/fs/eventpoll.c
index 3fd4014..2fdad42 100644
--- a/fs/eventpoll.c
+++ b/fs/eventpoll.c

@@ -1051,6 +1051,9 @@ asmlinkage long sys_epoll_create2(int size, int flags)
 	int error, fd = -1;
 	struct eventpoll *ep;
 
+	/* Check the EPOLL_* constant for consistency.  */
+	BUILD_BUG_ON(EPOLL_CLOEXEC != O_CLOEXEC);
+
 	if (flags & ~EPOLL_CLOEXEC)
 		return -EINVAL;
 

diff --git a/fs/inotify_user.c b/fs/inotify_user.c
index dc7e1f6..fe79c25 100644
--- a/fs/inotify_user.c
+++ b/fs/inotify_user.c

@@ -574,6 +574,10 @@ asmlinkage long sys_inotify_init1(int flags)
 	struct file *filp;
 	int fd, ret;
 
+	/* Check the IN_* constants for consistency.  */
+	BUILD_BUG_ON(IN_CLOEXEC != O_CLOEXEC);
+	BUILD_BUG_ON(IN_NONBLOCK != O_NONBLOCK);
+
 	if (flags & ~(IN_CLOEXEC | IN_NONBLOCK))
 		return -EINVAL;
 

diff --git a/fs/signalfd.c b/fs/signalfd.c
index 5441a4b..9c39bc7 100644
--- a/fs/signalfd.c
+++ b/fs/signalfd.c

@@ -211,6 +211,10 @@ asmlinkage long sys_signalfd4(int ufd, sigset_t __user *user_mask,
 	sigset_t sigmask;
 	struct signalfd_ctx *ctx;
 
+	/* Check the SFD_* constants for consistency.  */
+	BUILD_BUG_ON(SFD_CLOEXEC != O_CLOEXEC);
+	BUILD_BUG_ON(SFD_NONBLOCK != O_NONBLOCK);
+
 	if (flags & ~(SFD_CLOEXEC | SFD_NONBLOCK))
 		return -EINVAL;
 

diff --git a/fs/timerfd.c b/fs/timerfd.c
index 75d44ef..c502c60 100644
--- a/fs/timerfd.c
+++ b/fs/timerfd.c

@@ -184,6 +184,10 @@ asmlinkage long sys_timerfd_create(int clockid, int flags)
 	int ufd;
 	struct timerfd_ctx *ctx;
 
+	/* Check the TFD_* constants for consistency.  */
+	BUILD_BUG_ON(TFD_CLOEXEC != O_CLOEXEC);
+	BUILD_BUG_ON(TFD_NONBLOCK != O_NONBLOCK);
+
 	if (flags & ~(TFD_CLOEXEC | TFD_NONBLOCK))
 		return -EINVAL;
 	if (clockid != CLOCK_MONOTONIC &&

diff --git a/net/socket.c b/net/socket.c
index 7cb0a5e..962bf07 100644
--- a/net/socket.c
+++ b/net/socket.c

@@ -1219,6 +1219,13 @@ asmlinkage long sys_socket(int family, int type, int protocol)
 	struct socket *sock;
 	int flags;
 
+	/* Check the SOCK_* constants for consistency.  */
+	BUILD_BUG_ON(SOCK_CLOEXEC != O_CLOEXEC);
+	BUILD_BUG_ON(SOCK_NONBLOCK != O_NONBLOCK);
+	BUILD_BUG_ON((SOCK_MAX | SOCK_TYPE_MASK) != SOCK_TYPE_MASK);
+	BUILD_BUG_ON(SOCK_CLOEXEC & SOCK_TYPE_MASK);
+	BUILD_BUG_ON(SOCK_NONBLOCK & SOCK_TYPE_MASK);
+
 	flags = type & ~SOCK_TYPE_MASK;
 	if (flags & ~(SOCK_CLOEXEC | SOCK_NONBLOCK))
 		return -EINVAL;

On Tue, 6 May 2008 17:18:07 -0400 Ulrich Drepper [off-list ref] wrote:

The fifth assertion triggers with alpha allmodconfig.

From: Andrew Morton <akpm@linux-foundation.org>
Date: Mon, 12 May 2008 20:13:41 -0700

On Tue, 6 May 2008 17:18:07 -0400 Ulrich Drepper [off-list ref] wrote:

quoted

--- a/net/socket.c
+++ b/net/socket.c

@@ -1219,6 +1219,13 @@ asmlinkage long sys_socket(int family, int type, int protocol)
 	struct socket *sock;
 	int flags;
 
+	/* Check the SOCK_* constants for consistency.  */
+	BUILD_BUG_ON(SOCK_CLOEXEC != O_CLOEXEC);
+	BUILD_BUG_ON(SOCK_NONBLOCK != O_NONBLOCK);
+	BUILD_BUG_ON((SOCK_MAX | SOCK_TYPE_MASK) != SOCK_TYPE_MASK);
+	BUILD_BUG_ON(SOCK_CLOEXEC & SOCK_TYPE_MASK);
+	BUILD_BUG_ON(SOCK_NONBLOCK & SOCK_TYPE_MASK);

The fifth assertion triggers with alpha allmodconfig.

Unfortunately, MIPS and PARISC look like they will as well :-/

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Andrew Morton wrote:

quoted

+	BUILD_BUG_ON(SOCK_NONBLOCK & SOCK_TYPE_MASK);

The fifth assertion triggers with alpha allmodconfig.

This means the simple approach won't work for alpha.

The question is how to solve it?  I suggest the following:

- - define SOCK_NONBLOCK to some value > 16 in include/asm-alpha/socket.h

- - add #ifndef SOCK_NONBLOCK around the definition in include/linux/net.h

- - in sys_socket, sys_socketpair, sys_paccept

   int fflags = flags;
   if (SOCK_NONBLOCK != O_NONBLOCK && (flags & SOCK_NONBLOCK)) {
     fflags &= ~SOCK_NONBLOCK;
     fflags |= O_NONBLOCK;
   }

  and use fflags instead of flags in the places where we pass it on.

This has no costs on platforms other than alpha.

Shall I sent a patch?

- --
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org

iEYEARECAAYFAkgpEPoACgkQ2ijCOnn/RHQ38gCgyVOI/urGeoIr7CFIfs8C7OyR
I80AniPGGWyxZYUQg0pQlySQHqX/UYlx
=5fA2
-----END PGP SIGNATURE-----

On Mon, 12 May 2008 20:54:34 -0700 Ulrich Drepper [off-list ref] wrote:

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Andrew Morton wrote:

quoted

quoted

+	BUILD_BUG_ON(SOCK_NONBLOCK & SOCK_TYPE_MASK);

The fifth assertion triggers with alpha allmodconfig.

This means the simple approach won't work for alpha.

The question is how to solve it?  I suggest the following:

- - define SOCK_NONBLOCK to some value > 16 in include/asm-alpha/socket.h

- - add #ifndef SOCK_NONBLOCK around the definition in include/linux/net.h

With a comment, please.

- - in sys_socket, sys_socketpair, sys_paccept

   int fflags = flags;
   if (SOCK_NONBLOCK != O_NONBLOCK && (flags & SOCK_NONBLOCK)) {
     fflags &= ~SOCK_NONBLOCK;
     fflags |= O_NONBLOCK;
   }

  and use fflags instead of flags in the places where we pass it on.

This has no costs on platforms other than alpha.

Shall I sent a patch?

Sounds reasonable.

From: Ulrich Drepper <redacted>
Date: Mon, 12 May 2008 20:54:34 -0700

This has no costs on platforms other than alpha.

Shall I sent a patch?

You'll need to handle MIPS and PARISC as well, as they have
similar conflicts.

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

David Miller wrote:

You'll need to handle MIPS and PARISC as well, as they have
similar conflicts.

Is MIPS really a problem?  The value is 0x80.  128 protocol types should
be enough.  Even if not, there is no reason why the numbers should be
consecutive.  It's easy enough to skip over a bit.  I.e., after procotol
0x7f the next one would be 0x100.

And PA seems to have gotten O_NONBLOCK wrong.  SPARC does it right AFAICS.

- --
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (GNU/Linux)

iD8DBQFIKRdJ2ijCOnn/RHQRAovKAKDFz2ZJ0E2XYVu5Qn8fME+ieZZopwCgmH4D
ldfvNrhK8LdvjZzNPViILV0=
=Z9Dj
-----END PGP SIGNATURE-----

From: Ulrich Drepper <redacted>
Date: Mon, 12 May 2008 21:21:29 -0700

Is MIPS really a problem?  The value is 0x80.  128 protocol types should
be enough.  Even if not, there is no reason why the numbers should be
consecutive.  It's easy enough to skip over a bit.  I.e., after procotol
0x7f the next one would be 0x100.

Indeed, MIPS looks good.

And PA seems to have gotten O_NONBLOCK wrong.  SPARC does it right AFAICS.

I agree.

But that value is compiled into every parisc binary out there,
limiting what we can do about it in the short term.

On Mon, 12 May 2008 21:02:25 -0700 (PDT) David Miller [off-list ref] wrote:

From: Ulrich Drepper <redacted>
Date: Mon, 12 May 2008 20:54:34 -0700

quoted

This has no costs on platforms other than alpha.

Shall I sent a patch?

You'll need to handle MIPS and PARISC as well, as they have
similar conflicts.

m68k too..

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Andrew Morton wrote:

m68k too..

I cannot build it here but looking at the headers:

- - include/asm-m68k/fcntl.h does not define O_NONBLOCK; and therefore

- - the definition from include/asm-generic/fcntl.h is used which is 04000


Is there any magic I miss?

- --
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (GNU/Linux)

iD8DBQFIKSLi2ijCOnn/RHQRAvItAJ45n2h20UasryYucg18pd9ERWUpbACglsvR
ftad99qXQ20UwezP0DjbssA=
=orzm
-----END PGP SIGNATURE-----

On Mon, 12 May 2008 22:10:58 -0700 Ulrich Drepper [off-list ref] wrote:

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Andrew Morton wrote:

quoted

m68k too..

I cannot build it here but looking at the headers:

- - include/asm-m68k/fcntl.h does not define O_NONBLOCK; and therefore

- - the definition from include/asm-generic/fcntl.h is used which is 04000


Is there any magic I miss?

Different bug, I think.

net/socket.c: In function 'sys_paccept':
net/socket.c:1543: error: implicit declaration of function 'set_restore_sigmask'

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Andrew Morton wrote:

Different bug, I think.

net/socket.c: In function 'sys_paccept':
net/socket.c:1543: error: implicit declaration of function 'set_restore_sigmask'

Right, that's the signal mask stuff.  Should be fixed by the other patch
I sent.

- --
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (GNU/Linux)

iD8DBQFIKSpa2ijCOnn/RHQRAmPvAJ90D3ZHrIWs1ZE5K9XVEdZBh8vcdQCePcaw
1Hnyz58x9oizIXe6qiS82xU=
=o78L
-----END PGP SIGNATURE-----

On Mon, 12 May 2008 22:42:50 -0700 Ulrich Drepper [off-list ref] wrote:

Andrew Morton wrote:

quoted

Different bug, I think.

net/socket.c: In function 'sys_paccept':
net/socket.c:1543: error: implicit declaration of function 'set_restore_sigmask'

Right, that's the signal mask stuff.  Should be fixed by the other patch
I sent.

mips allmodconfig is unhappy still.


net/socket.c: In function `sys_socket':
net/socket.c:1225: error: `SOCK_CLOEXEC' undeclared (first use in this function)
net/socket.c:1225: error: (Each undeclared identifier is reported only once
net/socket.c:1225: error: for each function it appears in.)
net/socket.c:1226: error: `SOCK_NONBLOCK' undeclared (first use in this function)
net/socket.c:1227: error: `SOCK_TYPE_MASK' undeclared (first use in this function)
net/socket.c: In function `sys_socketpair':
net/socket.c:1268: error: `SOCK_TYPE_MASK' undeclared (first use in this function)
net/socket.c:1269: error: `SOCK_CLOEXEC' undeclared (first use in this function)
net/socket.c:1269: error: `SOCK_NONBLOCK' undeclared (first use in this function)
net/socket.c: In function `do_accept':
net/socket.c:1438: error: `SOCK_CLOEXEC' undeclared (first use in this function)
net/socket.c:1438: error: `SOCK_NONBLOCK' undeclared (first use in this function)

On Tue, 6 May 2008 17:18:07 -0400 Ulrich Drepper [off-list ref] wrote:

On alpha the

	BUILD_BUG_ON(SOCK_NONBLOCK & SOCK_TYPE_MASK);

seems to have gone away, but now

	BUILD_BUG_ON(SOCK_NONBLOCK != O_NONBLOCK);

is triggering.

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Andrew Morton wrote:

	BUILD_BUG_ON(SOCK_NONBLOCK != O_NONBLOCK);

After the addition of arch-specific SOCK_NONBLOCK values this line must
go.  I thought one of my patches already did that.

- --
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)

iEYEARECAAYFAkg21SoACgkQ2ijCOnn/RHQG2wCeNS8o/cVxuOYXRbCnwFTg1rcx
590AoIvJBnwNP9O26kIHNY3ExVTA2M2a
=5P4P
-----END PGP SIGNATURE-----