Messages in this thread | | | Subject | Re: [V9fs-developer] [PATCH kernel] 9p/trans_fd: Check file mode at opening | From | Alexey Kardashevskiy <> | Date | Wed, 29 Jul 2020 09:50:21 +1000 |
| |
On 29/07/2020 03:42, Greg Kurz wrote: > Hi Alexey, > > Working on 9p now ?!? ;-)
No, I am running syzkaller and seeing things :)
> Cc'ing Dominique Martinet who appears to be the person who takes care of 9p > these days. > > On Tue, 28 Jul 2020 22:41:29 +1000 > Alexey Kardashevskiy <aik@ozlabs.ru> wrote: > >> The "fd" transport layer uses 2 file descriptors passed externally >> and calls kernel_write()/kernel_read() on these. If files were opened >> without FMODE_WRITE/FMODE_READ, WARN_ON_ONCE() will fire. >> >> This adds file mode checking in p9_fd_open; this returns -EBADF to >> preserve the original behavior. >> > > So this would cause open() to fail with EBADF, which might look a bit > weird to userspace since it didn't pass an fd... Is this to have a > different error than -EIO that is returned when either rfd or wfd > doesn't point to an open file descriptor ?
This is only to preserve the existing behavior.
> If yes, why do we care ?
Without the patch, p9_fd_open() produces a kernel warning which is not great by itself and becomes crash with panic_on_warn.
> >> Found by syzkaller. >> >> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> >> --- >> net/9p/trans_fd.c | 7 ++++++- >> 1 file changed, 6 insertions(+), 1 deletion(-) >> >> diff --git a/net/9p/trans_fd.c b/net/9p/trans_fd.c >> index 13cd683a658a..62cdfbd01f0a 100644 >> --- a/net/9p/trans_fd.c >> +++ b/net/9p/trans_fd.c >> @@ -797,6 +797,7 @@ static int parse_opts(char *params, struct p9_fd_opts *opts) >> >> static int p9_fd_open(struct p9_client *client, int rfd, int wfd) >> { >> + bool perm; >> struct p9_trans_fd *ts = kzalloc(sizeof(struct p9_trans_fd), >> GFP_KERNEL); >> if (!ts) >> @@ -804,12 +805,16 @@ static int p9_fd_open(struct p9_client *client, int rfd, int wfd) >> >> ts->rd = fget(rfd); >> ts->wr = fget(wfd); >> - if (!ts->rd || !ts->wr) { >> + perm = ts->rd && (ts->rd->f_mode & FMODE_READ) && >> + ts->wr && (ts->wr->f_mode & FMODE_WRITE); >> + if (!ts->rd || !ts->wr || !perm) { >> if (ts->rd) >> fput(ts->rd); >> if (ts->wr) >> fput(ts->wr); >> kfree(ts); >> + if (!perm) >> + return -EBADF; >> return -EIO; >> } >> >
-- Alexey
| |