| Message ID | 20241010130231.3151896-1-ruanjinjie@huawei.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: microchip: vcap api: Fix memory leaks in vcap_api_encode_rule_test() | expand |
On Thu, Oct 10, 2024 at 09:02:31PM +0800, Jinjie Ruan wrote: > Commit a3c1e45156ad ("net: microchip: vcap: Fix use-after-free error in > kunit test") fixed the use-after-free error, but introduced below > memory leaks by removing necessary vcap_free_rule(), add it to fix it. > > unreferenced object 0xffffff80ca58b700 (size 192): > comm "kunit_try_catch", pid 1215, jiffies 4294898264 > hex dump (first 32 bytes): > 00 12 7a 00 05 00 00 00 0a 00 00 00 64 00 00 00 ..z.........d... > 00 00 00 00 00 00 00 00 00 04 0b cc 80 ff ff ff ................ > backtrace (crc 9c09c3fe): > [<0000000052a0be73>] kmemleak_alloc+0x34/0x40 > [<0000000043605459>] __kmalloc_cache_noprof+0x26c/0x2f4 > [<0000000040a01b8d>] vcap_alloc_rule+0x3cc/0x9c4 > [<000000003fe86110>] vcap_api_encode_rule_test+0x1ac/0x16b0 > [<00000000b3595fc4>] kunit_try_run_case+0x13c/0x3ac > [<0000000010f5d2bf>] kunit_generic_run_threadfn_adapter+0x80/0xec > [<00000000c5d82c9a>] kthread+0x2e8/0x374 > [<00000000f4287308>] ret_from_fork+0x10/0x20 I guess that the rest of the log could be trimmed from the commit message. But I don't feel strongly about that. Also, it is probably not necessary to repost just because of this, but as a bug fix this patch should be targeted at the net tree and that should be indicated in the subject. [PATCH net] ... ... > Cc: stable@vger.kernel.org > Fixes: a3c1e45156ad ("net: microchip: vcap: Fix use-after-free error in kunit test") > Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com> Reviewed-by: Simon Horman <horms@kernel.org>
> Cc: stable@vger.kernel.org > Fixes: a3c1e45156ad ("net: microchip: vcap: Fix use-after-free error in kunit test") > Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com> > --- > drivers/net/ethernet/microchip/vcap/vcap_api_kunit.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/net/ethernet/microchip/vcap/vcap_api_kunit.c b/drivers/net/ethernet/microchip/vcap/vcap_api_kunit.c > index f2a5a36fdacd..7251121ab196 100644 > --- a/drivers/net/ethernet/microchip/vcap/vcap_api_kunit.c > +++ b/drivers/net/ethernet/microchip/vcap/vcap_api_kunit.c > @@ -1444,6 +1444,8 @@ static void vcap_api_encode_rule_test(struct kunit *test) > > ret = vcap_del_rule(&test_vctrl, &test_netdev, id); > KUNIT_EXPECT_EQ(test, 0, ret); > + > + vcap_free_rule(rule); > } Wait, should vcap_del_rule not handle the freeing of the rule? Maybe Emil can shed some light on this.. /Daniel > > static void vcap_api_set_rule_counter_test(struct kunit *test) > -- > 2.34.1 >
On Fri Oct 11, 2024 at 12:24 PM CEST, Daniel Machon wrote: > > Cc: stable@vger.kernel.org > > Fixes: a3c1e45156ad ("net: microchip: vcap: Fix use-after-free error in kunit test") > > Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com> > > --- > > drivers/net/ethernet/microchip/vcap/vcap_api_kunit.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/drivers/net/ethernet/microchip/vcap/vcap_api_kunit.c b/drivers/net/ethernet/microchip/vcap/vcap_api_kunit.c > > index f2a5a36fdacd..7251121ab196 100644 > > --- a/drivers/net/ethernet/microchip/vcap/vcap_api_kunit.c > > +++ b/drivers/net/ethernet/microchip/vcap/vcap_api_kunit.c > > @@ -1444,6 +1444,8 @@ static void vcap_api_encode_rule_test(struct kunit *test) > > > > ret = vcap_del_rule(&test_vctrl, &test_netdev, id); > > KUNIT_EXPECT_EQ(test, 0, ret); > > + > > + vcap_free_rule(rule); > > } > > Wait, should vcap_del_rule not handle the freeing of the rule? > Maybe Emil can shed some light on this.. > > /Daniel > No, this is a bug. I made the mistake of thinking that vcap_del_rule freed the rule. However, it frees an internal copy of the rule, which is made in vcap_add_rule. The local copy must still be freed. I reproduced the leak and the patch fixes this. /Emil
On Thu Oct 10, 2024 at 3:02 PM CEST, Jinjie Ruan wrote: > Commit a3c1e45156ad ("net: microchip: vcap: Fix use-after-free error in > kunit test") fixed the use-after-free error, but introduced below > memory leaks by removing necessary vcap_free_rule(), add it to fix it. Thank you for the fix. I reproduced the bug and confirmed the fix. ... > Cc: stable@vger.kernel.org > Fixes: a3c1e45156ad ("net: microchip: vcap: Fix use-after-free error in kunit test") > Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com> Reviewed-by: Jens Emil Schulz Østergaard <jensemil.schulzostergaard@microchip.com>
> > > > Wait, should vcap_del_rule not handle the freeing of the rule? > > Maybe Emil can shed some light on this.. > > > > /Daniel > > > > No, this is a bug. I made the mistake of thinking that vcap_del_rule freed the > rule. > > However, it frees an internal copy of the rule, which is made in vcap_add_rule. > The local copy must still be freed. I reproduced the leak and the patch fixes > this. > > /Emil Ah, right. Thanks for clarifying! /Daniel
diff --git a/drivers/net/ethernet/microchip/vcap/vcap_api_kunit.c b/drivers/net/ethernet/microchip/vcap/vcap_api_kunit.c index f2a5a36fdacd..7251121ab196 100644 --- a/drivers/net/ethernet/microchip/vcap/vcap_api_kunit.c +++ b/drivers/net/ethernet/microchip/vcap/vcap_api_kunit.c @@ -1444,6 +1444,8 @@ static void vcap_api_encode_rule_test(struct kunit *test) ret = vcap_del_rule(&test_vctrl, &test_netdev, id); KUNIT_EXPECT_EQ(test, 0, ret); + + vcap_free_rule(rule); } static void vcap_api_set_rule_counter_test(struct kunit *test)
Commit a3c1e45156ad ("net: microchip: vcap: Fix use-after-free error in kunit test") fixed the use-after-free error, but introduced below memory leaks by removing necessary vcap_free_rule(), add it to fix it. unreferenced object 0xffffff80ca58b700 (size 192): comm "kunit_try_catch", pid 1215, jiffies 4294898264 hex dump (first 32 bytes): 00 12 7a 00 05 00 00 00 0a 00 00 00 64 00 00 00 ..z.........d... 00 00 00 00 00 00 00 00 00 04 0b cc 80 ff ff ff ................ backtrace (crc 9c09c3fe): [<0000000052a0be73>] kmemleak_alloc+0x34/0x40 [<0000000043605459>] __kmalloc_cache_noprof+0x26c/0x2f4 [<0000000040a01b8d>] vcap_alloc_rule+0x3cc/0x9c4 [<000000003fe86110>] vcap_api_encode_rule_test+0x1ac/0x16b0 [<00000000b3595fc4>] kunit_try_run_case+0x13c/0x3ac [<0000000010f5d2bf>] kunit_generic_run_threadfn_adapter+0x80/0xec [<00000000c5d82c9a>] kthread+0x2e8/0x374 [<00000000f4287308>] ret_from_fork+0x10/0x20 unreferenced object 0xffffff80cc0b0400 (size 64): comm "kunit_try_catch", pid 1215, jiffies 4294898265 hex dump (first 32 bytes): 80 04 0b cc 80 ff ff ff 18 b7 58 ca 80 ff ff ff ..........X..... 39 00 00 00 02 00 00 00 06 05 04 03 02 01 ff ff 9............... backtrace (crc daf014e9): [<0000000052a0be73>] kmemleak_alloc+0x34/0x40 [<0000000043605459>] __kmalloc_cache_noprof+0x26c/0x2f4 [<000000000ff63fd4>] vcap_rule_add_key+0x2cc/0x528 [<00000000dfdb1e81>] vcap_api_encode_rule_test+0x224/0x16b0 [<00000000b3595fc4>] kunit_try_run_case+0x13c/0x3ac [<0000000010f5d2bf>] kunit_generic_run_threadfn_adapter+0x80/0xec [<00000000c5d82c9a>] kthread+0x2e8/0x374 [<00000000f4287308>] ret_from_fork+0x10/0x20 unreferenced object 0xffffff80cc0b0480 (size 64): comm "kunit_try_catch", pid 1215, jiffies 4294898265 hex dump (first 32 bytes): 00 05 0b cc 80 ff ff ff 00 04 0b cc 80 ff ff ff ................ 43 00 00 00 02 00 00 00 88 75 32 34 9e b1 ff ff C........u24.... backtrace (crc b81c2109): [<0000000052a0be73>] kmemleak_alloc+0x34/0x40 [<0000000043605459>] __kmalloc_cache_noprof+0x26c/0x2f4 [<000000000ff63fd4>] vcap_rule_add_key+0x2cc/0x528 [<00000000455bcad8>] vcap_api_encode_rule_test+0x288/0x16b0 [<00000000b3595fc4>] kunit_try_run_case+0x13c/0x3ac [<0000000010f5d2bf>] kunit_generic_run_threadfn_adapter+0x80/0xec [<00000000c5d82c9a>] kthread+0x2e8/0x374 [<00000000f4287308>] ret_from_fork+0x10/0x20 unreferenced object 0xffffff80cc0b0500 (size 64): comm "kunit_try_catch", pid 1215, jiffies 4294898265 hex dump (first 32 bytes): 80 05 0b cc 80 ff ff ff 80 04 0b cc 80 ff ff ff ................ 26 00 00 00 00 00 00 00 01 01 32 34 9e b1 ff ff &.........24.... backtrace (crc cd869381): [<0000000052a0be73>] kmemleak_alloc+0x34/0x40 [<0000000043605459>] __kmalloc_cache_noprof+0x26c/0x2f4 [<000000000ff63fd4>] vcap_rule_add_key+0x2cc/0x528 [<0000000019ba4572>] vcap_api_encode_rule_test+0x2f4/0x16b0 [<00000000b3595fc4>] kunit_try_run_case+0x13c/0x3ac [<0000000010f5d2bf>] kunit_generic_run_threadfn_adapter+0x80/0xec [<00000000c5d82c9a>] kthread+0x2e8/0x374 [<00000000f4287308>] ret_from_fork+0x10/0x20 unreferenced object 0xffffff80cc0b0580 (size 64): comm "kunit_try_catch", pid 1215, jiffies 4294898265 hex dump (first 32 bytes): 00 06 0b cc 80 ff ff ff 00 05 0b cc 80 ff ff ff ................ 2d 00 00 00 00 00 00 00 00 00 32 34 9e b1 ff ff -.........24.... backtrace (crc b6e93f38): [<0000000052a0be73>] kmemleak_alloc+0x34/0x40 [<0000000043605459>] __kmalloc_cache_noprof+0x26c/0x2f4 [<000000000ff63fd4>] vcap_rule_add_key+0x2cc/0x528 [<000000002686800e>] vcap_api_encode_rule_test+0x3b4/0x16b0 [<00000000b3595fc4>] kunit_try_run_case+0x13c/0x3ac [<0000000010f5d2bf>] kunit_generic_run_threadfn_adapter+0x80/0xec [<00000000c5d82c9a>] kthread+0x2e8/0x374 [<00000000f4287308>] ret_from_fork+0x10/0x20 unreferenced object 0xffffff80cc0b0600 (size 64): comm "kunit_try_catch", pid 1215, jiffies 4294898265 hex dump (first 32 bytes): 80 06 0b cc 80 ff ff ff 80 05 0b cc 80 ff ff ff ................ 2e 00 00 00 01 00 00 00 05 00 00 00 0f 00 00 00 ................ backtrace (crc 756852a3): [<0000000052a0be73>] kmemleak_alloc+0x34/0x40 [<0000000043605459>] __kmalloc_cache_noprof+0x26c/0x2f4 [<000000000ff63fd4>] vcap_rule_add_key+0x2cc/0x528 [<0000000098d890ee>] vcap_api_encode_rule_test+0x414/0x16b0 [<00000000b3595fc4>] kunit_try_run_case+0x13c/0x3ac [<0000000010f5d2bf>] kunit_generic_run_threadfn_adapter+0x80/0xec [<00000000c5d82c9a>] kthread+0x2e8/0x374 [<00000000f4287308>] ret_from_fork+0x10/0x20 unreferenced object 0xffffff80cc0b0680 (size 64): comm "kunit_try_catch", pid 1215, jiffies 4294898265 hex dump (first 32 bytes): 00 09 0b cc 80 ff ff ff 00 06 0b cc 80 ff ff ff ................ 2c 00 00 00 01 00 00 00 01 cd ab ff ff ff ff ff ,............... backtrace (crc dbfa781b): [<0000000052a0be73>] kmemleak_alloc+0x34/0x40 [<0000000043605459>] __kmalloc_cache_noprof+0x26c/0x2f4 [<000000000ff63fd4>] vcap_rule_add_key+0x2cc/0x528 [<00000000178c94db>] vcap_api_encode_rule_test+0x474/0x16b0 [<00000000b3595fc4>] kunit_try_run_case+0x13c/0x3ac [<0000000010f5d2bf>] kunit_generic_run_threadfn_adapter+0x80/0xec [<00000000c5d82c9a>] kthread+0x2e8/0x374 [<00000000f4287308>] ret_from_fork+0x10/0x20 unreferenced object 0xffffff80cc0b0700 (size 64): comm "kunit_try_catch", pid 1215, jiffies 4294898265 hex dump (first 32 bytes): 80 07 0b cc 80 ff ff ff 28 b7 58 ca 80 ff ff ff ........(.X..... 3c 00 00 00 00 00 00 00 01 2f 03 b3 ec ff ff ff <......../...... backtrace (crc 8d877792): [<0000000052a0be73>] kmemleak_alloc+0x34/0x40 [<0000000043605459>] __kmalloc_cache_noprof+0x26c/0x2f4 [<000000006eadfab7>] vcap_rule_add_action+0x2d0/0x52c [<00000000323475d1>] vcap_api_encode_rule_test+0x4d4/0x16b0 [<00000000b3595fc4>] kunit_try_run_case+0x13c/0x3ac [<0000000010f5d2bf>] kunit_generic_run_threadfn_adapter+0x80/0xec [<00000000c5d82c9a>] kthread+0x2e8/0x374 [<00000000f4287308>] ret_from_fork+0x10/0x20 unreferenced object 0xffffff80cc0b0780 (size 64): comm "kunit_try_catch", pid 1215, jiffies 4294898265 hex dump (first 32 bytes): 00 08 0b cc 80 ff ff ff 00 07 0b cc 80 ff ff ff ................ 03 00 00 00 01 00 00 00 64 00 00 00 ec ff ff ff ........d....... backtrace (crc df76176e): [<0000000052a0be73>] kmemleak_alloc+0x34/0x40 [<0000000043605459>] __kmalloc_cache_noprof+0x26c/0x2f4 [<000000006eadfab7>] vcap_rule_add_action+0x2d0/0x52c [<000000005e4ec13f>] vcap_api_encode_rule_test+0x530/0x16b0 [<00000000b3595fc4>] kunit_try_run_case+0x13c/0x3ac [<0000000010f5d2bf>] kunit_generic_run_threadfn_adapter+0x80/0xec [<00000000c5d82c9a>] kthread+0x2e8/0x374 [<00000000f4287308>] ret_from_fork+0x10/0x20 unreferenced object 0xffffff80cc0b0800 (size 64): comm "kunit_try_catch", pid 1215, jiffies 4294898265 hex dump (first 32 bytes): 80 08 0b cc 80 ff ff ff 80 07 0b cc 80 ff ff ff ................ 29 00 00 00 01 00 00 00 01 00 00 00 ec ff ff ff )............... backtrace (crc 584e934a): [<0000000052a0be73>] kmemleak_alloc+0x34/0x40 [<0000000043605459>] __kmalloc_cache_noprof+0x26c/0x2f4 [<000000006eadfab7>] vcap_rule_add_action+0x2d0/0x52c [<00000000d9812c5f>] vcap_api_encode_rule_test+0x588/0x16b0 [<00000000b3595fc4>] kunit_try_run_case+0x13c/0x3ac [<0000000010f5d2bf>] kunit_generic_run_threadfn_adapter+0x80/0xec [<00000000c5d82c9a>] kthread+0x2e8/0x374 [<00000000f4287308>] ret_from_fork+0x10/0x20 unreferenced object 0xffffff80cc0b0880 (size 64): comm "kunit_try_catch", pid 1215, jiffies 4294898265 hex dump (first 32 bytes): 28 b7 58 ca 80 ff ff ff 00 08 0b cc 80 ff ff ff (.X............. 2a 00 00 00 01 00 00 00 01 00 00 00 ec ff ff ff *............... backtrace (crc 69b89f49): [<0000000052a0be73>] kmemleak_alloc+0x34/0x40 [<0000000043605459>] __kmalloc_cache_noprof+0x26c/0x2f4 [<000000006eadfab7>] vcap_rule_add_action+0x2d0/0x52c [<000000005fa426b8>] vcap_api_encode_rule_test+0x5e0/0x16b0 [<00000000b3595fc4>] kunit_try_run_case+0x13c/0x3ac [<0000000010f5d2bf>] kunit_generic_run_threadfn_adapter+0x80/0xec [<00000000c5d82c9a>] kthread+0x2e8/0x374 [<00000000f4287308>] ret_from_fork+0x10/0x20 unreferenced object 0xffffff80cc0b0900 (size 64): comm "kunit_try_catch", pid 1215, jiffies 4294898266 hex dump (first 32 bytes): 80 09 0b cc 80 ff ff ff 80 06 0b cc 80 ff ff ff ................ 7d 00 00 00 01 00 00 00 00 00 00 00 ff 00 00 00 }............... backtrace (crc 34181e56): [<0000000052a0be73>] kmemleak_alloc+0x34/0x40 [<0000000043605459>] __kmalloc_cache_noprof+0x26c/0x2f4 [<000000000ff63fd4>] vcap_rule_add_key+0x2cc/0x528 [<00000000991e3564>] vcap_val_rule+0xcf0/0x13e8 [<00000000fc9868e5>] vcap_api_encode_rule_test+0x678/0x16b0 [<00000000b3595fc4>] kunit_try_run_case+0x13c/0x3ac [<0000000010f5d2bf>] kunit_generic_run_threadfn_adapter+0x80/0xec [<00000000c5d82c9a>] kthread+0x2e8/0x374 [<00000000f4287308>] ret_from_fork+0x10/0x20 unreferenced object 0xffffff80cc0b0980 (size 64): comm "kunit_try_catch", pid 1215, jiffies 4294898266 hex dump (first 32 bytes): 18 b7 58 ca 80 ff ff ff 00 09 0b cc 80 ff ff ff ..X............. 67 00 00 00 00 00 00 00 01 01 74 88 c0 ff ff ff g.........t..... backtrace (crc 275fd9be): [<0000000052a0be73>] kmemleak_alloc+0x34/0x40 [<0000000043605459>] __kmalloc_cache_noprof+0x26c/0x2f4 [<000000000ff63fd4>] vcap_rule_add_key+0x2cc/0x528 [<000000001396a1a2>] test_add_def_fields+0xb0/0x100 [<000000006e7621f0>] vcap_val_rule+0xa98/0x13e8 [<00000000fc9868e5>] vcap_api_encode_rule_test+0x678/0x16b0 [<00000000b3595fc4>] kunit_try_run_case+0x13c/0x3ac [<0000000010f5d2bf>] kunit_generic_run_threadfn_adapter+0x80/0xec [<00000000c5d82c9a>] kthread+0x2e8/0x374 [<00000000f4287308>] ret_from_fork+0x10/0x20 Cc: stable@vger.kernel.org Fixes: a3c1e45156ad ("net: microchip: vcap: Fix use-after-free error in kunit test") Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com> --- drivers/net/ethernet/microchip/vcap/vcap_api_kunit.c | 2 ++ 1 file changed, 2 insertions(+)