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

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

[Ulrich Drepper]

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 <drepper@redhat.com>

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;

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

[Andrew Morton]

On Tue, 6 May 2008 17:18:07 -0400 Ulrich Drepper <drepper@redhat.com> wrote:

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

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

[David Miller]

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 <drepper@redhat.com> wrote:
> 
> > --- 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 :-/

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

[Ulrich Drepper]

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

Andrew Morton wrote:
>> +	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-----

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

[Andrew Morton]

On Mon, 12 May 2008 20:54:34 -0700 Ulrich Drepper <drepper@redhat.com> wrote:

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Andrew Morton wrote:
> >> +	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.

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

[David Miller]

From: Ulrich Drepper <drepper@redhat.com>
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.

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

[Ulrich Drepper]

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

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

[David Miller]

From: Ulrich Drepper <drepper@redhat.com>
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.

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

[Andrew Morton]

On Mon, 12 May 2008 21:02:25 -0700 (PDT) David Miller <davem@davemloft.net> wrote:

> From: Ulrich Drepper <drepper@redhat.com>
> 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.

m68k too..

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

[Ulrich Drepper]

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

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

[Andrew Morton]

On Mon, 12 May 2008 22:10:58 -0700 Ulrich Drepper <drepper@redhat.com> wrote:

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

Different bug, I think.

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

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

[Ulrich Drepper]

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

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

[Andrew Morton]

On Mon, 12 May 2008 22:42:50 -0700 Ulrich Drepper <drepper@redhat.com> wrote:

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

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)

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

[Andrew Morton]

On Tue, 6 May 2008 17:18:07 -0400 Ulrich Drepper <drepper@redhat.com> wrote:

> --- 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);

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.

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

[Ulrich Drepper]

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