Re: [PATCH 2/2] security: Fix call security_backing_file_free second time

Paul Moore <[email protected]>
Newsgroups org.kernel.vger.linux-security-module,org.kernel.vger.linux-kernel,org.kernel.vger.selinux
Message-ID <[email protected]>
On Jun 25, 2026 Cai Xinchen <[email protected]> wrote:
> 
> I found the following path:
> 
> alloc_empty_backing-file
>     init_file(&ff->file, xxx)
>         -> file_ref_init(&f->f_ref, 1); // only 1
>     error = init_backing_file
>         -> security_backing_file_alloc
>         -> rc = call_int_hook(backing_file_alloc, ...)

The good news is that as you mentioned, only SELinux defines a
backing_file_alloc hook and it always returns success/0.

>            if (unlikely(rc))
>            	security_backing_file_free(backing_file); // first call
>     if (unlikely(error)) {
>         fput(&ff->file);
>          -> if (unlikely(file_ref_put(&file->f_ref))) // zero
>                 __fput_deferred(file);
>                  -> ____fput -> __fput -> file_free(file);
>                  -> backing_file_free(backing_file(f));
>                  -> security_backing_file_free(&ff->file); // second call
> 
> Currently, only SELinux has the lsm backing_file_alloc hook, and the
> backing_file_free hook is not set. When security_backing_file_free is
> called for the first time, the blobs pointer is set to NULL. Therefore,
> double free will not occur in the code.
> 
> Fixes: 6af36aeb147a ("lsm: add backing_file LSM hooks")
> Signed-off-by: Cai Xinchen <[email protected]>
> ---
>  security/security.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/security/security.c b/security/security.c
> index 71aea8fdf014..595d3c73253e 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -2468,11 +2468,8 @@ int security_backing_file_alloc(struct file *backing_file,
>  	rc = lsm_backing_file_alloc(backing_file);
>  	if (rc)
>  		return rc;
> -	rc = call_int_hook(backing_file_alloc, backing_file, user_file);
> -	if (unlikely(rc))
> -		security_backing_file_free(backing_file);
>  
> -	return rc;
> +	return call_int_hook(backing_file_alloc, backing_file, user_file);
>  }

I think the better option would be to move the
call_void_hook(backing_file_free, ...) call in security_backing_file_free()
into the if-statment true block before we set the backing file's LSM blob
pointer to NULL and free the LSM blob.

--
paul-moore.com
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.