Re: [PATCH v2 0/8] introduce dedicated type for idmapped mounts

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Jun 21, 2022 at 04:14:46PM +0200, Christian Brauner wrote:
> From: "Christian Brauner (Microsoft)" <brauner@xxxxxxxxxx>
> 
> Hey everyone,
> 
> /* v2 */
> No major changes. The type got renamed since we agreed that the initial
> name wasn't great. There are some typo fixes in the commit messages and
> a few tweaks to the last commit and added Jan's rvb.
> 
> This series starts to introduce a new vfs{g,u}id_t type. It allows to
> distinguish {g,u}ids on idmapped mounts from filesystem k{g,u}ids.
> 
> We leverage the type framework to increase the safety for filesystems
> and the vfs when dealing with idmapped mounts.
> 
> The series introduces the type and converts the setattr codepaths to
> use the new type and associated helpers.
> 
> Currently these codepaths place the value that will ultimately be
> written to inode->i_{g,u}id into attr->ia_{g,u}id which allows to avoid
> changing a few callsites. But there are drawbacks to this approach.
> 
> As Linus rightly points out it makes some of the permission checks in
> the attribute code harder to understand than they need and should be and
> increases the probability for further issues.
> 
> This series makes it so that the values will always be treated as being
> mapped into the idmapped mount. Only when the filesystem object is
> actually updated will the value be mapped into the filesystem idmapping.
> 
> I first looked into this about ~7 months ago but put it on hold to focus
> on the testsuite. Linus expressed the desire for something like this
> last week so I got back to working on this.
> 
> I'd like to get at least this first series in for v5.20. The conversion
> can the continue until we can remove all the regular non-type safe
> helpers and will only be left with the type safe helpers.
> 
> Thanks!
> Christian

As I mentioned in my other responses I prefer to see comparisons with
invalid ids always evaluate as not equal. You can take or leave that
suggestion, but even without it this looks correct, and I think a
separate type is a good change to avoid confusion. For all the patches,
feel free to add:

Reviewed-by: Seth Forshee <sforshee@xxxxxxxxxxxxxxxx>

Seth



[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [NTFS 3]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [NTFS 3]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux