Linux-Fsdevel Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] fuse: Fix parameter for FS_IOC_{GET,SET}FLAGS
@ 2020-07-14 10:26 Chirantan Ekbote
  2020-07-15 12:20 ` Miklos Szeredi
  0 siblings, 1 reply; 4+ messages in thread
From: Chirantan Ekbote @ 2020-07-14 10:26 UTC (permalink / raw)
  To: Miklos Szeredi
  Cc: linux-fsdevel, Dylan Reid, Suleiman Souhlal, fuse-devel,
	Chirantan Ekbote

The ioctl encoding for this parameter is a long but the documentation
says it should be an int and the kernel drivers expect it to be an int.
If the fuse driver treats this as a long it might end up scribbling over
the stack of a userspace process that only allocated enough space for an
int.

This was previously discussed in [1] and a patch for fuse was proposed
in [2].  From what I can tell the patch in [2] was nacked in favor of
adding new, "fixed" ioctls and using those from userspace.  However
there is still no "fixed" version of these ioctls and the fact is that
it's sometimes infeasible to change all userspace to use the new one.

Handling the ioctls specially in the fuse driver seems like the most
pragmatic way for fuse servers to support them without causing crashes
in userspace applications that call them.

[1]: https://lore.kernel.org/linux-fsdevel/20131126200559.GH20559@hall.aurel32.net/T/
[2]: https://sourceforge.net/p/fuse/mailman/message/31771759/

Signed-off-by: Chirantan Ekbote <chirantan@chromium.org>
---
 fs/fuse/file.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index 66214707a9456..fc0a568ee28c8 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -18,6 +18,7 @@
 #include <linux/swap.h>
 #include <linux/falloc.h>
 #include <linux/uio.h>
+#include <linux/fs.h>
 
 static struct page **fuse_pages_alloc(unsigned int npages, gfp_t flags,
 				      struct fuse_page_desc **desc)
@@ -2760,7 +2761,16 @@ long fuse_do_ioctl(struct file *file, unsigned int cmd, unsigned long arg,
 		struct iovec *iov = iov_page;
 
 		iov->iov_base = (void __user *)arg;
-		iov->iov_len = _IOC_SIZE(cmd);
+
+		switch (cmd) {
+		case FS_IOC_GETFLAGS:
+		case FS_IOC_SETFLAGS:
+			iov->iov_len = sizeof(int);
+			break;
+		default:
+			iov->iov_len = _IOC_SIZE(cmd);
+			break;
+		}
 
 		if (_IOC_DIR(cmd) & _IOC_WRITE) {
 			in_iov = iov;
-- 
2.27.0.389.gc38d7665816-goog


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] fuse: Fix parameter for FS_IOC_{GET,SET}FLAGS
  2020-07-14 10:26 [PATCH] fuse: Fix parameter for FS_IOC_{GET,SET}FLAGS Chirantan Ekbote
@ 2020-07-15 12:20 ` Miklos Szeredi
  2020-07-15 23:06   ` Sedat Dilek
  0 siblings, 1 reply; 4+ messages in thread
From: Miklos Szeredi @ 2020-07-15 12:20 UTC (permalink / raw)
  To: Chirantan Ekbote; +Cc: linux-fsdevel, Dylan Reid, Suleiman Souhlal, fuse-devel

On Tue, Jul 14, 2020 at 12:26 PM Chirantan Ekbote
<chirantan@chromium.org> wrote:
>
> The ioctl encoding for this parameter is a long but the documentation
> says it should be an int and the kernel drivers expect it to be an int.
> If the fuse driver treats this as a long it might end up scribbling over
> the stack of a userspace process that only allocated enough space for an
> int.
>
> This was previously discussed in [1] and a patch for fuse was proposed
> in [2].  From what I can tell the patch in [2] was nacked in favor of
> adding new, "fixed" ioctls and using those from userspace.  However
> there is still no "fixed" version of these ioctls and the fact is that
> it's sometimes infeasible to change all userspace to use the new one.

Okay, applied.

Funny that no one came back with this issue for 7 years.

Thanks,
Miklos

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] fuse: Fix parameter for FS_IOC_{GET,SET}FLAGS
  2020-07-15 12:20 ` Miklos Szeredi
@ 2020-07-15 23:06   ` Sedat Dilek
  2020-07-16  5:17     ` Miklos Szeredi
  0 siblings, 1 reply; 4+ messages in thread
From: Sedat Dilek @ 2020-07-15 23:06 UTC (permalink / raw)
  To: Miklos Szeredi
  Cc: Chirantan Ekbote, linux-fsdevel, Dylan Reid, Suleiman Souhlal,
	fuse-devel

On Wed, Jul 15, 2020 at 5:05 PM Miklos Szeredi <miklos@szeredi.hu> wrote:
>
> On Tue, Jul 14, 2020 at 12:26 PM Chirantan Ekbote
> <chirantan@chromium.org> wrote:
> >
> > The ioctl encoding for this parameter is a long but the documentation
> > says it should be an int and the kernel drivers expect it to be an int.
> > If the fuse driver treats this as a long it might end up scribbling over
> > the stack of a userspace process that only allocated enough space for an
> > int.
> >
> > This was previously discussed in [1] and a patch for fuse was proposed
> > in [2].  From what I can tell the patch in [2] was nacked in favor of
> > adding new, "fixed" ioctls and using those from userspace.  However
> > there is still no "fixed" version of these ioctls and the fact is that
> > it's sometimes infeasible to change all userspace to use the new one.
>
> Okay, applied.
>

...and pushed? I do not see in in fuse.git.

- Sedat -

> Funny that no one came back with this issue for 7 years.
>
> Thanks,
> Miklos

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] fuse: Fix parameter for FS_IOC_{GET,SET}FLAGS
  2020-07-15 23:06   ` Sedat Dilek
@ 2020-07-16  5:17     ` Miklos Szeredi
  0 siblings, 0 replies; 4+ messages in thread
From: Miklos Szeredi @ 2020-07-16  5:17 UTC (permalink / raw)
  To: Sedat Dilek
  Cc: Chirantan Ekbote, linux-fsdevel, Dylan Reid, Suleiman Souhlal,
	fuse-devel

On Thu, Jul 16, 2020 at 1:06 AM Sedat Dilek <sedat.dilek@gmail.com> wrote:
>
> On Wed, Jul 15, 2020 at 5:05 PM Miklos Szeredi <miklos@szeredi.hu> wrote:
> >
> > On Tue, Jul 14, 2020 at 12:26 PM Chirantan Ekbote
> > <chirantan@chromium.org> wrote:
> > >
> > > The ioctl encoding for this parameter is a long but the documentation
> > > says it should be an int and the kernel drivers expect it to be an int.
> > > If the fuse driver treats this as a long it might end up scribbling over
> > > the stack of a userspace process that only allocated enough space for an
> > > int.
> > >
> > > This was previously discussed in [1] and a patch for fuse was proposed
> > > in [2].  From what I can tell the patch in [2] was nacked in favor of
> > > adding new, "fixed" ioctls and using those from userspace.  However
> > > there is still no "fixed" version of these ioctls and the fact is that
> > > it's sometimes infeasible to change all userspace to use the new one.
> >
> > Okay, applied.
> >
>
> ...and pushed? I do not see in in fuse.git.

Pushed now.

Thanks,
Miklos

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2020-07-16  5:17 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-14 10:26 [PATCH] fuse: Fix parameter for FS_IOC_{GET,SET}FLAGS Chirantan Ekbote
2020-07-15 12:20 ` Miklos Szeredi
2020-07-15 23:06   ` Sedat Dilek
2020-07-16  5:17     ` Miklos Szeredi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).