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

[Paul Moore <paul@paul-moore.com>]

On Jun 25, 2026 Cai Xinchen <caixinchen1@huawei.com> 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 <caixinchen1@huawei.com>
> ---
>  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