All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH bpf-next] selftests/xsk: fix double free
@ 2022-09-29  9:01 Magnus Karlsson
  2022-09-29 10:44 ` Maciej Fijalkowski
  2022-09-30 22:50 ` patchwork-bot+netdevbpf
  0 siblings, 2 replies; 6+ messages in thread
From: Magnus Karlsson @ 2022-09-29  9:01 UTC (permalink / raw)
  To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski
  Cc: jonathan.lemon, bpf

From: Magnus Karlsson <magnus.karlsson@intel.com>

Fix a double free at exit of the test suite.

Fixes: a693ff3ed561 ("selftests/xsk: Add support for executing tests on physical device")
Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
---
 tools/testing/selftests/bpf/xskxceiver.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
index ef33309bbe49..d1a5f3218c34 100644
--- a/tools/testing/selftests/bpf/xskxceiver.c
+++ b/tools/testing/selftests/bpf/xskxceiver.c
@@ -1953,9 +1953,6 @@ int main(int argc, char **argv)
 
 	pkt_stream_delete(tx_pkt_stream_default);
 	pkt_stream_delete(rx_pkt_stream_default);
-	free(ifobj_rx->umem);
-	if (!ifobj_tx->shared_umem)
-		free(ifobj_tx->umem);
 	ifobject_delete(ifobj_tx);
 	ifobject_delete(ifobj_rx);
 

base-commit: 8526f0d6135f77451566463ace6f0fb8b72cedaa
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next] selftests/xsk: fix double free
  2022-09-29  9:01 [PATCH bpf-next] selftests/xsk: fix double free Magnus Karlsson
@ 2022-09-29 10:44 ` Maciej Fijalkowski
  2022-09-30  0:52   ` Martin KaFai Lau
  2022-09-30 22:50 ` patchwork-bot+netdevbpf
  1 sibling, 1 reply; 6+ messages in thread
From: Maciej Fijalkowski @ 2022-09-29 10:44 UTC (permalink / raw)
  To: Magnus Karlsson
  Cc: magnus.karlsson, bjorn, ast, daniel, netdev, jonathan.lemon, bpf

On Thu, Sep 29, 2022 at 11:01:33AM +0200, Magnus Karlsson wrote:
> From: Magnus Karlsson <magnus.karlsson@intel.com>
> 
> Fix a double free at exit of the test suite.
> 
> Fixes: a693ff3ed561 ("selftests/xsk: Add support for executing tests on physical device")
> Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
> ---
>  tools/testing/selftests/bpf/xskxceiver.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
> index ef33309bbe49..d1a5f3218c34 100644
> --- a/tools/testing/selftests/bpf/xskxceiver.c
> +++ b/tools/testing/selftests/bpf/xskxceiver.c
> @@ -1953,9 +1953,6 @@ int main(int argc, char **argv)
>  
>  	pkt_stream_delete(tx_pkt_stream_default);
>  	pkt_stream_delete(rx_pkt_stream_default);
> -	free(ifobj_rx->umem);
> -	if (!ifobj_tx->shared_umem)
> -		free(ifobj_tx->umem);
>  	ifobject_delete(ifobj_tx);
>  	ifobject_delete(ifobj_rx);

So basically we free this inside ifobject_delete().

Acked-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>

>  
> 
> base-commit: 8526f0d6135f77451566463ace6f0fb8b72cedaa
> -- 
> 2.34.1
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next] selftests/xsk: fix double free
  2022-09-29 10:44 ` Maciej Fijalkowski
@ 2022-09-30  0:52   ` Martin KaFai Lau
  2022-09-30  7:52     ` Magnus Karlsson
  0 siblings, 1 reply; 6+ messages in thread
From: Martin KaFai Lau @ 2022-09-30  0:52 UTC (permalink / raw)
  To: Maciej Fijalkowski, Magnus Karlsson
  Cc: magnus.karlsson, bjorn, ast, daniel, netdev, jonathan.lemon, bpf

On 9/29/22 3:44 AM, Maciej Fijalkowski wrote:
> On Thu, Sep 29, 2022 at 11:01:33AM +0200, Magnus Karlsson wrote:
>> From: Magnus Karlsson <magnus.karlsson@intel.com>
>>
>> Fix a double free at exit of the test suite.
>>
>> Fixes: a693ff3ed561 ("selftests/xsk: Add support for executing tests on physical device")
>> Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
>> ---
>>   tools/testing/selftests/bpf/xskxceiver.c | 3 ---
>>   1 file changed, 3 deletions(-)
>>
>> diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
>> index ef33309bbe49..d1a5f3218c34 100644
>> --- a/tools/testing/selftests/bpf/xskxceiver.c
>> +++ b/tools/testing/selftests/bpf/xskxceiver.c
>> @@ -1953,9 +1953,6 @@ int main(int argc, char **argv)
>>   
>>   	pkt_stream_delete(tx_pkt_stream_default);
>>   	pkt_stream_delete(rx_pkt_stream_default);
>> -	free(ifobj_rx->umem);
>> -	if (!ifobj_tx->shared_umem)
shared_umem means ifobj_rx->umem and ifobj_tx->umem are the same?  No special 
handling is needed and ifobject_delete() will handle it?

>> -		free(ifobj_tx->umem);
>>   	ifobject_delete(ifobj_tx);
>>   	ifobject_delete(ifobj_rx);
> 
> So basically we free this inside ifobject_delete().


^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next] selftests/xsk: fix double free
  2022-09-30  0:52   ` Martin KaFai Lau
@ 2022-09-30  7:52     ` Magnus Karlsson
  2022-09-30 14:31       ` Magnus Karlsson
  0 siblings, 1 reply; 6+ messages in thread
From: Magnus Karlsson @ 2022-09-30  7:52 UTC (permalink / raw)
  To: Martin KaFai Lau
  Cc: Maciej Fijalkowski, magnus.karlsson, bjorn, ast, daniel, netdev,
	jonathan.lemon, bpf

On Fri, Sep 30, 2022 at 2:52 AM Martin KaFai Lau <martin.lau@linux.dev> wrote:
>
> On 9/29/22 3:44 AM, Maciej Fijalkowski wrote:
> > On Thu, Sep 29, 2022 at 11:01:33AM +0200, Magnus Karlsson wrote:
> >> From: Magnus Karlsson <magnus.karlsson@intel.com>
> >>
> >> Fix a double free at exit of the test suite.
> >>
> >> Fixes: a693ff3ed561 ("selftests/xsk: Add support for executing tests on physical device")
> >> Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
> >> ---
> >>   tools/testing/selftests/bpf/xskxceiver.c | 3 ---
> >>   1 file changed, 3 deletions(-)
> >>
> >> diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
> >> index ef33309bbe49..d1a5f3218c34 100644
> >> --- a/tools/testing/selftests/bpf/xskxceiver.c
> >> +++ b/tools/testing/selftests/bpf/xskxceiver.c
> >> @@ -1953,9 +1953,6 @@ int main(int argc, char **argv)
> >>
> >>      pkt_stream_delete(tx_pkt_stream_default);
> >>      pkt_stream_delete(rx_pkt_stream_default);
> >> -    free(ifobj_rx->umem);
> >> -    if (!ifobj_tx->shared_umem)
> shared_umem means ifobj_rx->umem and ifobj_tx->umem are the same?  No special
> handling is needed and ifobject_delete() will handle it?

You are correct, we will still have a double free in that case. Thanks
for spotting. Will send a v2.

> >> -            free(ifobj_tx->umem);
> >>      ifobject_delete(ifobj_tx);
> >>      ifobject_delete(ifobj_rx);
> >
> > So basically we free this inside ifobject_delete().
>

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next] selftests/xsk: fix double free
  2022-09-30  7:52     ` Magnus Karlsson
@ 2022-09-30 14:31       ` Magnus Karlsson
  0 siblings, 0 replies; 6+ messages in thread
From: Magnus Karlsson @ 2022-09-30 14:31 UTC (permalink / raw)
  To: Martin KaFai Lau
  Cc: Maciej Fijalkowski, magnus.karlsson, bjorn, ast, daniel, netdev,
	jonathan.lemon, bpf

On Fri, Sep 30, 2022 at 9:52 AM Magnus Karlsson
<magnus.karlsson@gmail.com> wrote:
>
> On Fri, Sep 30, 2022 at 2:52 AM Martin KaFai Lau <martin.lau@linux.dev> wrote:
> >
> > On 9/29/22 3:44 AM, Maciej Fijalkowski wrote:
> > > On Thu, Sep 29, 2022 at 11:01:33AM +0200, Magnus Karlsson wrote:
> > >> From: Magnus Karlsson <magnus.karlsson@intel.com>
> > >>
> > >> Fix a double free at exit of the test suite.
> > >>
> > >> Fixes: a693ff3ed561 ("selftests/xsk: Add support for executing tests on physical device")
> > >> Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
> > >> ---
> > >>   tools/testing/selftests/bpf/xskxceiver.c | 3 ---
> > >>   1 file changed, 3 deletions(-)
> > >>
> > >> diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
> > >> index ef33309bbe49..d1a5f3218c34 100644
> > >> --- a/tools/testing/selftests/bpf/xskxceiver.c
> > >> +++ b/tools/testing/selftests/bpf/xskxceiver.c
> > >> @@ -1953,9 +1953,6 @@ int main(int argc, char **argv)
> > >>
> > >>      pkt_stream_delete(tx_pkt_stream_default);
> > >>      pkt_stream_delete(rx_pkt_stream_default);
> > >> -    free(ifobj_rx->umem);
> > >> -    if (!ifobj_tx->shared_umem)
> > shared_umem means ifobj_rx->umem and ifobj_tx->umem are the same?  No special
> > handling is needed and ifobject_delete() will handle it?
>
> You are correct, we will still have a double free in that case. Thanks
> for spotting. Will send a v2.

Sorry, but I have to take my statement back. The v1 is actually
correct. The umem structure is unconditionally allocated in
ifobject_create(). Later when setting up the shared_umem, the
information from one of them is copied over to the other, except for
some information that is changed for the second umem structure. So the
v1 still stands.

> > >> -            free(ifobj_tx->umem);
> > >>      ifobject_delete(ifobj_tx);
> > >>      ifobject_delete(ifobj_rx);
> > >
> > > So basically we free this inside ifobject_delete().
> >

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next] selftests/xsk: fix double free
  2022-09-29  9:01 [PATCH bpf-next] selftests/xsk: fix double free Magnus Karlsson
  2022-09-29 10:44 ` Maciej Fijalkowski
@ 2022-09-30 22:50 ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 6+ messages in thread
From: patchwork-bot+netdevbpf @ 2022-09-30 22:50 UTC (permalink / raw)
  To: Magnus Karlsson
  Cc: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski,
	jonathan.lemon, bpf

Hello:

This patch was applied to bpf/bpf-next.git (master)
by Andrii Nakryiko <andrii@kernel.org>:

On Thu, 29 Sep 2022 11:01:33 +0200 you wrote:
> From: Magnus Karlsson <magnus.karlsson@intel.com>
> 
> Fix a double free at exit of the test suite.
> 
> Fixes: a693ff3ed561 ("selftests/xsk: Add support for executing tests on physical device")
> Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
> 
> [...]

Here is the summary with links:
  - [bpf-next] selftests/xsk: fix double free
    https://git.kernel.org/bpf/bpf-next/c/5f388bba7acb

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-09-30 22:50 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-29  9:01 [PATCH bpf-next] selftests/xsk: fix double free Magnus Karlsson
2022-09-29 10:44 ` Maciej Fijalkowski
2022-09-30  0:52   ` Martin KaFai Lau
2022-09-30  7:52     ` Magnus Karlsson
2022-09-30 14:31       ` Magnus Karlsson
2022-09-30 22:50 ` patchwork-bot+netdevbpf

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.