security_file_free contract/expectations

[Roberto Sassu]

On Wed, 2024-01-24 at 17:14 -0800, Casey Schaufler wrote:
> On 1/24/2024 3:46 PM, Ben Smith wrote:
> > Hi, I'm looking at a kernel panic and I'm trying to figure out whether the code in question is doing something that breaks the contract for the security_file_free hook. I'm specifically wondering whether it's expected/safe for code called from security_file_free to open and read from a file. In the case I'm looking at what happens is:
> 
> The only action that should take place in a _file_free() hook is to manage
> resources used by the LSM. That could involve a kfree() of the module's
> blob, or some other memory management operation.  Trying to access a file
> from this hook is a bad idea.

I'm introducing a new LSM called file_release, that can be suitable for
reading the file (actually IMA will use this functionality).

https://lore.kernel.org/linux-integrity/20240115181809.885385-14-roberto.sassu@huaweicloud.com/

Roberto

> > 
> > - Process exit
> > - exit_fs() sets current->fs to NULL
> > - exit_task_work() calls __fput() on files which were closed in exit_files()
> > - __fput() calls security_file_free()
> > - security_file_free() then reads the file that was just closed in order to gather information about it.
> > - a filesystem driver (I've seen this with two out-of-tree filesystems) then accesses current->fs and panics.
> > 
> > So I'm wondering if the expectation here is that filesystem code should NULL check current->fs before using it or that an LSM shouldn't try to read a file from security_file_free().
> > 
> >