1. Patchew
  2. linux
  3. View series

[PATCH] net: microchip: vcap api: Fix memory leaks in vcap_api_encode_rule_test()

Jinjie Ruan posted 1 patch 1 month, 2 weeks ago
There is a newer version of this series
drivers/net/ethernet/microchip/vcap/vcap_api_kunit.c | 2 ++
1 file changed, 2 insertions(+)
[PATCH] net: microchip: vcap api: Fix memory leaks in vcap_api_encode_rule_test()
Posted by Jinjie Ruan 1 month, 2 weeks ago 
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(+)

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)
-- 
2.34.1
Re: [PATCH] net: microchip: vcap api: Fix memory leaks in vcap_api_encode_rule_test()
Posted by Jens Emil Schulz Østergaard 1 month, 2 weeks ago 
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>
Re: [PATCH] net: microchip: vcap api: Fix memory leaks in vcap_api_encode_rule_test()
Posted by Daniel Machon 1 month, 2 weeks ago 
> 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
>
Re: [PATCH] net: microchip: vcap api: Fix memory leaks in vcap_api_encode_rule_test()
Posted by Jens Emil Schulz Østergaard 1 month, 2 weeks ago 
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
Re: [PATCH] net: microchip: vcap api: Fix memory leaks in vcap_api_encode_rule_test()
Posted by Daniel Machon 1 month, 2 weeks ago 
> >
> > 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
Re: [PATCH] net: microchip: vcap api: Fix memory leaks in vcap_api_encode_rule_test()
Posted by Simon Horman 1 month, 2 weeks ago 
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>

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.