diff mbox series

apparmor: fix bind mounts aborting with -ENOMEM

Message ID c70b386ac87254dcd3e23ae5ab168e44b1567e28.1576064594.git.ps@pks.im (mailing list archive)
State New, archived
Headers show
Series apparmor: fix bind mounts aborting with -ENOMEM | expand

Commit Message

Patrick Steinhardt Dec. 11, 2019, 11:44 a.m. UTC
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(-)

Comments

John Johansen Dec. 14, 2019, 5:39 a.m. UTC | #1
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,
>
diff mbox series

Patch

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,