[PATCH] apparmor: fix bind mounts aborting with -ENOMEM

[PATCH] apparmor: fix bind mounts aborting with -ENOMEM

[Patrick Steinhardt]

With commit df323337e507 (apparmor: Use a memory pool instead per-CPU
caches, 2019-05-03), AppArmor code was converted to use memory pools. In
that conversion, a bug snuck into the code that polices bind mounts that
causes all bind mounts to fail with -ENOMEM, as we erroneously error out
if `aa_get_buffer` returns a pointer instead of erroring out when it
does _not_ return a valid pointer.

Fix the issue by correctly checking for valid pointers returned by
`aa_get_buffer` to fix bind mounts with AppArmor.

Fixes: df323337e507 (apparmor: Use a memory pool instead per-CPU caches)
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---

I've fixed the issue on top of v5.5-rc1, where I in fact found
the issue.

 security/apparmor/mount.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/security/apparmor/mount.c b/security/apparmor/mount.c
index 4ed6688f9d40..e0828ee7a345 100644
--- a/security/apparmor/mount.c
+++ b/security/apparmor/mount.c
@@ -442,7 +442,7 @@ int aa_bind_mount(struct aa_label *label, const struct path *path,
 	buffer = aa_get_buffer(false);
 	old_buffer = aa_get_buffer(false);
 	error = -ENOMEM;
-	if (!buffer || old_buffer)
+	if (!buffer || !old_buffer)
 		goto out;
 
 	error = fn_for_each_confined(label, profile,
-- 
2.24.1

Re: [PATCH] apparmor: fix bind mounts aborting with -ENOMEM

[John Johansen]

On 12/11/19 3:44 AM, Patrick Steinhardt wrote:
> With commit df323337e507 (apparmor: Use a memory pool instead per-CPU
> caches, 2019-05-03), AppArmor code was converted to use memory pools. In
> that conversion, a bug snuck into the code that polices bind mounts that
> causes all bind mounts to fail with -ENOMEM, as we erroneously error out
> if `aa_get_buffer` returns a pointer instead of erroring out when it
> does _not_ return a valid pointer.
> 
> Fix the issue by correctly checking for valid pointers returned by
> `aa_get_buffer` to fix bind mounts with AppArmor.
> 
> Fixes: df323337e507 (apparmor: Use a memory pool instead per-CPU caches)
> Signed-off-by: Patrick Steinhardt <ps@pks.im>

Sigh yep, I'm not sure how that slipped through. Obviously there is an
issue with out mount regression tests that needs to be found and fixed.

I'll pull this in and send it up. Thanks Patrick


> ---
> 
> I've fixed the issue on top of v5.5-rc1, where I in fact found
> the issue.
> 
>  security/apparmor/mount.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/security/apparmor/mount.c b/security/apparmor/mount.c
> index 4ed6688f9d40..e0828ee7a345 100644
> --- a/security/apparmor/mount.c
> +++ b/security/apparmor/mount.c
> @@ -442,7 +442,7 @@ int aa_bind_mount(struct aa_label *label, const struct path *path,
>  	buffer = aa_get_buffer(false);
>  	old_buffer = aa_get_buffer(false);
>  	error = -ENOMEM;
> -	if (!buffer || old_buffer)
> +	if (!buffer || !old_buffer)
>  		goto out;
>  
>  	error = fn_for_each_confined(label, profile,
>