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);
On Wed, Mar 12, 2008 at 02:25:27PM -0400, Eric Paris wrote:
> 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.
On Wed, 2008-03-12 at 18:34 +0000, Al Viro wrote:
> On Wed, Mar 12, 2008 at 02:25:27PM -0400, Eric Paris wrote:
> > 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.
Ok, I'll just make SELinux happy allow the request if we don't have
FMODE_READ or FMODE_WRITE set.
-Eric
Eric Paris <[email protected]> writes:
> 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.
--
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."
On Wed, Mar 12, 2008 at 02:25:27PM -0400, Eric Paris wrote:
> 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.
> 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
On Sat, Mar 15, 2008 at 05:59:52PM -0400, Alan Cox wrote:
> On Wed, Mar 12, 2008 at 02:25:27PM -0400, Eric Paris wrote:
> > 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.
>
> > 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.
On Sat, Mar 15, 2008 at 05:59:52PM -0400, Alan Cox wrote:
> > 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.