linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] net: prestera: acl: Add check for kmemdup
@ 2022-09-28  9:20 Jiasheng Jiang
  2022-09-29 16:15 ` Jakub Kicinski
  0 siblings, 1 reply; 11+ messages in thread
From: Jiasheng Jiang @ 2022-09-28  9:20 UTC (permalink / raw)
  To: tchornyi, davem, edumazet, kuba, pabeni
  Cc: netdev, linux-kernel, Jiasheng Jiang

As the kemdup could return NULL, it should be better to check the return
value and return error if fails.
Moreover, the return value of prestera_acl_ruleset_keymask_set() should
be checked by cascade.

Fixes: 604ba230902d ("net: prestera: flower template support")
Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn>
---
 drivers/net/ethernet/marvell/prestera/prestera_acl.c    | 8 ++++++--
 drivers/net/ethernet/marvell/prestera/prestera_acl.h    | 4 ++--
 drivers/net/ethernet/marvell/prestera/prestera_flower.c | 6 +++++-
 3 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/marvell/prestera/prestera_acl.c b/drivers/net/ethernet/marvell/prestera/prestera_acl.c
index 3d4b85f2d541..f6b2933859d0 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_acl.c
+++ b/drivers/net/ethernet/marvell/prestera/prestera_acl.c
@@ -178,10 +178,14 @@ prestera_acl_ruleset_create(struct prestera_acl *acl,
 	return ERR_PTR(err);
 }
 
-void prestera_acl_ruleset_keymask_set(struct prestera_acl_ruleset *ruleset,
-				      void *keymask)
+int prestera_acl_ruleset_keymask_set(struct prestera_acl_ruleset *ruleset,
+				     void *keymask)
 {
 	ruleset->keymask = kmemdup(keymask, ACL_KEYMASK_SIZE, GFP_KERNEL);
+	if (!ruleset->keymask)
+		return -ENOMEM;
+
+	return 0;
 }
 
 int prestera_acl_ruleset_offload(struct prestera_acl_ruleset *ruleset)
diff --git a/drivers/net/ethernet/marvell/prestera/prestera_acl.h b/drivers/net/ethernet/marvell/prestera/prestera_acl.h
index 03fc5b9dc925..131bfbc87cd7 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_acl.h
+++ b/drivers/net/ethernet/marvell/prestera/prestera_acl.h
@@ -185,8 +185,8 @@ struct prestera_acl_ruleset *
 prestera_acl_ruleset_lookup(struct prestera_acl *acl,
 			    struct prestera_flow_block *block,
 			    u32 chain_index);
-void prestera_acl_ruleset_keymask_set(struct prestera_acl_ruleset *ruleset,
-				      void *keymask);
+int prestera_acl_ruleset_keymask_set(struct prestera_acl_ruleset *ruleset,
+				     void *keymask);
 bool prestera_acl_ruleset_is_offload(struct prestera_acl_ruleset *ruleset);
 int prestera_acl_ruleset_offload(struct prestera_acl_ruleset *ruleset);
 void prestera_acl_ruleset_put(struct prestera_acl_ruleset *ruleset);
diff --git a/drivers/net/ethernet/marvell/prestera/prestera_flower.c b/drivers/net/ethernet/marvell/prestera/prestera_flower.c
index 19d3b55c578e..cf551a8379ac 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_flower.c
+++ b/drivers/net/ethernet/marvell/prestera/prestera_flower.c
@@ -452,7 +452,9 @@ int prestera_flower_tmplt_create(struct prestera_flow_block *block,
 	}
 
 	/* preserve keymask/template to this ruleset */
-	prestera_acl_ruleset_keymask_set(ruleset, rule.re_key.match.mask);
+	err = prestera_acl_ruleset_keymask_set(ruleset, rule.re_key.match.mask);
+	if (err)
+		goto err_ruleset_keymask_set;
 
 	/* skip error, as it is not possible to reject template operation,
 	 * so, keep the reference to the ruleset for rules to be added
@@ -468,6 +470,8 @@ int prestera_flower_tmplt_create(struct prestera_flow_block *block,
 	list_add_rcu(&template->list, &block->template_list);
 	return 0;
 
+err_ruleset_keymask_set:
+	prestera_acl_ruleset_put(ruleset);
 err_ruleset_get:
 	kfree(template);
 err_malloc:
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] net: prestera: acl: Add check for kmemdup
  2022-09-28  9:20 [PATCH] net: prestera: acl: Add check for kmemdup Jiasheng Jiang
@ 2022-09-29 16:15 ` Jakub Kicinski
  0 siblings, 0 replies; 11+ messages in thread
From: Jakub Kicinski @ 2022-09-29 16:15 UTC (permalink / raw)
  To: Jiasheng Jiang; +Cc: tchornyi, davem, edumazet, pabeni, netdev, linux-kernel

On Wed, 28 Sep 2022 17:20:24 +0800 Jiasheng Jiang wrote:
> As the kemdup could return NULL, it should be better to check the return
> value and return error if fails.
> Moreover, the return value of prestera_acl_ruleset_keymask_set() should
> be checked by cascade.
> 
> Fixes: 604ba230902d ("net: prestera: flower template support")
> Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn>

You must CC the authors of patch you're fixing. 
get_maintainer will do that for you I don't understand why people can't
simply run that script :/ You CC linux-kernel for no apparent reason
yet you don't CC the guy who wrote the original patch.
If you could please explain what is going on maybe we can improve the
tooling or something.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] net: prestera: acl: Add check for kmemdup
  2022-09-30  4:48 Jiasheng Jiang
  2022-09-30  9:20 ` Taras Chornyi
@ 2022-10-03 11:40 ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 11+ messages in thread
From: patchwork-bot+netdevbpf @ 2022-10-03 11:40 UTC (permalink / raw)
  To: Jiasheng Jiang
  Cc: vmytnyk, davem, tchornyi, edumazet, kuba, pabeni, netdev, linux-kernel

Hello:

This patch was applied to netdev/net.git (master)
by David S. Miller <davem@davemloft.net>:

On Fri, 30 Sep 2022 12:48:43 +0800 you wrote:
> As the kemdup could return NULL, it should be better to check the return
> value and return error if fails.
> Moreover, the return value of prestera_acl_ruleset_keymask_set() should
> be checked by cascade.
> 
> Fixes: 604ba230902d ("net: prestera: flower template support")
> Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn>
> 
> [...]

Here is the summary with links:
  - net: prestera: acl: Add check for kmemdup
    https://git.kernel.org/netdev/net/c/9e6fd874c7bb

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] 11+ messages in thread

* Re: [PATCH] net: prestera: acl: Add check for kmemdup
  2022-09-30 16:58         ` Jakub Kicinski
@ 2022-09-30 18:28           ` Joe Perches
  0 siblings, 0 replies; 11+ messages in thread
From: Joe Perches @ 2022-09-30 18:28 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Jiasheng Jiang, pabeni, davem, tchornyi, edumazet, netdev,
	linux-kernel, Volodymyr Mytnyk, Greg Kroah-Hartman

On Fri, 2022-09-30 at 09:58 -0700, Jakub Kicinski wrote:
> On Fri, 30 Sep 2022 09:43:54 -0700 Joe Perches wrote:
> > > > There's no great way to identify "author" or "original submitter"
> > > > and frequently the "original submitter" isn't a maintainer anyway.  
> > > 
> > > Confusing sentence. We want for people who s-o-b'd the commit under
> > > Fixes to be CCed.  
> > 
> > If a file or a file modified by a patch is listed in the MAINTAINERS,
> > git history isn't used unless --git is specified.
> > 
> > For a patch, maybe the author and other SOBs of a commit specified
> > by a "Fixes:" line SHA-1 in the commit message could be added automatically.
> 
> Yes, git history isn't used, but the Fixes tag are consulted already
> AFAICT. We just need to steer people towards running the script on 
> the patch.
> 
> $ git format-patch net/main~..net/main -o /tmp/
> /tmp/0001-eth-alx-take-rtnl_lock-on-resume.patch
> 
> $ grep Fixes /tmp/0001-eth-alx-take-rtnl_lock-on-resume.patch
> Fixes: 4a5fe57e7751 ("alx: use fine-grained locking instead of RTNL")
> 
> $ git show 4a5fe57e7751 --pretty='%an <%ae>' --no-patch 
> Johannes Berg <johannes@sipsolutions.net>
> 
> $ ./scripts/get_maintainer.pl  /tmp/0001-eth-alx-take-rtnl_lock-on-resume.patch | grep blame
> "David S. Miller" <davem@davemloft.net> (maintainer:NETWORKING DRIVERS,commit_signer:2/4=50%,blamed_fixes:1/1=100%)
> Johannes Berg <johannes@sipsolutions.net> (blamed_fixes:1/1=100%)

Yeah, you kinda ruined my reveal.  It already does that.

Of course I did that 3 years ago and I forgot about it.

cheers, Joe

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] net: prestera: acl: Add check for kmemdup
  2022-09-30 16:43       ` Joe Perches
@ 2022-09-30 16:58         ` Jakub Kicinski
  2022-09-30 18:28           ` Joe Perches
  0 siblings, 1 reply; 11+ messages in thread
From: Jakub Kicinski @ 2022-09-30 16:58 UTC (permalink / raw)
  To: Joe Perches
  Cc: Jiasheng Jiang, pabeni, davem, tchornyi, edumazet, netdev,
	linux-kernel, Volodymyr Mytnyk, Greg Kroah-Hartman

On Fri, 30 Sep 2022 09:43:54 -0700 Joe Perches wrote:
> > > There's no great way to identify "author" or "original submitter"
> > > and frequently the "original submitter" isn't a maintainer anyway.  
> > 
> > Confusing sentence. We want for people who s-o-b'd the commit under
> > Fixes to be CCed.  
> 
> If a file or a file modified by a patch is listed in the MAINTAINERS,
> git history isn't used unless --git is specified.
> 
> For a patch, maybe the author and other SOBs of a commit specified
> by a "Fixes:" line SHA-1 in the commit message could be added automatically.

Yes, git history isn't used, but the Fixes tag are consulted already
AFAICT. We just need to steer people towards running the script on 
the patch.

$ git format-patch net/main~..net/main -o /tmp/
/tmp/0001-eth-alx-take-rtnl_lock-on-resume.patch

$ grep Fixes /tmp/0001-eth-alx-take-rtnl_lock-on-resume.patch
Fixes: 4a5fe57e7751 ("alx: use fine-grained locking instead of RTNL")

$ git show 4a5fe57e7751 --pretty='%an <%ae>' --no-patch 
Johannes Berg <johannes@sipsolutions.net>

$ ./scripts/get_maintainer.pl  /tmp/0001-eth-alx-take-rtnl_lock-on-resume.patch | grep blame
"David S. Miller" <davem@davemloft.net> (maintainer:NETWORKING DRIVERS,commit_signer:2/4=50%,blamed_fixes:1/1=100%)
Johannes Berg <johannes@sipsolutions.net> (blamed_fixes:1/1=100%)

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] net: prestera: acl: Add check for kmemdup
  2022-09-30 15:44     ` Jakub Kicinski
@ 2022-09-30 16:43       ` Joe Perches
  2022-09-30 16:58         ` Jakub Kicinski
  0 siblings, 1 reply; 11+ messages in thread
From: Joe Perches @ 2022-09-30 16:43 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Jiasheng Jiang, pabeni, davem, tchornyi, edumazet, netdev,
	linux-kernel, Volodymyr Mytnyk, Greg Kroah-Hartman

On Fri, 2022-09-30 at 08:44 -0700, Jakub Kicinski wrote:
> On Fri, 30 Sep 2022 08:20:47 -0700 Joe Perches wrote:
> > IMO: If Volodymyr wants to be a maintainer here, he should put
> > his email as an entry in the MAINTAINERS file for the subsystem.
> 
> It's about Fixes tags, unfortunately having everyone of note listed 
> in MAINTAINERS is pretty much impossible. Even tho we are trying.
> 
> > > > Maybe there is a problem of the script that misses one.  
> > 
> > I don't think so.  Maybe you have more evidence...
> 
> I'll CC you when I tell people to CC authors of patches under Fixes
> going forward, I don't have a list going back.
> 
> > > > Anyway, I have already submitted the same patch and added
> > > > "vmytnyk@marvell.com" this time.  
> > > 
> > > Ha! So you do indeed use it in a way I wasn't expecting :S
> > > Thanks for the explanation.
> > > 
> > > Joe, would you be okay to add a "big fat warning" to get_maintainer
> > > when people try to use the -f flag?  
> > 
> > No, not really.  -f isn't required when the file is in git anyway.
> 
> Ah. Yeah. I'd make it error out when run on a source file without -f :S
> 
> > > Maybe we can also change the message
> > > that's displayed when the script is run without arguments to not
> > > mention -f?  
> > 
> > I think that's a poor idea as frequently the script isn't used
> > on patches but simply to identify the maintainers of a particular
> > file or subsystem.
> 
> Identify the maintainers and report a bug, or something else? As a
> maintainer I can tell you that I don't see bug reports as often as I
> see trivial patches from noobs which miss CCs. And I personally don't
> think I ever used get_maintainer on anything else than a patch.
> 
> > > We're getting quite a few fixes which don't CC author, I'm guessing
> > > Jiasheng's approach may be a common one.  
> > 
> > There's no great way to identify "author" or "original submitter"
> > and frequently the "original submitter" isn't a maintainer anyway.
> 
> Confusing sentence. We want for people who s-o-b'd the commit under
> Fixes to be CCed.

If a file or a file modified by a patch is listed in the MAINTAINERS,
git history isn't used unless --git is specified.

For a patch, maybe the author and other SOBs of a commit specified
by a "Fixes:" line SHA-1 in the commit message could be added automatically.



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] net: prestera: acl: Add check for kmemdup
  2022-09-30 15:20   ` Joe Perches
@ 2022-09-30 15:44     ` Jakub Kicinski
  2022-09-30 16:43       ` Joe Perches
  0 siblings, 1 reply; 11+ messages in thread
From: Jakub Kicinski @ 2022-09-30 15:44 UTC (permalink / raw)
  To: Joe Perches
  Cc: Jiasheng Jiang, pabeni, davem, tchornyi, edumazet, netdev,
	linux-kernel, Volodymyr Mytnyk, Greg Kroah-Hartman

On Fri, 30 Sep 2022 08:20:47 -0700 Joe Perches wrote:
> IMO: If Volodymyr wants to be a maintainer here, he should put
> his email as an entry in the MAINTAINERS file for the subsystem.

It's about Fixes tags, unfortunately having everyone of note listed 
in MAINTAINERS is pretty much impossible. Even tho we are trying.

> > > Maybe there is a problem of the script that misses one.  
> 
> I don't think so.  Maybe you have more evidence...

I'll CC you when I tell people to CC authors of patches under Fixes
going forward, I don't have a list going back.

> > > Anyway, I have already submitted the same patch and added
> > > "vmytnyk@marvell.com" this time.  
> > 
> > Ha! So you do indeed use it in a way I wasn't expecting :S
> > Thanks for the explanation.
> > 
> > Joe, would you be okay to add a "big fat warning" to get_maintainer
> > when people try to use the -f flag?  
> 
> No, not really.  -f isn't required when the file is in git anyway.

Ah. Yeah. I'd make it error out when run on a source file without -f :S

> > Maybe we can also change the message
> > that's displayed when the script is run without arguments to not
> > mention -f?  
> 
> I think that's a poor idea as frequently the script isn't used
> on patches but simply to identify the maintainers of a particular
> file or subsystem.

Identify the maintainers and report a bug, or something else? As a
maintainer I can tell you that I don't see bug reports as often as I
see trivial patches from noobs which miss CCs. And I personally don't
think I ever used get_maintainer on anything else than a patch.

> > We're getting quite a few fixes which don't CC author, I'm guessing
> > Jiasheng's approach may be a common one.  
> 
> There's no great way to identify "author" or "original submitter"
> and frequently the "original submitter" isn't a maintainer anyway.

Confusing sentence. We want for people who s-o-b'd the commit under
Fixes to be CCed.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] net: prestera: acl: Add check for kmemdup
  2022-09-30 14:29 ` Jakub Kicinski
@ 2022-09-30 15:20   ` Joe Perches
  2022-09-30 15:44     ` Jakub Kicinski
  0 siblings, 1 reply; 11+ messages in thread
From: Joe Perches @ 2022-09-30 15:20 UTC (permalink / raw)
  To: Jakub Kicinski, Jiasheng Jiang
  Cc: pabeni, davem, tchornyi, edumazet, netdev, linux-kernel,
	Volodymyr Mytnyk

On Fri, 2022-09-30 at 07:29 -0700, Jakub Kicinski wrote:
> On Fri, 30 Sep 2022 13:03:17 +0800 Jiasheng Jiang wrote:
> > Actually, I used get_maintainer scripts and the results is as follow:
> > "./scripts/get_maintainer.pl -f drivers/net/ethernet/marvell/prestera/prestera_acl.c"
> > Taras Chornyi <tchornyi@marvell.com> (supporter:MARVELL PRESTERA ETHERNET SWITCH DRIVER)
> > "David S. Miller" <davem@davemloft.net> (maintainer:NETWORKING DRIVERS)
> > Eric Dumazet <edumazet@google.com> (maintainer:NETWORKING DRIVERS)
> > Jakub Kicinski <kuba@kernel.org> (maintainer:NETWORKING DRIVERS)
> > Paolo Abeni <pabeni@redhat.com> (maintainer:NETWORKING DRIVERS)
> > netdev@vger.kernel.org (open list:NETWORKING DRIVERS)
> > linux-kernel@vger.kernel.org (open list)
> > 
> > Therefore, I submitted my patch to the above addresses.
> > 
> > And this time I checked the fixes commit, and found that it has two
> > authors:
> > Signed-off-by: Volodymyr Mytnyk <vmytnyk@marvell.com>
> > Signed-off-by: David S. Miller <davem@davemloft.net>

IMO: If Volodymyr wants to be a maintainer here, he should put
his email as an entry in the MAINTAINERS file for the subsystem.

> > Maybe there is a problem of the script that misses one.

I don't think so.  Maybe you have more evidence...

> > Anyway, I have already submitted the same patch and added
> > "vmytnyk@marvell.com" this time.
> 
> Ha! So you do indeed use it in a way I wasn't expecting :S
> Thanks for the explanation.
> 
> Joe, would you be okay to add a "big fat warning" to get_maintainer
> when people try to use the -f flag?

No, not really.  -f isn't required when the file is in git anyway.

> Maybe we can also change the message
> that's displayed when the script is run without arguments to not
> mention -f?

I think that's a poor idea as frequently the script isn't used
on patches but simply to identify the maintainers of a particular
file or subsystem.

> We're getting quite a few fixes which don't CC author, I'm guessing
> Jiasheng's approach may be a common one.

There's no great way to identify "author" or "original submitter"
and frequently the "original submitter" isn't a maintainer anyway.


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] net: prestera: acl: Add check for kmemdup
  2022-09-30  5:03 Jiasheng Jiang
@ 2022-09-30 14:29 ` Jakub Kicinski
  2022-09-30 15:20   ` Joe Perches
  0 siblings, 1 reply; 11+ messages in thread
From: Jakub Kicinski @ 2022-09-30 14:29 UTC (permalink / raw)
  To: Jiasheng Jiang, Joe Perches
  Cc: pabeni, davem, tchornyi, edumazet, netdev, linux-kernel,
	Volodymyr Mytnyk

On Fri, 30 Sep 2022 13:03:17 +0800 Jiasheng Jiang wrote:
> Actually, I used get_maintainer scripts and the results is as follow:
> "./scripts/get_maintainer.pl -f drivers/net/ethernet/marvell/prestera/prestera_acl.c"
> Taras Chornyi <tchornyi@marvell.com> (supporter:MARVELL PRESTERA ETHERNET SWITCH DRIVER)
> "David S. Miller" <davem@davemloft.net> (maintainer:NETWORKING DRIVERS)
> Eric Dumazet <edumazet@google.com> (maintainer:NETWORKING DRIVERS)
> Jakub Kicinski <kuba@kernel.org> (maintainer:NETWORKING DRIVERS)
> Paolo Abeni <pabeni@redhat.com> (maintainer:NETWORKING DRIVERS)
> netdev@vger.kernel.org (open list:NETWORKING DRIVERS)
> linux-kernel@vger.kernel.org (open list)
> 
> Therefore, I submitted my patch to the above addresses.
> 
> And this time I checked the fixes commit, and found that it has two
> authors:
> Signed-off-by: Volodymyr Mytnyk <vmytnyk@marvell.com>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> 
> Maybe there is a problem of the script that misses one.
> Anyway, I have already submitted the same patch and added
> "vmytnyk@marvell.com" this time.

Ha! So you do indeed use it in a way I wasn't expecting :S
Thanks for the explanation.

Joe, would you be okay to add a "big fat warning" to get_maintainer
when people try to use the -f flag? Maybe we can also change the message
that's displayed when the script is run without arguments to not
mention -f?

We're getting quite a few fixes which don't CC author, I'm guessing
Jiasheng's approach may be a common one.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] net: prestera: acl: Add check for kmemdup
  2022-09-30  4:48 Jiasheng Jiang
@ 2022-09-30  9:20 ` Taras Chornyi
  2022-10-03 11:40 ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 11+ messages in thread
From: Taras Chornyi @ 2022-09-30  9:20 UTC (permalink / raw)
  To: Jiasheng Jiang, vmytnyk, davem, edumazet, kuba, pabeni
  Cc: netdev, linux-kernel


> ----------------------------------------------------------------------
> As the kemdup could return NULL, it should be better to check the return
> value and return error if fails.
> Moreover, the return value of prestera_acl_ruleset_keymask_set() should
> be checked by cascade.
>
> Fixes: 604ba230902d ("net: prestera: flower template support")
> Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn>
> ---
>   drivers/net/ethernet/marvell/prestera/prestera_acl.c    | 8 ++++++--
>   drivers/net/ethernet/marvell/prestera/prestera_acl.h    | 4 ++--
>   drivers/net/ethernet/marvell/prestera/prestera_flower.c | 6 +++++-
>   3 files changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/ethernet/marvell/prestera/prestera_acl.c b/drivers/net/ethernet/marvell/prestera/prestera_acl.c
> index 3d4b85f2d541..f6b2933859d0 100644
> --- a/drivers/net/ethernet/marvell/prestera/prestera_acl.c
> +++ b/drivers/net/ethernet/marvell/prestera/prestera_acl.c
> @@ -178,10 +178,14 @@ prestera_acl_ruleset_create(struct prestera_acl *acl,
>   	return ERR_PTR(err);
>   }
>   
> -void prestera_acl_ruleset_keymask_set(struct prestera_acl_ruleset *ruleset,
> -				      void *keymask)
> +int prestera_acl_ruleset_keymask_set(struct prestera_acl_ruleset *ruleset,
> +				     void *keymask)
>   {
>   	ruleset->keymask = kmemdup(keymask, ACL_KEYMASK_SIZE, GFP_KERNEL);
> +	if (!ruleset->keymask)
> +		return -ENOMEM;
> +
> +	return 0;
>   }
>   
>   int prestera_acl_ruleset_offload(struct prestera_acl_ruleset *ruleset)
> diff --git a/drivers/net/ethernet/marvell/prestera/prestera_acl.h b/drivers/net/ethernet/marvell/prestera/prestera_acl.h
> index 03fc5b9dc925..131bfbc87cd7 100644
> --- a/drivers/net/ethernet/marvell/prestera/prestera_acl.h
> +++ b/drivers/net/ethernet/marvell/prestera/prestera_acl.h
> @@ -185,8 +185,8 @@ struct prestera_acl_ruleset *
>   prestera_acl_ruleset_lookup(struct prestera_acl *acl,
>   			    struct prestera_flow_block *block,
>   			    u32 chain_index);
> -void prestera_acl_ruleset_keymask_set(struct prestera_acl_ruleset *ruleset,
> -				      void *keymask);
> +int prestera_acl_ruleset_keymask_set(struct prestera_acl_ruleset *ruleset,
> +				     void *keymask);
>   bool prestera_acl_ruleset_is_offload(struct prestera_acl_ruleset *ruleset);
>   int prestera_acl_ruleset_offload(struct prestera_acl_ruleset *ruleset);
>   void prestera_acl_ruleset_put(struct prestera_acl_ruleset *ruleset);
> diff --git a/drivers/net/ethernet/marvell/prestera/prestera_flower.c b/drivers/net/ethernet/marvell/prestera/prestera_flower.c
> index 19d3b55c578e..cf551a8379ac 100644
> --- a/drivers/net/ethernet/marvell/prestera/prestera_flower.c
> +++ b/drivers/net/ethernet/marvell/prestera/prestera_flower.c
> @@ -452,7 +452,9 @@ int prestera_flower_tmplt_create(struct prestera_flow_block *block,
>   	}
>   
>   	/* preserve keymask/template to this ruleset */
> -	prestera_acl_ruleset_keymask_set(ruleset, rule.re_key.match.mask);
> +	err = prestera_acl_ruleset_keymask_set(ruleset, rule.re_key.match.mask);
> +	if (err)
> +		goto err_ruleset_keymask_set;
>   
>   	/* skip error, as it is not possible to reject template operation,
>   	 * so, keep the reference to the ruleset for rules to be added
> @@ -468,6 +470,8 @@ int prestera_flower_tmplt_create(struct prestera_flow_block *block,
>   	list_add_rcu(&template->list, &block->template_list);
>   	return 0;
>   
> +err_ruleset_keymask_set:
> +	prestera_acl_ruleset_put(ruleset);
>   err_ruleset_get:
>   	kfree(template);
>   err_malloc:

Reviewed-by: Taras Chornyi<tchornyi@marvell.com>


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH] net: prestera: acl: Add check for kmemdup
@ 2022-09-30  4:48 Jiasheng Jiang
  2022-09-30  9:20 ` Taras Chornyi
  2022-10-03 11:40 ` patchwork-bot+netdevbpf
  0 siblings, 2 replies; 11+ messages in thread
From: Jiasheng Jiang @ 2022-09-30  4:48 UTC (permalink / raw)
  To: vmytnyk, davem, tchornyi, edumazet, kuba, pabeni
  Cc: netdev, linux-kernel, Jiasheng Jiang

As the kemdup could return NULL, it should be better to check the return
value and return error if fails.
Moreover, the return value of prestera_acl_ruleset_keymask_set() should
be checked by cascade.

Fixes: 604ba230902d ("net: prestera: flower template support")
Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn>
---
 drivers/net/ethernet/marvell/prestera/prestera_acl.c    | 8 ++++++--
 drivers/net/ethernet/marvell/prestera/prestera_acl.h    | 4 ++--
 drivers/net/ethernet/marvell/prestera/prestera_flower.c | 6 +++++-
 3 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/marvell/prestera/prestera_acl.c b/drivers/net/ethernet/marvell/prestera/prestera_acl.c
index 3d4b85f2d541..f6b2933859d0 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_acl.c
+++ b/drivers/net/ethernet/marvell/prestera/prestera_acl.c
@@ -178,10 +178,14 @@ prestera_acl_ruleset_create(struct prestera_acl *acl,
 	return ERR_PTR(err);
 }
 
-void prestera_acl_ruleset_keymask_set(struct prestera_acl_ruleset *ruleset,
-				      void *keymask)
+int prestera_acl_ruleset_keymask_set(struct prestera_acl_ruleset *ruleset,
+				     void *keymask)
 {
 	ruleset->keymask = kmemdup(keymask, ACL_KEYMASK_SIZE, GFP_KERNEL);
+	if (!ruleset->keymask)
+		return -ENOMEM;
+
+	return 0;
 }
 
 int prestera_acl_ruleset_offload(struct prestera_acl_ruleset *ruleset)
diff --git a/drivers/net/ethernet/marvell/prestera/prestera_acl.h b/drivers/net/ethernet/marvell/prestera/prestera_acl.h
index 03fc5b9dc925..131bfbc87cd7 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_acl.h
+++ b/drivers/net/ethernet/marvell/prestera/prestera_acl.h
@@ -185,8 +185,8 @@ struct prestera_acl_ruleset *
 prestera_acl_ruleset_lookup(struct prestera_acl *acl,
 			    struct prestera_flow_block *block,
 			    u32 chain_index);
-void prestera_acl_ruleset_keymask_set(struct prestera_acl_ruleset *ruleset,
-				      void *keymask);
+int prestera_acl_ruleset_keymask_set(struct prestera_acl_ruleset *ruleset,
+				     void *keymask);
 bool prestera_acl_ruleset_is_offload(struct prestera_acl_ruleset *ruleset);
 int prestera_acl_ruleset_offload(struct prestera_acl_ruleset *ruleset);
 void prestera_acl_ruleset_put(struct prestera_acl_ruleset *ruleset);
diff --git a/drivers/net/ethernet/marvell/prestera/prestera_flower.c b/drivers/net/ethernet/marvell/prestera/prestera_flower.c
index 19d3b55c578e..cf551a8379ac 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_flower.c
+++ b/drivers/net/ethernet/marvell/prestera/prestera_flower.c
@@ -452,7 +452,9 @@ int prestera_flower_tmplt_create(struct prestera_flow_block *block,
 	}
 
 	/* preserve keymask/template to this ruleset */
-	prestera_acl_ruleset_keymask_set(ruleset, rule.re_key.match.mask);
+	err = prestera_acl_ruleset_keymask_set(ruleset, rule.re_key.match.mask);
+	if (err)
+		goto err_ruleset_keymask_set;
 
 	/* skip error, as it is not possible to reject template operation,
 	 * so, keep the reference to the ruleset for rules to be added
@@ -468,6 +470,8 @@ int prestera_flower_tmplt_create(struct prestera_flow_block *block,
 	list_add_rcu(&template->list, &block->template_list);
 	return 0;
 
+err_ruleset_keymask_set:
+	prestera_acl_ruleset_put(ruleset);
 err_ruleset_get:
 	kfree(template);
 err_malloc:
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2022-10-03 11:40 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-28  9:20 [PATCH] net: prestera: acl: Add check for kmemdup Jiasheng Jiang
2022-09-29 16:15 ` Jakub Kicinski
2022-09-30  4:48 Jiasheng Jiang
2022-09-30  9:20 ` Taras Chornyi
2022-10-03 11:40 ` patchwork-bot+netdevbpf
2022-09-30  5:03 Jiasheng Jiang
2022-09-30 14:29 ` Jakub Kicinski
2022-09-30 15:20   ` Joe Perches
2022-09-30 15:44     ` Jakub Kicinski
2022-09-30 16:43       ` Joe Perches
2022-09-30 16:58         ` Jakub Kicinski
2022-09-30 18:28           ` Joe Perches

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).