security_file_free contract/expectations

[Tetsuo Handa]

On 2024/01/25 8:46, 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:
> 
> - 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().

I guess the out-of-tree filesystems are used by CrowdStrike products and
therefore the source code etc. cannot be shared. (It took me 2 years to prove
that an out-of-tree filesystem used by TrendMicro products was the culprit.)
But are you sure that the location that triggered panic is at reading current->fs ?

security_file_free() is not meant for reading files. But if the location were
really at reading current->fs, whether an LSM shouldn't try to read a file from
security_file_free() is a bogus question.

security_file_release() proposed by Roberto Sassu would be OK. But I guess that
the kernel version you want to load the out-of-tree filesystems is not the latest...

Without more details, we can't give you appropriate suggestions.