mirror of
https://gitlab.com/apparmor/apparmor
synced 2025-08-31 14:25:52 +00:00
Add dentry refcounting possible fix.
This commit is contained in:
61
kernel-patches/for-mainline/apparmorfs_dentry_refcount_fix
Normal file
61
kernel-patches/for-mainline/apparmorfs_dentry_refcount_fix
Normal file
@@ -0,0 +1,61 @@
|
||||
With the lkml patches currently in svn, there is a dentry refcounting
|
||||
bug somewhere. Unloading the apparmor module ends up leaving around the
|
||||
/sys/kernel/security/apparmor/ directory and files in place. Followup
|
||||
attempts to reload the module fail because because the securityfs files
|
||||
exist which causes the apparmorfs registration attempt to fail[1].
|
||||
Attempts to read from the leftover files (e.g. by the rcapparmor script
|
||||
looking to see if profiles are already loaded) would of course cause an
|
||||
oops because the backing operations no longer exist.
|
||||
|
||||
Closer examination showed that the reference count on each dentry/file
|
||||
in the apparmor/ directory was at 2 *after* securityfs_remove() had been
|
||||
called. The apparmor/ directory itself could not be removed because all
|
||||
of its member entries still existed. Diagnostic printk()s also showed that
|
||||
the reference count on the dentries is also 2 immediately after
|
||||
creation.
|
||||
|
||||
I'm not quite sure where the extra references come from, or why things
|
||||
worked before (and they definitely do work in a 2.6.18 era kernel). The
|
||||
following patch adjusts the reference counts in such a way that they go
|
||||
to zero on unloading, but I'm not sure they're the *right* fixes, it may
|
||||
just be papering over a refcounting bug elsewhere.
|
||||
|
||||
Any comments? (This patch is checked in to subversion.)
|
||||
|
||||
[1] There's also a *really* annoying bug with modprobe exposed here;
|
||||
when module_init(1) fails with -EEXIST returned, modprobe silently
|
||||
ignores it and returns success. We end up propogating the -EEXIST
|
||||
error from the apparmorfs creation, so it looks initially like module
|
||||
reloading succeeded when it didn't.
|
||||
|
||||
---
|
||||
security/apparmor/apparmorfs.c | 8 +++++++-
|
||||
1 file changed, 7 insertions(+), 1 deletion(-)
|
||||
|
||||
Index: b/security/apparmor/apparmorfs.c
|
||||
===================================================================
|
||||
--- a/security/apparmor/apparmorfs.c
|
||||
+++ b/security/apparmor/apparmorfs.c
|
||||
@@ -179,8 +179,11 @@ static void aafs_remove(const char *name
|
||||
struct dentry *dentry;
|
||||
|
||||
dentry = lookup_one_len(name, apparmor_dentry, strlen(name));
|
||||
- if (dentry && !IS_ERR(dentry))
|
||||
+ if (dentry && !IS_ERR(dentry)) {
|
||||
securityfs_remove(dentry);
|
||||
+ dput(dentry);
|
||||
+ } else
|
||||
+ AA_ERROR("Error removing AppArmor securityfs file %s\n", name);
|
||||
}
|
||||
|
||||
static int aafs_create(const char *name, int mask, struct file_operations *fops)
|
||||
@@ -190,6 +193,9 @@ static int aafs_create(const char *name,
|
||||
dentry = securityfs_create_file(name, S_IFREG | mask, apparmor_dentry,
|
||||
NULL, fops);
|
||||
|
||||
+ if (!IS_ERR(dentry))
|
||||
+ dput(dentry);
|
||||
+
|
||||
return IS_ERR(dentry) ? PTR_ERR(dentry) : 0;
|
||||
}
|
||||
|
@@ -46,6 +46,7 @@ mangle_on_audit.diff
|
||||
do_path_lookup-nameidata.diff
|
||||
sys_fchdir-nameidata.diff
|
||||
file_permission-nameidata.diff
|
||||
apparmorfs_dentry_refcount_fix
|
||||
# NOT YET
|
||||
leaf.diff
|
||||
fix_leaf.diff
|
||||
|
Reference in New Issue
Block a user