lkml.org 
[lkml]   [2016]   [Dec]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: FUSE: regression when clearing setuid bits on chown
From
Date
On Tue, 2016-12-06 at 15:51 +0100, Miklos Szeredi wrote:
> On Tue, Dec 6, 2016 at 3:45 PM, Jeff Layton <jlayton@redhat.com> wrote:
>
> >
> > >
> > > @@ -1739,8 +1739,6 @@ static int fuse_setattr(struct dentry *e
> > > * This should be done on write(), truncate() and chown().
> > > */
> > > if (!fc->handle_killpriv) {
> >
> > One more thing too. I don't think we really want to monkey with the mode
> > at all if there is a request to set the mode already in the request. So
> > maybe this should be:
> >
> > if (!fc->handle_killpriv && !(attr->ia_mode & ATTR_MODE))
> >
> > Granted that won't generally happen from normal process context, but we
> > could have knfsd in here too and I think that's possible from there.
>
> Apparently this can't happen even from knfsd; notify_change() has this comment:
>
> /*
> * We now pass ATTR_KILL_S*ID to the lower level setattr function so
> * that the function has the ability to reinterpret a mode change
> * that's due to these bits. This adds an implicit restriction that
> * no function will ever call notify_change with both ATTR_MODE and
> * ATTR_KILL_S*ID set.
> */
> if ((ia_valid & (ATTR_KILL_SUID|ATTR_KILL_SGID)) &&
> (ia_valid & ATTR_MODE))
> BUG();
>
>

Ahh right, I had forgotten about that. Eventually we may want to lift
that restriction, but you can add this to the current patch:

Reviewed-by: Jeff Layton <jlayton@redhat.com>

Thanks for fixing it quickly!

\
 
 \ /
  Last update: 2016-12-06 15:55    [W:0.990 / U:1.756 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site