On Fri, Mar 27, 2026 at 11:05 PM Paul Moore [off-list ref] wrote:
Stacked filesystems such as overlayfs do not currently provide the
necessary mechanisms for LSMs to properly enforce access controls on the
mmap() and mprotect() operations. In order to resolve this gap, a LSM
security blob is being added to the backing_file struct and the following
new LSM hooks are being created:
security_backing_file_alloc()
security_backing_file_free()
security_mmap_backing_file()
The first two hooks are to manage the lifecycle of the LSM security blob
in the backing_file struct, while the third provides a new mmap() access
control point for the underlying backing file. It is also expected that
LSMs will likely want to update their security_file_mprotect() callback
to address issues with their mprotect() controls, but that does not
require a change to the security_file_mprotect() LSM hook.
There are a two other small changes to support these new LSM hooks. We
pass the user file associated with a backing file down to
alloc_empty_backing_file() so it can be included in the
security_backing_file_alloc() hook, and we constify the file struct field
in the LSM common_audit_data struct to better support LSMs that need to
pass a const file struct pointer into the common LSM audit code.
Thanks to Arnd Bergmann for identifying the missing EXPORT_SYMBOL_GPL()
and supplying a fixup.
Cc: <redacted>
Acked-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Paul Moore <paul@paul-moore.com>
---
fs/backing-file.c | 18 ++++--
fs/erofs/ishare.c | 10 +++-
fs/file_table.c | 21 ++++++-
fs/fuse/passthrough.c | 2 +-
fs/internal.h | 3 +-
fs/overlayfs/dir.c | 2 +-
fs/overlayfs/file.c | 2 +-
include/linux/backing-file.h | 4 +-
include/linux/fs.h | 1 +
include/linux/lsm_audit.h | 2 +-
include/linux/lsm_hook_defs.h | 5 ++
include/linux/lsm_hooks.h | 1 +
include/linux/security.h | 22 ++++++++
security/lsm.h | 1 +
security/lsm_init.c | 9 +++
security/security.c | 100 ++++++++++++++++++++++++++++++++++
16 files changed, 187 insertions(+), 16 deletions(-)
That looks like a nice clean abstraction to me.
...
quoted hunk ↗ jump to hunk
diff --git a/fs/file_table.c b/fs/file_table.c
index aaa5faaace1e..0bdc26cae138 100644
--- a/fs/file_table.c
+++ b/fs/file_table.c
@@ -50,6 +50,7 @@ struct backing_file {
struct path user_path;
freeptr_t bf_freeptr;
};
+ void *security;
This needs ifdef SECURITY
and the name should be user_security
quoted hunk ↗ jump to hunk
};
#define backing_file(f) container_of(f, struct backing_file, file)
@@ -66,6 +67,11 @@ void backing_file_set_user_path(struct file *f, const struct path *path)
}
EXPORT_SYMBOL_GPL(backing_file_set_user_path);
+void *backing_file_security(const struct file *f)
+{
+ return backing_file(f)->security;
+}
I prefer the name backing_file_user_security()
Terminology here is very confusing but when saying
"backing file" it is more natural that one is referring to the
backing xfs file with overlayfs has opened.
The "backing file" already has an LSM blob f->f_security
which is fair the call it the "backing file's LSM blob"
Therefore, I think we need to make a distinction, as we did
with backing_file_user_path() and refer to this as something along
the lines of the "backing file's user LSM blob".
quoted hunk ↗ jump to hunk
+
static inline void file_free(struct file *f)
{
security_file_free(f);@@ -73,8 +79,11 @@ static inline void file_free(struct file *f)
percpu_counter_dec(&nr_files);
put_cred(f->f_cred);
if (unlikely(f->f_mode & FMODE_BACKING)) {
- path_put(backing_file_user_path(f));
- kmem_cache_free(bfilp_cachep, backing_file(f));
+ struct backing_file *ff = backing_file(f);
+
+ security_backing_file_free(&ff->security);
+ path_put(&ff->user_path);
+ kmem_cache_free(bfilp_cachep, ff);
Not directly related to your patch, but as this is growing, IMO
this would look cleaner with backing_file_free() inline helper
(see attached path).
quoted hunk ↗ jump to hunk
} else {
kmem_cache_free(filp_cachep, f);
}@@ -290,7 +299,8 @@ struct file *alloc_empty_file_noaccount(int flags, const struct cred *cred)
* This is only for kernel internal use, and the allocate file must not be
* installed into file tables or such.
*/
-struct file *alloc_empty_backing_file(int flags, const struct cred *cred)
+struct file *alloc_empty_backing_file(int flags, const struct cred *cred,
+ const struct file *user_file)
{
struct backing_file *ff;
int error;@@ -306,6 +316,11 @@ struct file *alloc_empty_backing_file(int flags, const struct cred *cred)
}
ff->file.f_mode |= FMODE_BACKING | FMODE_NOACCOUNT;
+ error = security_backing_file_alloc(&ff->security, user_file);
+ if (unlikely(error)) {
+ fput(&ff->file);
+ return ERR_PTR(error);
+ }
return &ff->file;
}
There is an API issue here.
in order to call fput() we must ensure that user_security was initialized to
NULL (or allocated).
I don't think that we want security_backing_file_alloc() to provide this
semantic and the current patch does not implement it.
Furthermore, user_path is also not initialized in the error case.
Attached UNTESTED fixup patch to suggest a cleanup with
init_backing_file() helper.
It also changes the variable and helper name to user_security
and plays some trick to avoid many ifdef SECURITY.
Feel free to take whichever bits you like with/without attribution.
If you prefer, attached also a proper prep patch.
compile tested only.
Thanks,
Amir.