Linux-Fsdevel Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] fuse:rely on fuse_perm for exec when no mode bits set
@ 2020-04-01  9:31 Chakra Divi
  2020-04-20 11:25 ` Miklos Szeredi
  0 siblings, 1 reply; 7+ messages in thread
From: Chakra Divi @ 2020-04-01  9:31 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: chakragithub, linux-fsdevel, linux-kernel

In current code, for exec we are checking mode bits
for x bit set even though the fuse_perm_getattr returns
success. Changes in this patch avoids mode bit explicit
check, leaves the exec checking to fuse file system
in uspace.

Signed-off-by: Chakra Divi <chakragithub@gmail.com>
---
 fs/fuse/dir.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index de1e2fde..7b44c4e 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -1196,12 +1196,7 @@ static int fuse_permission(struct inode *inode, int mask)
 		err = fuse_access(inode, mask);
 	} else if ((mask & MAY_EXEC) && S_ISREG(inode->i_mode)) {
 		if (!(inode->i_mode & S_IXUGO)) {
-			if (refreshed)
-				return -EACCES;
-
 			err = fuse_perm_getattr(inode, mask);
-			if (!err && !(inode->i_mode & S_IXUGO))
-				return -EACCES;
 		}
 	}
 	return err;
-- 
2.7.4


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

* Re: [PATCH] fuse:rely on fuse_perm for exec when no mode bits set
  2020-04-01  9:31 [PATCH] fuse:rely on fuse_perm for exec when no mode bits set Chakra Divi
@ 2020-04-20 11:25 ` Miklos Szeredi
  2020-04-21 10:51   ` Chakra Divi
  0 siblings, 1 reply; 7+ messages in thread
From: Miklos Szeredi @ 2020-04-20 11:25 UTC (permalink / raw)
  To: Chakra Divi; +Cc: linux-fsdevel, linux-kernel

On Wed, Apr 1, 2020 at 11:31 AM Chakra Divi <chakragithub@gmail.com> wrote:
>
> In current code, for exec we are checking mode bits
> for x bit set even though the fuse_perm_getattr returns
> success. Changes in this patch avoids mode bit explicit
> check, leaves the exec checking to fuse file system
> in uspace.

Why is this needed?

Thanks,
Miklos

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

* Re: [PATCH] fuse:rely on fuse_perm for exec when no mode bits set
  2020-04-20 11:25 ` Miklos Szeredi
@ 2020-04-21 10:51   ` Chakra Divi
  2020-04-27 13:46     ` Chakra Divi
  0 siblings, 1 reply; 7+ messages in thread
From: Chakra Divi @ 2020-04-21 10:51 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: linux-fsdevel, linux-kernel

On Mon, Apr 20, 2020 at 4:55 PM Miklos Szeredi <miklos@szeredi.hu> wrote:
>
> On Wed, Apr 1, 2020 at 11:31 AM Chakra Divi <chakragithub@gmail.com> wrote:
> >
> > In current code, for exec we are checking mode bits
> > for x bit set even though the fuse_perm_getattr returns
> > success. Changes in this patch avoids mode bit explicit
> > check, leaves the exec checking to fuse file system
> > in uspace.
>
> Why is this needed?

Thanks for responding Miklos. We have an use case with our remote file
system mounted on fuse , where permissions checks will happen remotely
without the need of mode bits. In case of read, write it worked
without issues. But for executable files, we found that fuse kernel is
explicitly checking 'x' mode bit set on the file. We want this
checking also to be pushed to remote instead of kernel doing it - so
modified the kernel code to send getattr op to usespace in exec case
too.

> Thanks,
> Miklos

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

* Re: [PATCH] fuse:rely on fuse_perm for exec when no mode bits set
  2020-04-21 10:51   ` Chakra Divi
@ 2020-04-27 13:46     ` Chakra Divi
  2020-04-28  8:21       ` Miklos Szeredi
  0 siblings, 1 reply; 7+ messages in thread
From: Chakra Divi @ 2020-04-27 13:46 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: linux-fsdevel, linux-kernel

On Tue, Apr 21, 2020 at 4:21 PM Chakra Divi <chakragithub@gmail.com> wrote:
>
> On Mon, Apr 20, 2020 at 4:55 PM Miklos Szeredi <miklos@szeredi.hu> wrote:
> >
> > On Wed, Apr 1, 2020 at 11:31 AM Chakra Divi <chakragithub@gmail.com> wrote:
> > >
> > > In current code, for exec we are checking mode bits
> > > for x bit set even though the fuse_perm_getattr returns
> > > success. Changes in this patch avoids mode bit explicit
> > > check, leaves the exec checking to fuse file system
> > > in uspace.
> >
> > Why is this needed?
>
> Thanks for responding Miklos. We have an use case with our remote file
> system mounted on fuse , where permissions checks will happen remotely
> without the need of mode bits. In case of read, write it worked
> without issues. But for executable files, we found that fuse kernel is
> explicitly checking 'x' mode bit set on the file. We want this
> checking also to be pushed to remote instead of kernel doing it - so
> modified the kernel code to send getattr op to usespace in exec case
> too.

Any help on this Miklos....

Thanks,
Chakra
> > Thanks,
> > Miklos

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

* Re: [PATCH] fuse:rely on fuse_perm for exec when no mode bits set
  2020-04-27 13:46     ` Chakra Divi
@ 2020-04-28  8:21       ` Miklos Szeredi
  2020-05-08 10:13         ` Chakra Divi
  0 siblings, 1 reply; 7+ messages in thread
From: Miklos Szeredi @ 2020-04-28  8:21 UTC (permalink / raw)
  To: Chakra Divi; +Cc: linux-fsdevel, linux-kernel

On Mon, Apr 27, 2020 at 3:46 PM Chakra Divi <chakragithub@gmail.com> wrote:
>
> On Tue, Apr 21, 2020 at 4:21 PM Chakra Divi <chakragithub@gmail.com> wrote:
> >
> > On Mon, Apr 20, 2020 at 4:55 PM Miklos Szeredi <miklos@szeredi.hu> wrote:
> > >
> > > On Wed, Apr 1, 2020 at 11:31 AM Chakra Divi <chakragithub@gmail.com> wrote:
> > > >
> > > > In current code, for exec we are checking mode bits
> > > > for x bit set even though the fuse_perm_getattr returns
> > > > success. Changes in this patch avoids mode bit explicit
> > > > check, leaves the exec checking to fuse file system
> > > > in uspace.
> > >
> > > Why is this needed?
> >
> > Thanks for responding Miklos. We have an use case with our remote file
> > system mounted on fuse , where permissions checks will happen remotely
> > without the need of mode bits. In case of read, write it worked
> > without issues. But for executable files, we found that fuse kernel is
> > explicitly checking 'x' mode bit set on the file. We want this
> > checking also to be pushed to remote instead of kernel doing it - so
> > modified the kernel code to send getattr op to usespace in exec case
> > too.
>
> Any help on this Miklos....

I still don't understand what you are requesting.  What your patch
does is unconditionally allow execution, even without any 'x' bits in
the mode.  What does that achieve?

Thanks,
Miklos

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

* Re: [PATCH] fuse:rely on fuse_perm for exec when no mode bits set
  2020-04-28  8:21       ` Miklos Szeredi
@ 2020-05-08 10:13         ` Chakra Divi
  2020-05-11 12:25           ` Miklos Szeredi
  0 siblings, 1 reply; 7+ messages in thread
From: Chakra Divi @ 2020-05-08 10:13 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: linux-fsdevel, linux-kernel

On Tue, Apr 28, 2020 at 1:51 PM Miklos Szeredi <miklos@szeredi.hu> wrote:
>
> On Mon, Apr 27, 2020 at 3:46 PM Chakra Divi <chakragithub@gmail.com> wrote:
> >
> > On Tue, Apr 21, 2020 at 4:21 PM Chakra Divi <chakragithub@gmail.com> wrote:
> > >
> > > On Mon, Apr 20, 2020 at 4:55 PM Miklos Szeredi <miklos@szeredi.hu> wrote:
> > > >
> > > > On Wed, Apr 1, 2020 at 11:31 AM Chakra Divi <chakragithub@gmail.com> wrote:
> > > > >
> > > > > In current code, for exec we are checking mode bits
> > > > > for x bit set even though the fuse_perm_getattr returns
> > > > > success. Changes in this patch avoids mode bit explicit
> > > > > check, leaves the exec checking to fuse file system
> > > > > in uspace.
> > > >
> > > > Why is this needed?
> > >
> > > Thanks for responding Miklos. We have an use case with our remote file
> > > system mounted on fuse , where permissions checks will happen remotely
> > > without the need of mode bits. In case of read, write it worked
> > > without issues. But for executable files, we found that fuse kernel is
> > > explicitly checking 'x' mode bit set on the file. We want this
> > > checking also to be pushed to remote instead of kernel doing it - so
> > > modified the kernel code to send getattr op to usespace in exec case
> > > too.
> >
> > Any help on this Miklos....
>
> I still don't understand what you are requesting.  What your patch
> does is unconditionally allow execution, even without any 'x' bits in
> the mode.  What does that achieve?

Thanks for the help Miklos. We have a network based filesystem that
supports acls.
As our filesystem give granular access, we wipe out the mode bits and
completely rely on ACLs.

Fuse works well for all other ops (with default_permissions disabled )
 as all the checks are done at the filesystems.
But only executables have problems because fuse kernel rejects the
execution by doing access checks on mode bit.
To push this check to filesystem, in the above patch - i'm relying on
return value from fuse_perm_getattr() ignoring the mode bits.

When the fuse module is asked to rely on filesystem for access checks,
why do we need this explicit check for executables?
I found out that it is the same issue with nfs too. Is there a reason
for it ? Should we not push this check to filesystem ?

Thanks,
Chakra
>
> Thanks,
> Miklos

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

* Re: [PATCH] fuse:rely on fuse_perm for exec when no mode bits set
  2020-05-08 10:13         ` Chakra Divi
@ 2020-05-11 12:25           ` Miklos Szeredi
  0 siblings, 0 replies; 7+ messages in thread
From: Miklos Szeredi @ 2020-05-11 12:25 UTC (permalink / raw)
  To: Chakra Divi; +Cc: linux-fsdevel, linux-kernel

On Fri, May 8, 2020 at 12:14 PM Chakra Divi <chakragithub@gmail.com> wrote:
>
> On Tue, Apr 28, 2020 at 1:51 PM Miklos Szeredi <miklos@szeredi.hu> wrote:
> >
> > On Mon, Apr 27, 2020 at 3:46 PM Chakra Divi <chakragithub@gmail.com> wrote:
> > >
> > > On Tue, Apr 21, 2020 at 4:21 PM Chakra Divi <chakragithub@gmail.com> wrote:
> > > >
> > > > On Mon, Apr 20, 2020 at 4:55 PM Miklos Szeredi <miklos@szeredi.hu> wrote:
> > > > >
> > > > > On Wed, Apr 1, 2020 at 11:31 AM Chakra Divi <chakragithub@gmail.com> wrote:
> > > > > >
> > > > > > In current code, for exec we are checking mode bits
> > > > > > for x bit set even though the fuse_perm_getattr returns
> > > > > > success. Changes in this patch avoids mode bit explicit
> > > > > > check, leaves the exec checking to fuse file system
> > > > > > in uspace.
> > > > >
> > > > > Why is this needed?
> > > >
> > > > Thanks for responding Miklos. We have an use case with our remote file
> > > > system mounted on fuse , where permissions checks will happen remotely
> > > > without the need of mode bits. In case of read, write it worked
> > > > without issues. But for executable files, we found that fuse kernel is
> > > > explicitly checking 'x' mode bit set on the file. We want this
> > > > checking also to be pushed to remote instead of kernel doing it - so
> > > > modified the kernel code to send getattr op to usespace in exec case
> > > > too.
> > >
> > > Any help on this Miklos....
> >
> > I still don't understand what you are requesting.  What your patch
> > does is unconditionally allow execution, even without any 'x' bits in
> > the mode.  What does that achieve?
>
> Thanks for the help Miklos. We have a network based filesystem that
> supports acls.
> As our filesystem give granular access, we wipe out the mode bits and
> completely rely on ACLs.

Are you using POSIX ACLs?   Why can't you translate the ACL's back
into mode bits (that's what all filesystems do)?

>
> Fuse works well for all other ops (with default_permissions disabled )
>  as all the checks are done at the filesystems.
> But only executables have problems because fuse kernel rejects the
> execution by doing access checks on mode bit.
> To push this check to filesystem, in the above patch - i'm relying on
> return value from fuse_perm_getattr() ignoring the mode bits.
>
> When the fuse module is asked to rely on filesystem for access checks,
> why do we need this explicit check for executables?

Because there's no other check.  Have you noticed that with your patch
*all* files become executable?  I guess that's not what you wanted...

Thanks,
Miklos

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

end of thread, other threads:[~2020-05-11 12:26 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-01  9:31 [PATCH] fuse:rely on fuse_perm for exec when no mode bits set Chakra Divi
2020-04-20 11:25 ` Miklos Szeredi
2020-04-21 10:51   ` Chakra Divi
2020-04-27 13:46     ` Chakra Divi
2020-04-28  8:21       ` Miklos Szeredi
2020-05-08 10:13         ` Chakra Divi
2020-05-11 12:25           ` 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).