All of lore.kernel.org
 help / color / mirror / Atom feed
* [patch 2/5] vfs: O_* bit numbers uniqueness check
@ 2010-07-20 22:29 akpm
  2010-08-26 23:22 ` James Bottomley
  0 siblings, 1 reply; 6+ messages in thread
From: akpm @ 2010-07-20 22:29 UTC (permalink / raw)
  To: viro
  Cc: linux-fsdevel, akpm, fengguang.wu, davem, eparis, hch, jamie,
	rdreier, schwab, sfr

From: Wu Fengguang <fengguang.wu@intel.com>

The O_* bit numbers are defined in 20+ arch/*, and can silently overlap. 
Add a compile time check to ensure the uniqueness as suggested by David
Miller.

Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
Cc: David Miller <davem@davemloft.net>
Cc: Stephen Rothwell <sfr@canb.auug.org.au>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Eric Paris <eparis@redhat.com>
Cc: Roland Dreier <rdreier@cisco.com>
Cc: Jamie Lokier <jamie@shareable.org>
Cc: Andreas Schwab <schwab@linux-m68k.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 fs/fcntl.c                  |   15 +++++++++++++--
 include/asm-generic/fcntl.h |    4 ++++
 2 files changed, 17 insertions(+), 2 deletions(-)

diff -puN fs/fcntl.c~vfs-o_-bit-numbers-uniqueness-check fs/fcntl.c
--- a/fs/fcntl.c~vfs-o_-bit-numbers-uniqueness-check
+++ a/fs/fcntl.c
@@ -767,11 +767,22 @@ void kill_fasync(struct fasync_struct **
 }
 EXPORT_SYMBOL(kill_fasync);
 
-static int __init fasync_init(void)
+static int __init fcntl_init(void)
 {
+	/* please add new bits here to ensure allocation uniqueness */
+	BUILD_BUG_ON(19 - 1 /* for O_RDONLY being 0 */ != HWEIGHT32(
+		O_RDONLY	| O_WRONLY	| O_RDWR	|
+		O_CREAT		| O_EXCL	| O_NOCTTY	|
+		O_TRUNC		| O_APPEND	| O_NONBLOCK	|
+		__O_SYNC	| O_DSYNC	| FASYNC	|
+		O_DIRECT	| O_LARGEFILE	| O_DIRECTORY	|
+		O_NOFOLLOW	| O_NOATIME	| O_CLOEXEC	|
+		FMODE_EXEC
+		));
+
 	fasync_cache = kmem_cache_create("fasync_cache",
 		sizeof(struct fasync_struct), 0, SLAB_PANIC, NULL);
 	return 0;
 }
 
-module_init(fasync_init)
+module_init(fcntl_init)
diff -puN include/asm-generic/fcntl.h~vfs-o_-bit-numbers-uniqueness-check include/asm-generic/fcntl.h
--- a/include/asm-generic/fcntl.h~vfs-o_-bit-numbers-uniqueness-check
+++ a/include/asm-generic/fcntl.h
@@ -11,6 +11,10 @@
  * -Eric Paris
  */
 
+/*
+ * When introducing new O_* bits, please check its uniqueness in fcntl_init().
+ */
+
 #define O_ACCMODE	00000003
 #define O_RDONLY	00000000
 #define O_WRONLY	00000001
_

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [patch 2/5] vfs: O_* bit numbers uniqueness check
  2010-07-20 22:29 [patch 2/5] vfs: O_* bit numbers uniqueness check akpm
@ 2010-08-26 23:22 ` James Bottomley
  2010-08-27  1:27   ` Jamie Lokier
  0 siblings, 1 reply; 6+ messages in thread
From: James Bottomley @ 2010-08-26 23:22 UTC (permalink / raw)
  To: akpm
  Cc: viro, linux-fsdevel, fengguang.wu, davem, eparis, hch, jamie,
	rdreier, schwab, sfr

On Tue, 2010-07-20 at 15:29 -0700, akpm@linux-foundation.org wrote:
> From: Wu Fengguang <fengguang.wu@intel.com>
> 
> The O_* bit numbers are defined in 20+ arch/*, and can silently overlap. 
> Add a compile time check to ensure the uniqueness as suggested by David
> Miller.

Can we get this reverted or fixed?  It's causing the parisc compiles to
fail.  The reason is O_NONBLOCK on parisc has a dual value:

#define O_NONBLOCK	000200004 /* HPUX has separate NDELAY & NONBLOCK */

The fix would be to take O_NONBLOCK out.

James

---

diff --git a/fs/fcntl.c b/fs/fcntl.c
index 6769fd0..aeb02aa 100644
--- a/fs/fcntl.c
+++ b/fs/fcntl.c
@@ -770,10 +770,11 @@ EXPORT_SYMBOL(kill_fasync);
 static int __init fcntl_init(void)
 {
 	/* please add new bits here to ensure allocation uniqueness */
-	BUILD_BUG_ON(19 - 1 /* for O_RDONLY being 0 */ != HWEIGHT32(
+	BUILD_BUG_ON(18 - 1 /* for O_RDONLY being 0 */ != HWEIGHT32(
 		O_RDONLY	| O_WRONLY	| O_RDWR	|
 		O_CREAT		| O_EXCL	| O_NOCTTY	|
-		O_TRUNC		| O_APPEND	| O_NONBLOCK	|
+		/* remove O_NONBLOCK it's a two bit define on parisc */
+		O_TRUNC		| O_APPEND	| /* O_NONBLOCK	| */
 		__O_SYNC	| O_DSYNC	| FASYNC	|
 		O_DIRECT	| O_LARGEFILE	| O_DIRECTORY	|
 		O_NOFOLLOW	| O_NOATIME	| O_CLOEXEC	|



^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [patch 2/5] vfs: O_* bit numbers uniqueness check
  2010-08-26 23:22 ` James Bottomley
@ 2010-08-27  1:27   ` Jamie Lokier
  2010-08-27  5:53     ` James Bottomley
  0 siblings, 1 reply; 6+ messages in thread
From: Jamie Lokier @ 2010-08-27  1:27 UTC (permalink / raw)
  To: James Bottomley
  Cc: akpm, viro, linux-fsdevel, fengguang.wu, davem, eparis, hch,
	rdreier, schwab, sfr

James Bottomley wrote:
> On Tue, 2010-07-20 at 15:29 -0700, akpm@linux-foundation.org wrote:
> > From: Wu Fengguang <fengguang.wu@intel.com>
> > 
> > The O_* bit numbers are defined in 20+ arch/*, and can silently overlap. 
> > Add a compile time check to ensure the uniqueness as suggested by David
> > Miller.
> 
> Can we get this reverted or fixed?  It's causing the parisc compiles to
> fail.  The reason is O_NONBLOCK on parisc has a dual value:
> 
> #define O_NONBLOCK	000200004 /* HPUX has separate NDELAY & NONBLOCK */
> 
> The fix would be to take O_NONBLOCK out.

A more thoroughly checking fix would be

BUILD_BUG_ON(18 - 1 /* For O_RDONLY being 0 */
             + HWEIGHT32(O_NONBLOCK) /* Because it's 2 bits on parisc */
             != HWEIGHT32(    .... all the bits ....     ));

Am I allowed to Sign-off handwavy pseudocode? ;-)

-- Jamie

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [patch 2/5] vfs: O_* bit numbers uniqueness check
  2010-08-27  1:27   ` Jamie Lokier
@ 2010-08-27  5:53     ` James Bottomley
  2010-09-04  1:22       ` [PATCH] vfs: take O_NONBLOCK out of the O_* uniqueness test Wu Fengguang
  0 siblings, 1 reply; 6+ messages in thread
From: James Bottomley @ 2010-08-27  5:53 UTC (permalink / raw)
  To: Jamie Lokier
  Cc: akpm, viro, linux-fsdevel, fengguang.wu, davem, eparis, hch,
	rdreier, schwab, sfr

On Fri, 2010-08-27 at 02:27 +0100, Jamie Lokier wrote:
> James Bottomley wrote:
> > On Tue, 2010-07-20 at 15:29 -0700, akpm@linux-foundation.org wrote:
> > > From: Wu Fengguang <fengguang.wu@intel.com>
> > > 
> > > The O_* bit numbers are defined in 20+ arch/*, and can silently overlap. 
> > > Add a compile time check to ensure the uniqueness as suggested by David
> > > Miller.
> > 
> > Can we get this reverted or fixed?  It's causing the parisc compiles to
> > fail.  The reason is O_NONBLOCK on parisc has a dual value:
> > 
> > #define O_NONBLOCK	000200004 /* HPUX has separate NDELAY & NONBLOCK */
> > 
> > The fix would be to take O_NONBLOCK out.
> 
> A more thoroughly checking fix would be
> 
> BUILD_BUG_ON(18 - 1 /* For O_RDONLY being 0 */
>              + HWEIGHT32(O_NONBLOCK) /* Because it's 2 bits on parisc */
>              != HWEIGHT32(    .... all the bits ....     ));

Well, it works, but is it wise?  There are several missing flags in this
expression: O_NDELAY being the most noticeable one.  It's defined to be
the sames as O_NONBLOCK on some platforms and not on others.  Our
O_NONBLOCK problem is essentially the same class.

> Am I allowed to Sign-off handwavy pseudocode? ;-)

Nope ... you'd have to code it as a real patch ...

James



^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH] vfs: take O_NONBLOCK out of the O_* uniqueness test
  2010-08-27  5:53     ` James Bottomley
@ 2010-09-04  1:22       ` Wu Fengguang
  0 siblings, 0 replies; 6+ messages in thread
From: Wu Fengguang @ 2010-09-04  1:22 UTC (permalink / raw)
  To: James Bottomley
  Cc: Jamie Lokier, akpm, viro, linux-fsdevel, davem, eparis, hch,
	rdreier, schwab, sfr

I tend to agree with James' approach. How about this patch?
Compile tested on x86.

===
vfs: take O_NONBLOCK out of the O_* uniqueness test
From: James Bottomley <James.Bottomley@suse.de>

O_NONBLOCK on parisc has a dual value:

#define O_NONBLOCK	000200004 /* HPUX has separate NDELAY & NONBLOCK */

It is caught by the O_* bits uniqueness check and leads to a parisc
compile error.  The fix would be to take O_NONBLOCK out.

Cc: Jamie Lokier <jamie@shareable.org>
Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
Signed-off-by: James Bottomley <James.Bottomley@suse.de>
---
 fs/fcntl.c |   10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

--- linux-next.orig/fs/fcntl.c	2010-08-20 06:57:11.000000000 +0800
+++ linux-next/fs/fcntl.c	2010-09-04 09:16:09.000000000 +0800
@@ -769,11 +769,15 @@ EXPORT_SYMBOL(kill_fasync);
 
 static int __init fcntl_init(void)
 {
-	/* please add new bits here to ensure allocation uniqueness */
-	BUILD_BUG_ON(19 - 1 /* for O_RDONLY being 0 */ != HWEIGHT32(
+	/*
+	 * Please add new bits here to ensure allocation uniqueness.
+	 * Exceptions: O_NONBLOCK is a two bit define on parisc; O_NDELAY
+	 * is defined as O_NONBLOCK on some platforms and not on others.
+	 */
+	BUILD_BUG_ON(18 - 1 /* for O_RDONLY being 0 */ != HWEIGHT32(
 		O_RDONLY	| O_WRONLY	| O_RDWR	|
 		O_CREAT		| O_EXCL	| O_NOCTTY	|
-		O_TRUNC		| O_APPEND	| O_NONBLOCK	|
+		O_TRUNC		| O_APPEND	| /* O_NONBLOCK	| */
 		__O_SYNC	| O_DSYNC	| FASYNC	|
 		O_DIRECT	| O_LARGEFILE	| O_DIRECTORY	|
 		O_NOFOLLOW	| O_NOATIME	| O_CLOEXEC	|

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [patch 2/5] vfs: O_* bit numbers uniqueness check
@ 2010-05-24 19:24 akpm
  0 siblings, 0 replies; 6+ messages in thread
From: akpm @ 2010-05-24 19:24 UTC (permalink / raw)
  To: viro
  Cc: linux-fsdevel, akpm, fengguang.wu, davem, eparis, hch, jamie,
	rdreier, schwab, sfr

From: Wu Fengguang <fengguang.wu@intel.com>

The O_* bit numbers are defined in 20+ arch/*, and can silently overlap. 
Add a compile time check to ensure the uniqueness as suggested by David
Miller.

Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
Cc: David Miller <davem@davemloft.net>
Cc: Stephen Rothwell <sfr@canb.auug.org.au>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Eric Paris <eparis@redhat.com>
Cc: Roland Dreier <rdreier@cisco.com>
Cc: Jamie Lokier <jamie@shareable.org>
Cc: Andreas Schwab <schwab@linux-m68k.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 fs/fcntl.c                  |   15 +++++++++++++--
 include/asm-generic/fcntl.h |    4 ++++
 2 files changed, 17 insertions(+), 2 deletions(-)

diff -puN fs/fcntl.c~vfs-o_-bit-numbers-uniqueness-check fs/fcntl.c
--- a/fs/fcntl.c~vfs-o_-bit-numbers-uniqueness-check
+++ a/fs/fcntl.c
@@ -762,11 +762,22 @@ void kill_fasync(struct fasync_struct **
 }
 EXPORT_SYMBOL(kill_fasync);
 
-static int __init fasync_init(void)
+static int __init fcntl_init(void)
 {
+	/* please add new bits here to ensure allocation uniqueness */
+	BUILD_BUG_ON(19 - 1 /* for O_RDONLY being 0 */ != HWEIGHT32(
+		O_RDONLY	| O_WRONLY	| O_RDWR	|
+		O_CREAT		| O_EXCL	| O_NOCTTY	|
+		O_TRUNC		| O_APPEND	| O_NONBLOCK	|
+		__O_SYNC	| O_DSYNC	| FASYNC	|
+		O_DIRECT	| O_LARGEFILE	| O_DIRECTORY	|
+		O_NOFOLLOW	| O_NOATIME	| O_CLOEXEC	|
+		FMODE_EXEC
+		));
+
 	fasync_cache = kmem_cache_create("fasync_cache",
 		sizeof(struct fasync_struct), 0, SLAB_PANIC, NULL);
 	return 0;
 }
 
-module_init(fasync_init)
+module_init(fcntl_init)
diff -puN include/asm-generic/fcntl.h~vfs-o_-bit-numbers-uniqueness-check include/asm-generic/fcntl.h
--- a/include/asm-generic/fcntl.h~vfs-o_-bit-numbers-uniqueness-check
+++ a/include/asm-generic/fcntl.h
@@ -11,6 +11,10 @@
  * -Eric Paris
  */
 
+/*
+ * When introducing new O_* bits, please check its uniqueness in fcntl_init().
+ */
+
 #define O_ACCMODE	00000003
 #define O_RDONLY	00000000
 #define O_WRONLY	00000001
_

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-09-04  1:22 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-07-20 22:29 [patch 2/5] vfs: O_* bit numbers uniqueness check akpm
2010-08-26 23:22 ` James Bottomley
2010-08-27  1:27   ` Jamie Lokier
2010-08-27  5:53     ` James Bottomley
2010-09-04  1:22       ` [PATCH] vfs: take O_NONBLOCK out of the O_* uniqueness test Wu Fengguang
  -- strict thread matches above, loose matches on Subject: below --
2010-05-24 19:24 [patch 2/5] vfs: O_* bit numbers uniqueness check akpm

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.