* [PATCH] fs: Evaluate O_WRONLY | O_RDWR to O_RDWR
@ 2018-12-13 11:57 Goldwyn Rodrigues
2018-12-13 12:10 ` Al Viro
0 siblings, 1 reply; 6+ messages in thread
From: Goldwyn Rodrigues @ 2018-12-13 11:57 UTC (permalink / raw)
To: linux-fsdevel
Cc: amir73il, zohar, syzbot+ae82084b07d0297e566b, syzkaller-bugs,
linux-integrity, linux-unionfs, dvyukov
A user can open(O_WRONLY | O_RDWR) and the options are valid.
However, OPEN_FMODE() evaluates both FMODE_READ and FMODE_WRITE,
as negative. We also need to protect the lower layers from this
anomaly.
Solve it by dropping O_WRONLY, so O_RDWR takes precedence.
Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
Reported-by: syzbot+ae82084b07d0297e566b@syzkaller.appspotmail.com
Fixes: a408e4a86b36 ("ima: open a new file instance if no read permissions")
---
fs/open.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/fs/open.c b/fs/open.c
index 0285ce7dbd51..3d0a21600fa3 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -924,7 +924,12 @@ EXPORT_SYMBOL(open_with_fake_path);
static inline int build_open_flags(int flags, umode_t mode, struct open_flags *op)
{
int lookup_flags = 0;
- int acc_mode = ACC_MODE(flags);
+ int acc_mode;
+
+ if ((flags & (O_RDWR | O_WRONLY)) == (O_RDWR | O_WRONLY))
+ flags &= ~O_WRONLY;
+
+ acc_mode = ACC_MODE(flags);
/*
* Clear out all open flags we don't know about so that we don't report
--
2.16.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] fs: Evaluate O_WRONLY | O_RDWR to O_RDWR
2018-12-13 11:57 [PATCH] fs: Evaluate O_WRONLY | O_RDWR to O_RDWR Goldwyn Rodrigues
@ 2018-12-13 12:10 ` Al Viro
2018-12-13 12:16 ` Amir Goldstein
2018-12-13 12:16 ` Dmitry Vyukov
0 siblings, 2 replies; 6+ messages in thread
From: Al Viro @ 2018-12-13 12:10 UTC (permalink / raw)
To: Goldwyn Rodrigues
Cc: linux-fsdevel, amir73il, zohar, syzbot+ae82084b07d0297e566b,
syzkaller-bugs, linux-integrity, linux-unionfs, dvyukov
On Thu, Dec 13, 2018 at 05:57:17AM -0600, Goldwyn Rodrigues wrote:
> A user can open(O_WRONLY | O_RDWR) and the options are valid.
> However, OPEN_FMODE() evaluates both FMODE_READ and FMODE_WRITE,
> as negative. We also need to protect the lower layers from this
> anomaly.
>
> Solve it by dropping O_WRONLY, so O_RDWR takes precedence.
Congratulations, you've broken fdutils... Passing 3 in lower bits
of open() flags is *not* the same as O_RDWR; behaiviour is
different and deliberately chosen by existing userland code.
IOW, NAK.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] fs: Evaluate O_WRONLY | O_RDWR to O_RDWR
2018-12-13 12:10 ` Al Viro
@ 2018-12-13 12:16 ` Amir Goldstein
2018-12-13 12:16 ` Dmitry Vyukov
1 sibling, 0 replies; 6+ messages in thread
From: Amir Goldstein @ 2018-12-13 12:16 UTC (permalink / raw)
To: Goldwyn Rodrigues
Cc: linux-fsdevel, zohar, syzbot+ae82084b07d0297e566b,
syzkaller-bugs, linux-integrity, overlayfs, Dmitry Vyukov,
Al Viro
On Thu, Dec 13, 2018 at 2:10 PM Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> On Thu, Dec 13, 2018 at 05:57:17AM -0600, Goldwyn Rodrigues wrote:
> > A user can open(O_WRONLY | O_RDWR) and the options are valid.
> > However, OPEN_FMODE() evaluates both FMODE_READ and FMODE_WRITE,
> > as negative. We also need to protect the lower layers from this
> > anomaly.
> >
> > Solve it by dropping O_WRONLY, so O_RDWR takes precedence.
>
> Congratulations, you've broken fdutils... Passing 3 in lower bits
> of open() flags is *not* the same as O_RDWR; behaiviour is
> different and deliberately chosen by existing userland code.
>
> IOW, NAK.
Yap, sorry, I missed FMODE_WRITE_IOCTL when I made this suggestion.
open mode 3 seems to be reserved to some special case of ioctl to floppy
block device.
I guess it would be simpler for you to accommodate for the special case
in ima_calc_file_hash() by masking out O_ACCMODE from flags before
adding O_RDONLY.
Thanks,
Amir.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] fs: Evaluate O_WRONLY | O_RDWR to O_RDWR
2018-12-13 12:10 ` Al Viro
2018-12-13 12:16 ` Amir Goldstein
@ 2018-12-13 12:16 ` Dmitry Vyukov
2018-12-13 12:34 ` Al Viro
1 sibling, 1 reply; 6+ messages in thread
From: Dmitry Vyukov @ 2018-12-13 12:16 UTC (permalink / raw)
To: Al Viro
Cc: Goldwyn Rodrigues, linux-fsdevel, Amir Goldstein, Mimi Zohar,
syzbot+ae82084b07d0297e566b, syzkaller-bugs, linux-integrity,
overlayfs
On Thu, Dec 13, 2018 at 1:10 PM Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> On Thu, Dec 13, 2018 at 05:57:17AM -0600, Goldwyn Rodrigues wrote:
> > A user can open(O_WRONLY | O_RDWR) and the options are valid.
> > However, OPEN_FMODE() evaluates both FMODE_READ and FMODE_WRITE,
> > as negative. We also need to protect the lower layers from this
> > anomaly.
> >
> > Solve it by dropping O_WRONLY, so O_RDWR takes precedence.
>
> Congratulations, you've broken fdutils... Passing 3 in lower bits
> of open() flags is *not* the same as O_RDWR; behaiviour is
> different and deliberately chosen by existing userland code.
>
> IOW, NAK.
I am eager to hear what it means then.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] fs: Evaluate O_WRONLY | O_RDWR to O_RDWR
2018-12-13 12:16 ` Dmitry Vyukov
@ 2018-12-13 12:34 ` Al Viro
2018-12-13 14:05 ` Goldwyn Rodrigues
0 siblings, 1 reply; 6+ messages in thread
From: Al Viro @ 2018-12-13 12:34 UTC (permalink / raw)
To: Dmitry Vyukov
Cc: Goldwyn Rodrigues, linux-fsdevel, Amir Goldstein, Mimi Zohar,
syzbot+ae82084b07d0297e566b, syzkaller-bugs, linux-integrity,
overlayfs
On Thu, Dec 13, 2018 at 01:16:45PM +0100, Dmitry Vyukov wrote:
> On Thu, Dec 13, 2018 at 1:10 PM Al Viro <viro@zeniv.linux.org.uk> wrote:
> >
> > On Thu, Dec 13, 2018 at 05:57:17AM -0600, Goldwyn Rodrigues wrote:
> > > A user can open(O_WRONLY | O_RDWR) and the options are valid.
> > > However, OPEN_FMODE() evaluates both FMODE_READ and FMODE_WRITE,
> > > as negative. We also need to protect the lower layers from this
> > > anomaly.
> > >
> > > Solve it by dropping O_WRONLY, so O_RDWR takes precedence.
> >
> > Congratulations, you've broken fdutils... Passing 3 in lower bits
> > of open() flags is *not* the same as O_RDWR; behaiviour is
> > different and deliberately chosen by existing userland code.
> >
> > IOW, NAK.
>
> I am eager to hear what it means then.
Open for ioctls, basically. No read/write allowed, no checks for
media writability, etc. done at open() time, both read and write
*permissions* required from device node.
Think of the things like formatting a floppy, for example...
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] fs: Evaluate O_WRONLY | O_RDWR to O_RDWR
2018-12-13 12:34 ` Al Viro
@ 2018-12-13 14:05 ` Goldwyn Rodrigues
0 siblings, 0 replies; 6+ messages in thread
From: Goldwyn Rodrigues @ 2018-12-13 14:05 UTC (permalink / raw)
To: Al Viro
Cc: Dmitry Vyukov, linux-fsdevel, Amir Goldstein, Mimi Zohar,
syzbot+ae82084b07d0297e566b, syzkaller-bugs, linux-integrity,
overlayfs
On 12:34 13/12, Al Viro wrote:
> On Thu, Dec 13, 2018 at 01:16:45PM +0100, Dmitry Vyukov wrote:
> > On Thu, Dec 13, 2018 at 1:10 PM Al Viro <viro@zeniv.linux.org.uk> wrote:
> > >
> > > On Thu, Dec 13, 2018 at 05:57:17AM -0600, Goldwyn Rodrigues wrote:
> > > > A user can open(O_WRONLY | O_RDWR) and the options are valid.
> > > > However, OPEN_FMODE() evaluates both FMODE_READ and FMODE_WRITE,
> > > > as negative. We also need to protect the lower layers from this
> > > > anomaly.
> > > >
> > > > Solve it by dropping O_WRONLY, so O_RDWR takes precedence.
> > >
> > > Congratulations, you've broken fdutils... Passing 3 in lower bits
> > > of open() flags is *not* the same as O_RDWR; behaiviour is
> > > different and deliberately chosen by existing userland code.
> > >
> > > IOW, NAK.
> >
> > I am eager to hear what it means then.
>
> Open for ioctls, basically. No read/write allowed, no checks for
> media writability, etc. done at open() time, both read and write
> *permissions* required from device node.
>
> Think of the things like formatting a floppy, for example...
Sorry, I was not aware of the ioctl usage. Thanks.
--
Goldwyn
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-12-13 14:05 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-13 11:57 [PATCH] fs: Evaluate O_WRONLY | O_RDWR to O_RDWR Goldwyn Rodrigues
2018-12-13 12:10 ` Al Viro
2018-12-13 12:16 ` Amir Goldstein
2018-12-13 12:16 ` Dmitry Vyukov
2018-12-13 12:34 ` Al Viro
2018-12-13 14:05 ` Goldwyn Rodrigues
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.