net/enic: adjust memory check and use in proper order

Message ID tencent_1C910791FD18399E80538A08419DAB6FBE07@qq.com (mailing list archive)
State Accepted, archived
Delegated to: Ferruh Yigit
Headers
Series net/enic: adjust memory check and use in proper order |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/github-robot: build success github build: passed
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-aarch64-unit-testing success Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS
ci/iol-abi-testing warning Testing issues
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS

Commit Message

Weiguo Li Jan. 25, 2022, noon UTC
  Fixes: bb66d562aefc ("net/enic: share flow actions with same signature")

Signed-off-by: Weiguo Li <liwg06@foxmail.com>
---
 drivers/net/enic/enic_fm_flow.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

Comments

John Daley (johndale) Jan. 26, 2022, 10 p.m. UTC | #1
Reviewed-by: John Daley <johndale@cisco.com>

Thanks,
John

-----Original Message-----
From: Weiguo Li <liwg06@foxmail.com> 
Sent: Tuesday, January 25, 2022 4:01 AM
To: John Daley (johndale) <johndale@cisco.com>
Cc: dev@dpdk.org
Subject: [PATCH] net/enic: adjust memory check and use in proper order

Fixes: bb66d562aefc ("net/enic: share flow actions with same signature")

Signed-off-by: Weiguo Li <liwg06@foxmail.com>
---
 drivers/net/enic/enic_fm_flow.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/enic/enic_fm_flow.c b/drivers/net/enic/enic_fm_flow.c index bf04d714d0..d8718d17ef 100644
--- a/drivers/net/enic/enic_fm_flow.c
+++ b/drivers/net/enic/enic_fm_flow.c
@@ -2521,11 +2521,11 @@ enic_action_handle_get(struct enic_flowman *fm, struct fm_action *action_in,
 		memcpy(fma, action_in, sizeof(*fma));
 
 		ah = calloc(1, sizeof(*ah));
-		memcpy(&ah->key, action_in, sizeof(struct fm_action));
 		if (ah == NULL)
 			return rte_flow_error_set(error, ENOMEM,
 					   RTE_FLOW_ERROR_TYPE_HANDLE,
 					   NULL, "enic: calloc(fm-action)");
+		memcpy(&ah->key, action_in, sizeof(struct fm_action));
 		args[0] = FM_ACTION_ALLOC;
 		args[1] = fm->cmd.pa;
 		ret = flowman_cmd(fm, args, 2);
--
2.25.1
  
Ferruh Yigit Jan. 28, 2022, 2:47 p.m. UTC | #2
On 1/26/2022 10:00 PM, John Daley (johndale) wrote:

<...>

> Thanks,
> John
> 
> -----Original Message-----
> From: Weiguo Li <liwg06@foxmail.com>
> Sent: Tuesday, January 25, 2022 4:01 AM
> To: John Daley (johndale) <johndale@cisco.com>
> Cc: dev@dpdk.org
> Subject: [PATCH] net/enic: adjust memory check and use in proper order
> 
> Fixes: bb66d562aefc ("net/enic: share flow actions with same signature")
> 
> Signed-off-by: Weiguo Li <liwg06@foxmail.com>
> 
> Reviewed-by: John Daley <johndale@cisco.com>
> 
Applied to dpdk-next-net/main, thanks.
  
Thomas Monjalon Feb. 2, 2022, 11:50 a.m. UTC | #3
> > Subject: [PATCH] net/enic: adjust memory check and use in proper order
> > 
> > Fixes: bb66d562aefc ("net/enic: share flow actions with same signature")
> > 
> > Signed-off-by: Weiguo Li <liwg06@foxmail.com>
> > 
> > Reviewed-by: John Daley <johndale@cisco.com>
> > 
> Applied to dpdk-next-net/main, thanks.

Please could we have a little of explanation in the commit log?
The title says "adjust [...] and use in proper order"
but I see only a move of the check.
  
Weiguo Li Feb. 3, 2022, 8:26 a.m. UTC | #4
Hi Monjalon,

This fix only moves the memcpy after to null check.
so the misplaced of memcpy and null check is reordered.

I thought maybe it's too trival a fix to add more explanation.

Sorry if it makes any confusion.
(last mail is garbled in format)

-Weiguo

> -----Original Message-----
> From: "Thomas Monjalon" <thomas@monjalon.net>;
> Date: Wed, Feb 2, 2022 07:50 PM
> To: "John Daley (johndale)"<johndale@cisco.com>;"Weiguo Li"<liwg06@foxmail.com>;
> Cc: "dev"<dev@dpdk.org>;"Ferruh Yigit"<ferruh.yigit@intel.com>;
> Subject: Re: [PATCH] net/enic: adjust memory check and use in proper order
>
> > > Subject: [PATCH] net/enic: adjust memory check and use in proper order
> > >
> > > Fixes: bb66d562aefc ("net/enic: share flow actions with same signature")
> > >
> > > Signed-off-by: Weiguo Li <liwg06@foxmail.com>
> > >
> > > Reviewed-by: John Daley <johndale@cisco.com>
> > >
> > > Applied to dpdk-next-net/main, thanks.

> Please could we have a little of explanation in the commit log?
> The title says "adjust [...] and use in proper order"
> but I see only a move of the check.
  
Ferruh Yigit Feb. 3, 2022, 10:57 a.m. UTC | #5
On 2/2/2022 11:50 AM, Thomas Monjalon wrote:
>>> Subject: [PATCH] net/enic: adjust memory check and use in proper order
>>>
>>> Fixes: bb66d562aefc ("net/enic: share flow actions with same signature")
>>>
>>> Signed-off-by: Weiguo Li <liwg06@foxmail.com>
>>>
>>> Reviewed-by: John Daley <johndale@cisco.com>
>>>
>> Applied to dpdk-next-net/main, thanks.
> 
> Please could we have a little of explanation in the commit log?
> The title says "adjust [...] and use in proper order"
> but I see only a move of the check.
> 
> 

Is following good? If so I can update it in next-net.

     net/enic: fix dereference before null check
     
     Move memcpy to 'ah->key' after 'ah' null check
     
     Fixes: bb66d562aefc ("net/enic: share flow actions with same signature")
     Cc: stable@dpdk.org
  
Thomas Monjalon Feb. 3, 2022, 11:11 a.m. UTC | #6
03/02/2022 11:57, Ferruh Yigit:
> On 2/2/2022 11:50 AM, Thomas Monjalon wrote:
> >>> Subject: [PATCH] net/enic: adjust memory check and use in proper order
> >>>
> >>> Fixes: bb66d562aefc ("net/enic: share flow actions with same signature")
> >>>
> >>> Signed-off-by: Weiguo Li <liwg06@foxmail.com>
> >>>
> >>> Reviewed-by: John Daley <johndale@cisco.com>
> >>>
> >> Applied to dpdk-next-net/main, thanks.
> > 
> > Please could we have a little of explanation in the commit log?
> > The title says "adjust [...] and use in proper order"
> > but I see only a move of the check.
> > 
> > 
> 
> Is following good? If so I can update it in next-net.
> 
>      net/enic: fix dereference before null check
>      
>      Move memcpy to 'ah->key' after 'ah' null check
>      
>      Fixes: bb66d562aefc ("net/enic: share flow actions with same signature")
>      Cc: stable@dpdk.org

Looks better to me, thanks.
  
Ferruh Yigit Feb. 3, 2022, 11:22 a.m. UTC | #7
On 2/3/2022 11:11 AM, Thomas Monjalon wrote:
> 03/02/2022 11:57, Ferruh Yigit:
>> On 2/2/2022 11:50 AM, Thomas Monjalon wrote:
>>>>> Subject: [PATCH] net/enic: adjust memory check and use in proper order
>>>>>
>>>>> Fixes: bb66d562aefc ("net/enic: share flow actions with same signature")
>>>>>
>>>>> Signed-off-by: Weiguo Li <liwg06@foxmail.com>
>>>>>
>>>>> Reviewed-by: John Daley <johndale@cisco.com>
>>>>>
>>>> Applied to dpdk-next-net/main, thanks.
>>>
>>> Please could we have a little of explanation in the commit log?
>>> The title says "adjust [...] and use in proper order"
>>> but I see only a move of the check.
>>>
>>>
>>
>> Is following good? If so I can update it in next-net.
>>
>>       net/enic: fix dereference before null check
>>       
>>       Move memcpy to 'ah->key' after 'ah' null check
>>       
>>       Fixes: bb66d562aefc ("net/enic: share flow actions with same signature")
>>       Cc: stable@dpdk.org
> 
> Looks better to me, thanks.
> 
> 

Updated in next-net, thanks.
  

Patch

diff --git a/drivers/net/enic/enic_fm_flow.c b/drivers/net/enic/enic_fm_flow.c
index bf04d714d0..d8718d17ef 100644
--- a/drivers/net/enic/enic_fm_flow.c
+++ b/drivers/net/enic/enic_fm_flow.c
@@ -2521,11 +2521,11 @@  enic_action_handle_get(struct enic_flowman *fm, struct fm_action *action_in,
 		memcpy(fma, action_in, sizeof(*fma));
 
 		ah = calloc(1, sizeof(*ah));
-		memcpy(&ah->key, action_in, sizeof(struct fm_action));
 		if (ah == NULL)
 			return rte_flow_error_set(error, ENOMEM,
 					   RTE_FLOW_ERROR_TYPE_HANDLE,
 					   NULL, "enic: calloc(fm-action)");
+		memcpy(&ah->key, action_in, sizeof(struct fm_action));
 		args[0] = FM_ACTION_ALLOC;
 		args[1] = fm->cmd.pa;
 		ret = flowman_cmd(fm, args, 2);