* [PATCH -next 0/2] Fix call security_backing_file_free second time
@ 2026-06-26 1:17 Cai Xinchen
2026-06-26 1:17 ` [PATCH -next 1/2] security: Some cleanup code Cai Xinchen
2026-06-26 1:17 ` [PATCH -next 2/2] security: Fix call security_backing_file_free second time Cai Xinchen
0 siblings, 2 replies; 5+ messages in thread
From: Cai Xinchen @ 2026-06-26 1:17 UTC (permalink / raw)
To: paul, jmorris, serge, stephen.smalley.work, omosnace, amir73il,
brauner
Cc: linux-security-module, linux-kernel, selinux, caixinchen1,
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
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH -next 1/2] security: Some cleanup code
2026-06-26 1:17 [PATCH -next 0/2] Fix call security_backing_file_free second time Cai Xinchen
@ 2026-06-26 1:17 ` Cai Xinchen
2026-07-02 21:09 ` [PATCH " Paul Moore
2026-06-26 1:17 ` [PATCH -next 2/2] security: Fix call security_backing_file_free second time Cai Xinchen
1 sibling, 1 reply; 5+ messages in thread
From: Cai Xinchen @ 2026-06-26 1:17 UTC (permalink / raw)
To: paul, jmorris, serge, stephen.smalley.work, omosnace, amir73il,
brauner
Cc: linux-security-module, linux-kernel, selinux, caixinchen1,
lujialin4
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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH -next 2/2] security: Fix call security_backing_file_free second time
2026-06-26 1:17 [PATCH -next 0/2] Fix call security_backing_file_free second time Cai Xinchen
2026-06-26 1:17 ` [PATCH -next 1/2] security: Some cleanup code Cai Xinchen
@ 2026-06-26 1:17 ` Cai Xinchen
2026-07-02 21:09 ` [PATCH " Paul Moore
1 sibling, 1 reply; 5+ messages in thread
From: Cai Xinchen @ 2026-06-26 1:17 UTC (permalink / raw)
To: paul, jmorris, serge, stephen.smalley.work, omosnace, amir73il,
brauner
Cc: linux-security-module, linux-kernel, selinux, caixinchen1,
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 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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] security: Some cleanup code
2026-06-26 1:17 ` [PATCH -next 1/2] security: Some cleanup code Cai Xinchen
@ 2026-07-02 21:09 ` Paul Moore
0 siblings, 0 replies; 5+ messages in thread
From: Paul Moore @ 2026-07-02 21:09 UTC (permalink / raw)
To: Cai Xinchen, jmorris, serge, stephen.smalley.work, omosnace,
amir73il, brauner
Cc: linux-security-module, linux-kernel, selinux, caixinchen1,
lujialin4
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] security: Fix call security_backing_file_free second time
2026-06-26 1:17 ` [PATCH -next 2/2] security: Fix call security_backing_file_free second time Cai Xinchen
@ 2026-07-02 21:09 ` Paul Moore
0 siblings, 0 replies; 5+ messages in thread
From: Paul Moore @ 2026-07-02 21:09 UTC (permalink / raw)
To: Cai Xinchen, jmorris, serge, stephen.smalley.work, omosnace,
amir73il, brauner
Cc: linux-security-module, linux-kernel, selinux, caixinchen1,
lujialin4
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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-07-02 21:09 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-26 1:17 [PATCH -next 0/2] Fix call security_backing_file_free second time Cai Xinchen
2026-06-26 1:17 ` [PATCH -next 1/2] security: Some cleanup code Cai Xinchen
2026-07-02 21:09 ` [PATCH " Paul Moore
2026-06-26 1:17 ` [PATCH -next 2/2] security: Fix call security_backing_file_free second time Cai Xinchen
2026-07-02 21:09 ` [PATCH " Paul Moore
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox