Re: [PATCH] fs: don't block i_writecount during exec

[Roberto Sassu <roberto.sassu@huaweicloud.com>]

On Wed, 2024-09-04 at 19:04 +0200, Jann Horn wrote:
> [necrothreading...]
> [+IMA folks]
> 
> On Fri, May 31, 2024 at 3:01 PM Christian Brauner <brauner@kernel.org> wrote:
> > Back in 2021 we already discussed removing deny_write_access() for
> > executables. Back then I was hesistant because I thought that this might
> > cause issues in userspace. But even back then I had started taking some
> > notes on what could potentially depend on this and I didn't come up with
> > a lot so I've changed my mind and I would like to try this.
> [snip]
> > Yes, someone in userspace could potentially be relying on this. It's not
> > completely out of the realm of possibility but let's find out if that's
> > actually the case and not guess.
> 
> FYI, ima_bprm_check() still has a comment that claims that executables
> use deny_write_access():
> 
> /**
>  * ima_bprm_check - based on policy, collect/store measurement.
>  * @bprm: contains the linux_binprm structure
>  *
>  * The OS protects against an executable file, already open for write,
>  * from being executed in deny_write_access() and an executable file,
>  * already open for execute, from being modified in get_write_access().
>  * So we can be certain that what we verify and measure here is actually
>  * what is being executed.
>  *
>  * On success return 0.  On integrity appraisal error, assuming the file
>  * is in policy and IMA-appraisal is in enforcing mode, return -EACCES.
>  */
> 
> But what actually happens in there is not so different from what
> happens in ima_file_mmap(), so I think probably the only change
> required here is to fix up the comment...

We need to do the violation check for the BPRM_CHECK IMA hook too:

	violation_check = ((func == FILE_CHECK || func == MMAP_CHECK
||
			    func == MMAP_CHECK_REQPROT) &&
			   (ima_policy_flag & IMA_MEASURE));

Roberto