[PATCH -next 0/2] Fix call security_backing_file_free second time

[PATCH -next 0/2] Fix call security_backing_file_free second time

[Cai Xinchen]

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, ...)
           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

Cai Xinchen (2):
  security: Some cleanup code
  security: Fix call security_backing_file_free second time

 security/lsm_init.c      | 1 -
 security/security.c      | 5 +----
 security/selinux/hooks.c | 1 -
 3 files changed, 1 insertion(+), 6 deletions(-)

-- 
2.34.1

[PATCH -next 1/2] security: Some cleanup code

[Cai Xinchen]

Delete an unnecessary blank line and a blobs variable with
duplicate assignment.

Signed-off-by: Cai Xinchen <caixinchen1@huawei.com>
---
 security/lsm_init.c      | 1 -
 security/selinux/hooks.c | 1 -
 2 files changed, 2 deletions(-)

diff --git a/security/lsm_init.c b/security/lsm_init.c
index 7c0fd17f1601..d7384866e3a5 100644
--- a/security/lsm_init.c
+++ b/security/lsm_init.c
@@ -290,7 +290,6 @@ static void __init lsm_prepare(struct lsm_info *lsm)
 		return;
 
 	/* Register the LSM blob sizes. */
-	blobs = lsm->blobs;
 	lsm_blob_size_update(&blobs->lbs_cred, &blob_sizes.lbs_cred);
 	lsm_blob_size_update(&blobs->lbs_file, &blob_sizes.lbs_file);
 	lsm_blob_size_update(&blobs->lbs_backing_file,
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 1a713d96206f..e5930ebc9e37 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -1748,7 +1748,6 @@ static int bpf_fd_pass(const struct file *file, u32 sid);
 
 static int __file_has_perm(const struct cred *cred, const struct file *file,
 			   u32 av, bool bf_user_file)
-
 {
 	struct common_audit_data ad;
 	struct inode *inode;
-- 
2.34.1

Re: [PATCH 1/2] security: Some cleanup code

[Paul Moore]

On Jun 25, 2026 Cai Xinchen <caixinchen1@huawei.com> wrote:
> 
> Delete an unnecessary blank line and a blobs variable with
> duplicate assignment.
> 
> Signed-off-by: Cai Xinchen <caixinchen1@huawei.com>
> ---
>  security/lsm_init.c      | 1 -
>  security/selinux/hooks.c | 1 -
>  2 files changed, 2 deletions(-)

Please split this into two patches: one for the LSM (lsm_init.c) and
one for SELinux (hooks.c).  They are two different subsystems, mailing
lists, etc.

> diff --git a/security/lsm_init.c b/security/lsm_init.c
> index 7c0fd17f1601..d7384866e3a5 100644
> --- a/security/lsm_init.c
> +++ b/security/lsm_init.c
> @@ -290,7 +290,6 @@ static void __init lsm_prepare(struct lsm_info *lsm)
>  		return;
>  
>  	/* Register the LSM blob sizes. */
> -	blobs = lsm->blobs;
>  	lsm_blob_size_update(&blobs->lbs_cred, &blob_sizes.lbs_cred);
>  	lsm_blob_size_update(&blobs->lbs_file, &blob_sizes.lbs_file);
>  	lsm_blob_size_update(&blobs->lbs_backing_file,
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 1a713d96206f..e5930ebc9e37 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -1748,7 +1748,6 @@ static int bpf_fd_pass(const struct file *file, u32 sid);
>  
>  static int __file_has_perm(const struct cred *cred, const struct file *file,
>  			   u32 av, bool bf_user_file)
> -
>  {
>  	struct common_audit_data ad;
>  	struct inode *inode;
> -- 
> 2.34.1

--
paul-moore.com

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

[Cai Xinchen]

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, ...)
           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);
 }
 
 /**
-- 
2.34.1

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

[Paul Moore]

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