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@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 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?
