From a4a2ab16d0e1e270f81b3ab9ffcba586cce42287 Mon Sep 17 00:00:00 2001 From: Steve Beattie Date: Thu, 3 May 2007 04:09:35 +0000 Subject: [PATCH] Add dentry refcounting possible fix. --- .../apparmorfs_dentry_refcount_fix | 61 +++++++++++++++++++ kernel-patches/for-mainline/series | 1 + 2 files changed, 62 insertions(+) create mode 100644 kernel-patches/for-mainline/apparmorfs_dentry_refcount_fix diff --git a/kernel-patches/for-mainline/apparmorfs_dentry_refcount_fix b/kernel-patches/for-mainline/apparmorfs_dentry_refcount_fix new file mode 100644 index 000000000..2c0d828e8 --- /dev/null +++ b/kernel-patches/for-mainline/apparmorfs_dentry_refcount_fix @@ -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; + } + diff --git a/kernel-patches/for-mainline/series b/kernel-patches/for-mainline/series index 560c18647..569712043 100644 --- a/kernel-patches/for-mainline/series +++ b/kernel-patches/for-mainline/series @@ -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