On Tue, Dec 6, 2016 at 3:45 PM, Jeff Layton <jlayton@xxxxxxxxxx> 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(); Thanks, Miklos -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html