mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v2 -next 0/2] security: Fix call security_backing_file_free second time
@ 2026-07-07  8:06 Cai Xinchen
  2026-07-07  8:06 ` [PATCH v2 -next 1/2] security: Delete dumplicate assignment Cai Xinchen
  2026-07-07  8:06 ` [PATCH v2 -next 2/2] security: Fix call security_backing_file_free second time Cai Xinchen
  0 siblings, 2 replies; 7+ messages in thread
From: Cai Xinchen @ 2026-07-07  8:06 UTC (permalink / raw)
  To: paul, jmorris, serge, amir73il, brauner, caixinchen1
  Cc: linux-security-module, linux-kernel, lujialin4

v2: 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.

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 it always
return 0. 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.

Cai Xinchen (2):
  security: Delete dumplicate assignment
  security: Fix call security_backing_file_free second time

 security/lsm_init.c | 1 -
 security/security.c | 3 +--
 2 files changed, 1 insertion(+), 3 deletions(-)

-- 
2.34.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v2 -next 1/2] security: Delete dumplicate assignment
  2026-07-07  8:06 [PATCH v2 -next 0/2] security: Fix call security_backing_file_free second time Cai Xinchen
@ 2026-07-07  8:06 ` Cai Xinchen
  2026-08-27 16:30   ` [PATCH v2 " Paul Moore
  2026-07-07  8:06 ` [PATCH v2 -next 2/2] security: Fix call security_backing_file_free second time Cai Xinchen
  1 sibling, 1 reply; 7+ messages in thread
From: Cai Xinchen @ 2026-07-07  8:06 UTC (permalink / raw)
  To: paul, jmorris, serge, amir73il, brauner, caixinchen1
  Cc: linux-security-module, linux-kernel, lujialin4

Delete a blobs variable with duplicate assignment.

Signed-off-by: Cai Xinchen <caixinchen1@huawei.com>
---
 security/lsm_init.c | 1 -
 1 file changed, 1 deletion(-)

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,
-- 
2.34.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v2 -next 2/2] security: Fix call security_backing_file_free second time
  2026-07-07  8:06 [PATCH v2 -next 0/2] security: Fix call security_backing_file_free second time Cai Xinchen
  2026-07-07  8:06 ` [PATCH v2 -next 1/2] security: Delete dumplicate assignment Cai Xinchen
@ 2026-07-07  8:06 ` Cai Xinchen
  2026-08-27 16:30   ` [PATCH v2 " Paul Moore
  1 sibling, 1 reply; 7+ messages in thread
From: Cai Xinchen @ 2026-07-07  8:06 UTC (permalink / raw)
  To: paul, jmorris, serge, amir73il, brauner, caixinchen1
  Cc: linux-security-module, linux-kernel, lujialin4

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 it always
return 0. 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 | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/security/security.c b/security/security.c
index 71aea8fdf014..bec2f4ebea34 100644
--- a/security/security.c
+++ b/security/security.c
@@ -2486,9 +2486,8 @@ void security_backing_file_free(struct file *backing_file)
 {
 	void *blob = backing_file_security(backing_file);
 
-	call_void_hook(backing_file_free, backing_file);
-
 	if (blob) {
+		call_void_hook(backing_file_free, backing_file);
 		backing_file_set_security(backing_file, NULL);
 		kmem_cache_free(lsm_backing_file_cache, blob);
 	}
-- 
2.34.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 1/2] security: Delete dumplicate assignment
  2026-07-07  8:06 ` [PATCH v2 -next 1/2] security: Delete dumplicate assignment Cai Xinchen
@ 2026-08-27 16:30   ` Paul Moore
  2026-08-31 21:28     ` Paul Moore
  0 siblings, 1 reply; 7+ messages in thread
From: Paul Moore @ 2026-08-27 16:30 UTC (permalink / raw)
  To: Cai Xinchen, jmorris, serge, amir73il, brauner, caixinchen1
  Cc: linux-security-module, linux-kernel, lujialin4

On Jul  7, 2026 Cai Xinchen <caixinchen1@huawei.com> wrote:
> 
> Delete a blobs variable with duplicate assignment.
> 
> Signed-off-by: Cai Xinchen <caixinchen1@huawei.com>
> ---
>  security/lsm_init.c | 1 -
>  1 file changed, 1 deletion(-)

Merged into lsm/dev-staging and I'll plan to move it to lsm/dev once the
merge window closes, thanks!

--
paul-moore.com

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 2/2] security: Fix call security_backing_file_free  second time
  2026-07-07  8:06 ` [PATCH v2 -next 2/2] security: Fix call security_backing_file_free second time Cai Xinchen
@ 2026-08-27 16:30   ` Paul Moore
  2026-08-31 21:29     ` Paul Moore
  0 siblings, 1 reply; 7+ messages in thread
From: Paul Moore @ 2026-08-27 16:30 UTC (permalink / raw)
  To: Cai Xinchen, jmorris, serge, amir73il, brauner, caixinchen1
  Cc: linux-security-module, linux-kernel, lujialin4

On Jul  7, 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, ...)
>            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 it always
> return 0. 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 | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)

Merged into lsm/dev-staging, will be moved to lsm/dev after the merge
window.

--
paul-moore.com

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 1/2] security: Delete dumplicate assignment
  2026-08-27 16:30   ` [PATCH v2 " Paul Moore
@ 2026-08-31 21:28     ` Paul Moore
  0 siblings, 0 replies; 7+ messages in thread
From: Paul Moore @ 2026-08-31 21:28 UTC (permalink / raw)
  To: Cai Xinchen, jmorris, serge, amir73il, brauner
  Cc: linux-security-module, linux-kernel, lujialin4

On Thu, Aug 27, 2026 at 12:30 PM Paul Moore <paul@paul-moore.com> wrote:
>
> On Jul  7, 2026 Cai Xinchen <caixinchen1@huawei.com> wrote:
> >
> > Delete a blobs variable with duplicate assignment.
> >
> > Signed-off-by: Cai Xinchen <caixinchen1@huawei.com>
> > ---
> >  security/lsm_init.c | 1 -
> >  1 file changed, 1 deletion(-)
>
> Merged into lsm/dev-staging and I'll plan to move it to lsm/dev once the
> merge window closes, thanks!

This is now merged into lsm/dev, thanks again.

-- 
paul-moore.com

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 2/2] security: Fix call security_backing_file_free second time
  2026-08-27 16:30   ` [PATCH v2 " Paul Moore
@ 2026-08-31 21:29     ` Paul Moore
  0 siblings, 0 replies; 7+ messages in thread
From: Paul Moore @ 2026-08-31 21:29 UTC (permalink / raw)
  To: Cai Xinchen, jmorris, serge, amir73il, brauner
  Cc: linux-security-module, linux-kernel, lujialin4

On Thu, Aug 27, 2026 at 12:30 PM Paul Moore <paul@paul-moore.com> wrote:
> On Jul  7, 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, ...)
> >            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 it always
> > return 0. 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 | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
>
> Merged into lsm/dev-staging, will be moved to lsm/dev after the merge
> window.

This is now in lsm/dev, thanks!

-- 
paul-moore.com

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-08-31 21:29 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-07-07  8:06 [PATCH v2 -next 0/2] security: Fix call security_backing_file_free second time Cai Xinchen
2026-07-07  8:06 ` [PATCH v2 -next 1/2] security: Delete dumplicate assignment Cai Xinchen
2026-08-27 16:30   ` [PATCH v2 " Paul Moore
2026-08-31 21:28     ` Paul Moore
2026-07-07  8:06 ` [PATCH v2 -next 2/2] security: Fix call security_backing_file_free second time Cai Xinchen
2026-08-27 16:30   ` [PATCH v2 " Paul Moore
2026-08-31 21:29     ` Paul Moore

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®