Re: [PATCH 0/4] fs: add mount_setattr()

From: Christian Brauner
Date: Sun Jul 19 2020 - 13:55:22 EST


On Sun, Jul 19, 2020 at 06:10:54PM +0100, Al Viro wrote:
> On Tue, Jul 14, 2020 at 06:14:11PM +0200, Christian Brauner wrote:
>
> > mount_setattr() can be expected to grow over time and is designed with
> > extensibility in mind. It follows the extensible syscall pattern we have
> > used with other syscalls such as openat2(), clone3(),
> > sched_{set,get}attr(), and others.
>
> I.e. it's a likely crap insertion vector; any patches around that thing
> will require the same level of review as addition of a brand new syscall.

Which is just how we should and hopefully treat any meaningful
extension, yes. Otherwise let's just never add a flag argument to any
syscall and only have dup()- and accept()-like syscalls.

> And they will be harder to spot - consider the likely subjects for such
> patches and compare to open addition of a new syscall...

In the new revision I have dropped the atime argument because David
already plumbed setting atime into fsmount() via flags and making
userspace jump through more hoops by adding more constants seems
pointless. So the additional arguments can be moved up because we're
below the 6 syscall args limit.

Though I really want to stress that while I see the worry it is less a
technial argument but one for our review process where we should treat
extensions to syscalls as strict as syscall additions. Which yes, very
much so.

Thanks!
Christian