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