Re: [PATCH][RFC] fs: add levels to inode write access

[Christian Brauner <brauner@kernel.org>]

On Fri, May 31, 2024 at 12:02:16PM +0200, Christian Brauner wrote:
> On Thu, May 30, 2024 at 08:49:12AM -0700, Linus Torvalds wrote:
> > On Thu, 30 May 2024 at 03:32, Christian Brauner <brauner@kernel.org> wrote:
> > >
> > > Ofc depends on whether Linus still agrees that removing this might be
> > > something we could try.
> > 
> > I _definitely_ do not want to see any more complex deny_write_access().
> > 
> > So yes, if people have good reasons to override the inode write
> > access, I'd rather remove it entirely than make it some eldritch
> > horror that is even worse than what we have now.
> > 
> > It would obviously have to be tested in case some odd case actually
> > depends on the ETXTBSY semantics, since we *have* supported it for a
> > long time.  But iirc nobody even noticed when we removed it from
> > shared libraries, so...
> > 
> > That said, verity seems to depend on it as a way to do the
> > "enable_verity()" atomically with no concurrent writes, and I see some
> > i_writecount noise in the integrity code too.
> > 
> > But maybe that's just a belt-and-suspenders thing?
> > 
> > Because if execve() no longer does it, I think we should just remove
> > that i_writecount thing entirely.
> 
> deny_write_access() is being used from kernel_read_file() which has a
> few wrappers around it and they are used in various places:
> 
> (1) kernel_read_file() based helpers:
>   (1.1) kernel_read_file_from_path()
>   (1.2) kernel_read_file_from_path_initns()
>   (1.3) kernel_read_file_from_fd()
> 
> (2) kernel_read_file() users:
>     (2.1) kernel/module/main.c:init_module_from_file()
>     (2.2) security/loadpin/loadpin.c:read_trusted_verity_root_digests()
> 
> (3) kernel_read_file_from_path() users:
>     (3.1) security/integrity/digsig.c:integrity_load_x509()
>     (3.2) security/integrity/ima/ima_fs.c:ima_read_busy()
> 
> (4) kernel_read_file_from_path_initns() users:
>     (4.1) drivers/base/firmware_loader/main.c:fw_get_filesystem_firmware()
> 
> (5) kernel_read_file_from_fd() users:
>     (5.1) kernel/kexec_file.c:kimage_file_prepare_segments()
> 
> In order to remove i_writecount completely we would need to do this in

Sorry, typo s/i_write_count/deny_write_access()/g
(I don't think we can remove i_writecount itself as it's used for file
leases and locks.)

> multiple steps as some of that stuff seems potentially sensitive.
> 
> The exec deny write mechanism can be removed because we have a decent
> understanding of the implications and there's decent justification for
> removing it.
> 
> So I propose that I do various testing (LTP) etc. now, send the patch
> and then put this into -next to see if anything breaks?