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