| Message ID | 20200615121257.798894-6-hch@lst.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [01/13] cachefiles: switch to kernel_write | expand |
On Mon, Jun 15, 2020 at 02:12:49PM +0200, Christoph Hellwig wrote: > We still need to check if the fѕ is open write, even for the low-level > helper. Do we need the analogous check for FMODE_READ in the __kernel_read() patch? > @@ -505,6 +505,8 @@ ssize_t __kernel_write(struct file *file, const void *buf, size_t count, loff_t > const char __user *p; > ssize_t ret; > > + if (!(file->f_mode & FMODE_WRITE)) > + return -EBADF; > if (!(file->f_mode & FMODE_CAN_WRITE)) > return -EINVAL; > > -- > 2.26.2 >
On Mon, Jun 15, 2020 at 05:34:39AM -0700, Matthew Wilcox wrote: > On Mon, Jun 15, 2020 at 02:12:49PM +0200, Christoph Hellwig wrote: > > We still need to check if the fѕ is open write, even for the low-level > > helper. > > Do we need the analogous check for FMODE_READ in the __kernel_read() > patch? Yes, we should probably grow it. Hoping that none of the caller actually wants to mess with files not open for reading as some of them are rather dodgy.
On Mon, Jun 15, 2020 at 5:13 AM Christoph Hellwig <hch@lst.de> wrote: > > We still need to check if the fѕ is open write, even for the low-level > helper. Is there actually a way to trigger something like this? I'm wondering if it's worth a WARN_ON_ONCE()? It doesn't sound sensible to have some kernel functionality try to write to a file it didn't open for write, and sounds like a kernel bug if this case were to ever trigger.. Linus
On Mon, Jun 15, 2020 at 09:39:31AM -0700, Linus Torvalds wrote: > On Mon, Jun 15, 2020 at 5:13 AM Christoph Hellwig <hch@lst.de> wrote: > > > > We still need to check if the fѕ is open write, even for the low-level > > helper. > > Is there actually a way to trigger something like this? I'm wondering > if it's worth a WARN_ON_ONCE()? > > It doesn't sound sensible to have some kernel functionality try to > write to a file it didn't open for write, and sounds like a kernel bug > if this case were to ever trigger.. Yes, this would be bug in the calling code.
From: Linus Torvalds > Sent: 15 June 2020 17:40 > On Mon, Jun 15, 2020 at 5:13 AM Christoph Hellwig <hch@lst.de> wrote: > > > > We still need to check if the fѕ is open write, even for the low-level > > helper. > > Is there actually a way to trigger something like this? I'm wondering > if it's worth a WARN_ON_ONCE()? > > It doesn't sound sensible to have some kernel functionality try to > write to a file it didn't open for write, and sounds like a kernel bug > if this case were to ever trigger.. It's a cheap test at the top of some fairly heavy code. Failing the request will soon identify the bug. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
diff --git a/fs/read_write.c b/fs/read_write.c index 2c601d853ff3d8..76be155ad98242 100644 --- a/fs/read_write.c +++ b/fs/read_write.c @@ -505,6 +505,8 @@ ssize_t __kernel_write(struct file *file, const void *buf, size_t count, loff_t const char __user *p; ssize_t ret; + if (!(file->f_mode & FMODE_WRITE)) + return -EBADF; if (!(file->f_mode & FMODE_CAN_WRITE)) return -EINVAL;
We still need to check if the fѕ is open write, even for the low-level helper. Signed-off-by: Christoph Hellwig <hch@lst.de> --- fs/read_write.c | 2 ++ 1 file changed, 2 insertions(+)