On Sun, Jan 21, 2024 at 06:50:57PM +0100, Christian Brauner wrote: > > - We have a small offlist discussion with Christian about adding fs_type->allow_idmap > > hook. Christian pointed out that it would be nice to have a superblock flag instead like > > SB_I_NOIDMAP and we can set this flag during mount time if we see that the filesystem does not > > support idmappings. But, unfortunately, I didn't succeed here because the kernel will > > know if the filesystem supports idmapping or not after FUSE_INIT request, but FUSE_INIT request > > is being sent at the end of the mounting process, so the mount and superblock will exist and > > visible by the userspace in that time. It seems like setting SB_I_NOIDMAP flag, in this > > case, is too late as a user may do the trick by creating an idmapped mount while it wasn't > > restricted by SB_I_NOIDMAP. Alternatively, we can introduce a "positive" version SB_I_ALLOWIDMAP > > I see. > > > and a "weak" version of FS_ALLOW_IDMAP like FS_MAY_ALLOW_IDMAP. So if FS_MAY_ALLOW_IDMAP is set, > > then SB_I_ALLOWIDMAP has to be set on the superblock to allow the creation of an idmapped mount. > > But that's a matter of our discussion. > > I dislike making adding a struct super_block method. Because it means that we > call into the filesystem from generic mount code and specifically with the > namespace semaphore held. If there's ever any network filesystem that e.g., > calls to a hung server it will lockup the whole system. So I'm opposed to > calling into the filesystem here at all. It's also ugly because this is really > a vfs level change. The only involvement should be whether the filesystem type > can actually support this ideally. > > I think we should handle this within FUSE. So we allow the creation of idmapped > mounts just based on FS_ALLOW_IDMAP. And if the server doesn't support the > FUSE_OWNER_UID_GID_EXT then we simply refuse all creation requests originating > from an idmapped mount. Either we return EOPNOSUPP or we return EOVERFLOW to > indicate that we can't represent the owner correctly because the server is > missing the required extension. Could fuse just set SB_I_NOIDMAP initially then clear it if the init reply indicates idmap support? This is like the "weak" FS_ALLOW_IDMAP option without requiring another file_system_type flag.
