Skip to contentSkip to conversation

correct flags to f_mode conversion in __dentry_open

RFC In review Eric Paris 8 participants 9 messages Mar 12, 2008 – May 22, 2008 VFS

lore
Share & tools
lore

Eric Paris

author
#0 lore raw

RFC 1/1 correct flags to f_mode conversion in __dentry_open

I recently tried to add an SELinux BUG_ON in the case where the kernel made a permission request for no permissions and was able to stumble over it with something as simple as

open("/dev/null", 3);

Notice that 3 == (O_RDWR | O_WRONLY)

First question, is 3 ever a valid flag from from userspace to sys_open? I see in the comments proceeding do_filp_open()

/*
* Note that while the flag value (low two bits) for sys_open means:

 *      00 - read-only
 *      01 - write-only
 *      10 - read-write
 *      11 - special
 * it is changed into
 *      00 - no permissions needed
 *      01 - read-permission
 *      10 - write-permission
 *      11 - read-write

* for the internal routines (ie open_namei()/follow_link() etc). 00 is
* used by symlinks.
*/

Where someone indicated that 11 is 'special.' Does 'special' really mean invalid? And how should 'special' map to FMODE_*?

I also see that do_filp_open() does the mapping like:

        if ((namei_flags+1) & O_ACCMODE)
                namei_flags++;
and on another code path __dentry_open() is doing a similar mapping:
       f->f_mode = ((flags+1) & O_ACCMODE) | FMODE_LSEEK |
                               FMODE_PREAD | FMODE_PWRITE;

The issue at hand is that the pass through do_filp_open() with flags = 3 will result in the lower two bits still being 11 and SELinux will test for RDWR. But the pass through __dentry_open will result in an f_mode of 00 and will result in SELinux hitting my new (not yet in kernel) BUG_ON() since 11 was mapped to 00.

What is this mapping supposed to be? What is 'special' supposed to mean? I added the following patch which makes the __dentry_open() conversion more like the do_filp_open() conversion and my machine seems to be working well and surviving/acting as the way I expected. What does 11 really mean and should it really always be mapped to (FMODE_READ | FMODE_WRITE) or should it continue to get mapped to 'no permission?'

-Eric

---

fs/open.c +6 -2
index 5419853..6e04926 100644
@@ -736,10 +736,14 @@ static struct file *__dentry_open(struct dentry *dentry, struct vfsmount *mnt,
 {
 	struct inode *inode;
 	int error;
+	mode_t f_mode;
+
+	f_mode = flags & O_ACCMODE;
+	if ((f_mode+1) & O_ACCMODE)
+		f_mode++;
 
 	f->f_flags = flags;
-	f->f_mode = ((flags+1) & O_ACCMODE) | FMODE_LSEEK |
-				FMODE_PREAD | FMODE_PWRITE;
+	f->f_mode = f_mode | FMODE_LSEEK | FMODE_PREAD | FMODE_PWRITE;
 	inode = dentry->d_inode;
 	if (f->f_mode & FMODE_WRITE) {
 		error = get_write_access(inode);
maintainer
#1 lore raw
Quoted 9 lines · Eric Paris

I recently tried to add an SELinux BUG_ON in the case where the kernel made a permission request for no permissions and was able to stumble over it with something as simple as

open("/dev/null", 3);

Notice that 3 == (O_RDWR | O_WRONLY)

First question, is 3 ever a valid flag from from userspace to sys_open?

Yes. "Check for both read and write permissions, set neither FMODE_READ nor FMODE_WRITE".

Don't break drivers, please - some use that for "ioctl-only" opens, with special semantics for those.

author
#2 lore raw
Quoted 9 lines · Al Viro
Quoted 8 lines · Eric Paris

I recently tried to add an SELinux BUG_ON in the case where the kernel made a permission request for no permissions and was able to stumble over it with something as simple as

open("/dev/null", 3);

Notice that 3 == (O_RDWR | O_WRONLY)

Quoted 7 lines
Quoted 1 line

First question, is 3 ever a valid flag from from userspace to sys_open?

Yes. "Check for both read and write permissions, set neither FMODE_READ nor FMODE_WRITE".

Don't break drivers, please - some use that for "ioctl-only" opens, with special semantics for those.

Ok, I'll just make SELinux happy allow the request if we don't have FMODE_READ or FMODE_WRITE set.

-Eric

#3 lore raw
In reply to Eric Paris
Quoted 1 line · Eric Paris

Notice that 3 == (O_RDWR | O_WRONLY)

Note that O_RDWR and O_WRONLY are not simple flags, but two possible values of a 2-bit field (O_ACCMODE). Thus it does not makes sense to add them together.

Andreas.

Signature · 4 lines
Andreas Schwab, SuSE Labs, [email protected]
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."
#4 lore raw
In reply to Eric Paris
Quoted 9 lines · Eric Paris

I recently tried to add an SELinux BUG_ON in the case where the kernel made a permission request for no permissions and was able to stumble over it with something as simple as

open("/dev/null", 3);

Notice that 3 == (O_RDWR | O_WRONLY)

First question, is 3 ever a valid flag from from userspace to sys_open?

Yes.

Quoted 2 lines

does 11 really mean and should it really always be mapped to (FMODE_READ | FMODE_WRITE) or should it continue to get mapped to 'no permission?'

We've always mapped 3 to "no permission" to read or write. It's a linuxism

#5 lore raw
Quoted 9 lines · Alan Cox
Quoted 8 lines · Eric Paris

I recently tried to add an SELinux BUG_ON in the case where the kernel made a permission request for no permissions and was able to stumble over it with something as simple as

open("/dev/null", 3);

Notice that 3 == (O_RDWR | O_WRONLY)

Quoted 8 lines
Quoted 1 line

First question, is 3 ever a valid flag from from userspace to sys_open?

Yes.

Quoted 2 lines

does 11 really mean and should it really always be mapped to (FMODE_READ | FMODE_WRITE) or should it continue to get mapped to 'no permission?'

We've always mapped 3 to "no permission" to read or write. It's a linuxism

Note that we *do* request MAY_READ|MAY_WRITE; "no permission" part is about what you can do to resulting descriptor afterwards.

#6 lore raw
In reply to Alan Cox
Quoted 4 lines · Alan Cox
Quoted 2 lines

does 11 really mean and should it really always be mapped to (FMODE_READ | FMODE_WRITE) or should it continue to get mapped to 'no permission?'

We've always mapped 3 to "no permission" to read or write. It's a linuxism

I've tripped over this recently aswell. It would for sure be useful to add a sumbolic O_FOO constant for this magic value '3' and document it in the manpage.

#7 lore raw
Quoted 8 lines · Christoph Hellwig
Quoted 3 lines · Alan Cox
Quoted 2 lines

does 11 really mean and should it really always be mapped to (FMODE_READ | FMODE_WRITE) or should it continue to get mapped to 'no permission?'

We've always mapped 3 to "no permission" to read or write. It's a linuxism

I've tripped over this recently aswell. It would for sure be useful to add a sumbolic O_FOO constant for this magic value '3' and document it in the manpage.

Late follow-up to this thread
(http://thread.gmane.org/gmane.linux.kernel/653123):
I propose to add the following text to the open(2) man page.

Unlike the other values that can be specified in flags, the access mode values O_RDONLY, O_WRONLY, and O_RDWR, do not specify individual bits. Rather, they define the low order two bits of flags, and are defined respectively as 0, 1, and 2. In other words, the combination O_RDONLY | O_WRONLY is a logical error, and certainly does not have the same meaning as O_RDWR. Linux reserves the special, non-standard access mode 3 (binary 11) in flags to mean:
check for read and write permission on the file and return a descriptor that can't be used for reading or writing. This non-standard access mode is used by some Linux drivers to return a descriptor that is only to be used for device-specific ioctl(2) operations.

Seem okay?

Cheers,

Michael

Signature · 3 lines
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Found a bug? http://www.kernel.org/doc/man-pages/reporting_bugs.html
#8 lore raw
Quoted 28 lines · Michael Kerrisk
Quoted 8 lines · Christoph Hellwig
Quoted 3 lines · Alan Cox
Quoted 2 lines

does 11 really mean and should it really always be mapped to (FMODE_READ | FMODE_WRITE) or should it continue to get mapped to 'no permission?'

We've always mapped 3 to "no permission" to read or write. It's a linuxism

I've tripped over this recently aswell. It would for sure be useful to add a sumbolic O_FOO constant for this magic value '3' and document it in the manpage.

Late follow-up to this thread
(http://thread.gmane.org/gmane.linux.kernel/653123):
I propose to add the following text to the open(2) man page.

Unlike the other values that can be specified in flags, the access mode values O_RDONLY, O_WRONLY, and O_RDWR, do not specify individual bits. Rather, they define the low order two bits of flags, and are defined respectively as 0, 1, and 2. In other words, the combination O_RDONLY | O_WRONLY is a logical error, and certainly does not have the same meaning as O_RDWR. Linux reserves the special, non-standard access mode 3 (binary 11) in flags to mean:
check for read and write permission on the file and return a descriptor that can't be used for reading or writing. This non-standard access mode is used by some Linux drivers to return a descriptor that is only to be used for device-specific ioctl(2) operations.

Quoted 1 line

Seem okay?

Looks good to me. We definitely need this documented.

- James

Signature · 2 lines
James Morris
<[email protected]>