linux-kernel - Re: [PATCH] apparmor: test: Fix memory leak for aa_unpack

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <4b7f9b0b-1a24-46fc-9046-069f06f3c965@canonical.com>
Date: Sat, 9 Nov 2024 11:36:02 -0800
From: John Johansen <john.johansen@...onical.com>
To: Jinjie Ruan <ruanjinjie@...wei.com>, paul@...l-moore.com,
 jmorris@...ei.org, serge@...lyn.com, skhan@...uxfoundation.org,
 mike.salvatore@...onical.com, kees@...nel.org, brendan.higgins@...ux.dev,
 apparmor@...ts.ubuntu.com, linux-security-module@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH] apparmor: test: Fix memory leak for aa_unpack_strdup()

On 10/10/24 18:22, Jinjie Ruan wrote:
> The string allocated by kmemdup() in aa_unpack_strdup() is not
> freed and cause following memory leaks, free them to fix it.
> 
> 	unreferenced object 0xffffff80c6af8a50 (size 8):
> 	  comm "kunit_try_catch", pid 225, jiffies 4294894407
> 	  hex dump (first 8 bytes):
> 	    74 65 73 74 69 6e 67 00                          testing.
> 	  backtrace (crc 5eab668b):
> 	    [<0000000001e3714d>] kmemleak_alloc+0x34/0x40
> 	    [<000000006e6c7776>] __kmalloc_node_track_caller_noprof+0x300/0x3e0
> 	    [<000000006870467c>] kmemdup_noprof+0x34/0x60
> 	    [<000000001176bb03>] aa_unpack_strdup+0xd0/0x18c
> 	    [<000000008ecde918>] policy_unpack_test_unpack_strdup_with_null_name+0xf8/0x3ec
> 	    [<0000000032ef8f77>] kunit_try_run_case+0x13c/0x3ac
> 	    [<00000000f3edea23>] kunit_generic_run_threadfn_adapter+0x80/0xec
> 	    [<00000000adf936cf>] kthread+0x2e8/0x374
> 	    [<0000000041bb1628>] ret_from_fork+0x10/0x20
> 	unreferenced object 0xffffff80c2a29090 (size 8):
> 	  comm "kunit_try_catch", pid 227, jiffies 4294894409
> 	  hex dump (first 8 bytes):
> 	    74 65 73 74 69 6e 67 00                          testing.
> 	  backtrace (crc 5eab668b):
> 	    [<0000000001e3714d>] kmemleak_alloc+0x34/0x40
> 	    [<000000006e6c7776>] __kmalloc_node_track_caller_noprof+0x300/0x3e0
> 	    [<000000006870467c>] kmemdup_noprof+0x34/0x60
> 	    [<000000001176bb03>] aa_unpack_strdup+0xd0/0x18c
> 	    [<0000000046a45c1a>] policy_unpack_test_unpack_strdup_with_name+0xd0/0x3c4
> 	    [<0000000032ef8f77>] kunit_try_run_case+0x13c/0x3ac
> 	    [<00000000f3edea23>] kunit_generic_run_threadfn_adapter+0x80/0xec
> 	    [<00000000adf936cf>] kthread+0x2e8/0x374
> 	    [<0000000041bb1628>] ret_from_fork+0x10/0x20
> 
> Cc: stable@...r.kernel.org
> Fixes: 4d944bcd4e73 ("apparmor: add AppArmor KUnit tests for policy unpack")
> Signed-off-by: Jinjie Ruan <ruanjinjie@...wei.com>

Acked-by: John Johansen <john.johansen@...onical.com>

I have pulled this into my tree

> ---
>   security/apparmor/policy_unpack_test.c | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/security/apparmor/policy_unpack_test.c b/security/apparmor/policy_unpack_test.c
> index c64733d6c98f..f070902da8fc 100644
> --- a/security/apparmor/policy_unpack_test.c
> +++ b/security/apparmor/policy_unpack_test.c
> @@ -281,6 +281,8 @@ static void policy_unpack_test_unpack_strdup_with_null_name(struct kunit *test)
>   			   ((uintptr_t)puf->e->start <= (uintptr_t)string)
>   			   && ((uintptr_t)string <= (uintptr_t)puf->e->end));
>   	KUNIT_EXPECT_STREQ(test, string, TEST_STRING_DATA);
> +
> +	kfree(string);
>   }
>   
>   static void policy_unpack_test_unpack_strdup_with_name(struct kunit *test)
> @@ -296,6 +298,8 @@ static void policy_unpack_test_unpack_strdup_with_name(struct kunit *test)
>   			   ((uintptr_t)puf->e->start <= (uintptr_t)string)
>   			   && ((uintptr_t)string <= (uintptr_t)puf->e->end));
>   	KUNIT_EXPECT_STREQ(test, string, TEST_STRING_DATA);
> +
> +	kfree(string);
>   }
>   
>   static void policy_unpack_test_unpack_strdup_out_of_bounds(struct kunit *test)
> @@ -313,6 +317,8 @@ static void policy_unpack_test_unpack_strdup_out_of_bounds(struct kunit *test)
>   	KUNIT_EXPECT_EQ(test, size, 0);
>   	KUNIT_EXPECT_NULL(test, string);
>   	KUNIT_EXPECT_PTR_EQ(test, puf->e->pos, start);
> +
> +	kfree(string);
>   }
>   
>   static void policy_unpack_test_unpack_nameX_with_null_name(struct kunit *test)