[linux-cifs-client] Re: [PATCH 00/25] move handling of setuid/gid bits from VFS into individual setattr functions (RESEND)

[Andrew Morton]

On Wed, 8 Aug 2007 08:54:35 -0400 Jeff Layton <jlayton at redhat.com> wrote:

> On Tue, 7 Aug 2007 17:15:01 -0700
> Andrew Morton <akpm at linux-foundation.org> wrote:
> 
> > On Mon, 6 Aug 2007 09:54:03 -0400
> > Jeff Layton <jlayton at redhat.com> wrote:
> > 
> > Is there any way in which we can prevent these problems?  Say
> > 
> > - rename something so that unconverted filesystems will reliably fail to
> >   compile?
> > 
> 
> I suppose we could rename the .setattr inode operation to something
> else, but then we'll be stuck with it for at least a while. That seems
> sort of kludgey too...

Sure.  We're changing the required behaviour of .setattr.  Changing its
name is a fine and reasonably reliable way to communicate that fact.

> > - leave existing filesystems alone, but add a new
> >   inode_operations.setattr_jeff, which the networked filesytems can
> >   implement, and teach core vfs to call setattr_jeff in preference to
> >   setattr?
> > 
> > Something else?
> 
> There's also the approach suggested by Miklos: Add a new inode flag that
> tells notify_change not to convert ATTR_KILL_S* flags into a mode
> change. Basically, allow filesystems to "opt out" of that behavior. 
> 
> I'd definitly pick that over a new inode op. That would also allow the
> default case be for the VFS to continue handling these flags.
> Everything would continue to work but filesystems that need to handle
> these flags differently would be able to do so.
> 

We should opt for whatever produces the best end state in the kernel tree. 
ie: if it takes more work and a larger patch to create a better result,
let's go for the better result.  We merge large patches all the time.  We
prefer to smash through, get it right whatever the transient cost.  But
quietly making out-of-tree filesystems less secure is a pretty high cost.

I'm suspecting that adding more flags and some code to test them purely to
minimise the size of the patch and to retain compatibility with the old
.setattr is not a good tradeoff, given that we'd carry the flags and tests
for evermore.

So I'd suggest s/setattr/something_else/g.