lkml.org 
[lkml]   [2008]   [Mar]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Subject[RFC] correct flags to f_mode conversion in __dentry_open
From
Date
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

---

diff --git a/fs/open.c b/fs/open.c
index 5419853..6e04926 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -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);



\
 
 \ /
  Last update: 2008-03-12 19:29    [W:0.084 / U:0.520 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site