what's going on with aa_destroy_aafs() call in apparmor_init()?

what's going on with aa_destroy_aafs() call in apparmor_init()?

[Al Viro]

	Correct me if I'm wrong, but as far as I can tell apparmor_init()
ends up being called from security_init(), which is called before the call
of vfs_caches_init(), not to mention fs_initcall stuff.

	If that's the case, what is this doing there?
error:
        aa_destroy_aafs();
	AA_ERROR("Error creating AppArmor securityfs\n");
	return error;

aa_create_aafs() is called via fs_initcall; moreover, it will bail out
if called before apparmor_initialized has become true, so...

While we are at it, what will happen if apparmor_init() succeeds, but
aa_create_fs() fails afterwards?

If nothing else, aa_null_path will be left {NULL, NULL}, which will
immediately oops dentry_open() in aa_inherit_files()...

Re: what's going on with aa_destroy_aafs() call in apparmor_init()?

[John Johansen]

On 9/21/25 00:36, Al Viro wrote:
> 	Correct me if I'm wrong, but as far as I can tell apparmor_init()
> ends up being called from security_init(), which is called before the call
> of vfs_caches_init(), not to mention fs_initcall stuff.

sigh, yes that is what is happening.

> 	If that's the case, what is this doing there?
> error:
>          aa_destroy_aafs();
> 	AA_ERROR("Error creating AppArmor securityfs\n");
> 	return error;

it shouldn't be it should have been dropped ages ago. Its a mess up on a
patch and then it just got carried forward

> aa_create_aafs() is called via fs_initcall; moreover, it will bail out
> if called before apparmor_initialized has become true, so...
>
> While we are at it, what will happen if apparmor_init() succeeds, but
> aa_create_fs() fails afterwards?

Yeah that one is ugly. I will have to poke at the best solution atm.
For current kernels, no policy will load. Backing out the LSM hooks
isn't going to happen but we will need to just run in an unconfined
state and make sure any references to the fs handle the null condition.

> If nothing else, aa_null_path will be left {NULL, NULL}, which will
> immediately oops dentry_open() in aa_inherit_files()...

indeed, I have to wonder if it was always that way or I just messed up.
I borrowed the pattern from selinux but should have also dug into what
it was doing.

regardless it needs to be fixed.
We can check that its null and set the file to null for replace fds.

Give me a bit to see about some patches.