On Thu, May 30, 2024 at 08:49:12AM -0700, Linus Torvalds wrote:
> On Thu, 30 May 2024 at 03:32, Christian Brauner <brauner@xxxxxxxxxx> 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
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?
