Re: Re: [PATCH] net: prestera: acl: Add check for kmemdup

Re: Re: [PATCH] net: prestera: acl: Add check for kmemdup

[Jiasheng Jiang]

On Fri, 30 Sep 2022 00:15:27 +0800 Jakub Kicinski wrote:
> 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.

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.

Thanks,
Jiang

Re: [PATCH] net: prestera: acl: Add check for kmemdup

[Jakub Kicinski]

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.

Re: [PATCH] net: prestera: acl: Add check for kmemdup

[Joe Perches]

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.

Re: [PATCH] net: prestera: acl: Add check for kmemdup

[Jakub Kicinski]

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.

Re: [PATCH] net: prestera: acl: Add check for kmemdup

[Joe Perches]

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.

Re: [PATCH] net: prestera: acl: Add check for kmemdup

[Jakub Kicinski]

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%)

Re: [PATCH] net: prestera: acl: Add check for kmemdup

[Joe Perches]

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

[PATCH] net: prestera: acl: Add check for kmemdup

[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

Re: [PATCH] net: prestera: acl: Add check for kmemdup

[Taras Chornyi]

> ----------------------------------------------------------------------
> 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>

Re: [PATCH] net: prestera: acl: Add check for kmemdup

[patchwork-bot+netdevbpf]

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

[PATCH] net: prestera: acl: Add check for kmemdup

[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

Re: [PATCH] net: prestera: acl: Add check for kmemdup

[Jakub Kicinski]

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.