linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net] net: dsa: don't assign an error value to tag_ops
@ 2021-03-22 20:26 George McCollister
  2021-03-22 20:46 ` Vladimir Oltean
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: George McCollister @ 2021-03-22 20:26 UTC (permalink / raw)
  Cc: Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean,
	David S. Miller, Jakub Kicinski, netdev, linux-kernel,
	George McCollister

Use a temporary variable to hold the return value from
dsa_tag_driver_get() instead of assigning it to dst->tag_ops. Leaving
an error value in dst->tag_ops can result in deferencing an invalid
pointer when a deferred switch configuration happens later.

Fixes: 357f203bb3b5 ("net: dsa: keep a copy of the tagging protocol in the DSA switch tree")

Signed-off-by: George McCollister <george.mccollister@gmail.com>
---
 net/dsa/dsa2.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index eb709d988c54..8f9e35e1aa89 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -1068,6 +1068,7 @@ static int dsa_port_parse_cpu(struct dsa_port *dp, struct net_device *master)
 {
 	struct dsa_switch *ds = dp->ds;
 	struct dsa_switch_tree *dst = ds->dst;
+	const struct dsa_device_ops *tag_ops;
 	enum dsa_tag_protocol tag_protocol;
 
 	tag_protocol = dsa_get_tag_protocol(dp, master);
@@ -1082,14 +1083,16 @@ static int dsa_port_parse_cpu(struct dsa_port *dp, struct net_device *master)
 		 * nothing to do here.
 		 */
 	} else {
-		dst->tag_ops = dsa_tag_driver_get(tag_protocol);
-		if (IS_ERR(dst->tag_ops)) {
-			if (PTR_ERR(dst->tag_ops) == -ENOPROTOOPT)
+		tag_ops = dsa_tag_driver_get(tag_protocol);
+		if (IS_ERR(tag_ops)) {
+			if (PTR_ERR(tag_ops) == -ENOPROTOOPT)
 				return -EPROBE_DEFER;
 			dev_warn(ds->dev, "No tagger for this switch\n");
 			dp->master = NULL;
-			return PTR_ERR(dst->tag_ops);
+			return PTR_ERR(tag_ops);
 		}
+
+		dst->tag_ops = tag_ops;
 	}
 
 	dp->master = master;
-- 
2.11.0


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

* Re: [PATCH net] net: dsa: don't assign an error value to tag_ops
  2021-03-22 20:26 [PATCH net] net: dsa: don't assign an error value to tag_ops George McCollister
@ 2021-03-22 20:46 ` Vladimir Oltean
  2021-03-22 20:51   ` George McCollister
  2021-03-22 20:52 ` Vladimir Oltean
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 6+ messages in thread
From: Vladimir Oltean @ 2021-03-22 20:46 UTC (permalink / raw)
  To: George McCollister
  Cc: Andrew Lunn, Vivien Didelot, Florian Fainelli, David S. Miller,
	Jakub Kicinski, netdev, linux-kernel

On Mon, Mar 22, 2021 at 03:26:50PM -0500, George McCollister wrote:
> Use a temporary variable to hold the return value from
> dsa_tag_driver_get() instead of assigning it to dst->tag_ops. Leaving
> an error value in dst->tag_ops can result in deferencing an invalid
> pointer when a deferred switch configuration happens later.
> 
> Fixes: 357f203bb3b5 ("net: dsa: keep a copy of the tagging protocol in the DSA switch tree")
> 
> Signed-off-by: George McCollister <george.mccollister@gmail.com>
> ---

Who dereferences the invalid pointer? dsa_tree_free I guess?

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

* Re: [PATCH net] net: dsa: don't assign an error value to tag_ops
  2021-03-22 20:46 ` Vladimir Oltean
@ 2021-03-22 20:51   ` George McCollister
  0 siblings, 0 replies; 6+ messages in thread
From: George McCollister @ 2021-03-22 20:51 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: Andrew Lunn, Vivien Didelot, Florian Fainelli, David S. Miller,
	Jakub Kicinski, netdev, open list

On Mon, Mar 22, 2021 at 3:46 PM Vladimir Oltean <olteanv@gmail.com> wrote:
>
> On Mon, Mar 22, 2021 at 03:26:50PM -0500, George McCollister wrote:
> > Use a temporary variable to hold the return value from
> > dsa_tag_driver_get() instead of assigning it to dst->tag_ops. Leaving
> > an error value in dst->tag_ops can result in deferencing an invalid
> > pointer when a deferred switch configuration happens later.
> >
> > Fixes: 357f203bb3b5 ("net: dsa: keep a copy of the tagging protocol in the DSA switch tree")
> >
> > Signed-off-by: George McCollister <george.mccollister@gmail.com>
> > ---
>
> Who dereferences the invalid pointer? dsa_tree_free I guess?

I saw it occur just above on the following line the next time
dsa_port_parse_cpu() is called:
if (dst->tag_ops->proto != tag_protocol) {

-George

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

* Re: [PATCH net] net: dsa: don't assign an error value to tag_ops
  2021-03-22 20:26 [PATCH net] net: dsa: don't assign an error value to tag_ops George McCollister
  2021-03-22 20:46 ` Vladimir Oltean
@ 2021-03-22 20:52 ` Vladimir Oltean
  2021-03-22 23:22 ` Florian Fainelli
  2021-03-23  0:30 ` patchwork-bot+netdevbpf
  3 siblings, 0 replies; 6+ messages in thread
From: Vladimir Oltean @ 2021-03-22 20:52 UTC (permalink / raw)
  To: George McCollister
  Cc: Andrew Lunn, Vivien Didelot, Florian Fainelli, David S. Miller,
	Jakub Kicinski, netdev, linux-kernel

On Mon, Mar 22, 2021 at 03:26:50PM -0500, George McCollister wrote:
> Use a temporary variable to hold the return value from
> dsa_tag_driver_get() instead of assigning it to dst->tag_ops. Leaving
> an error value in dst->tag_ops can result in deferencing an invalid
> pointer when a deferred switch configuration happens later.
> 
> Fixes: 357f203bb3b5 ("net: dsa: keep a copy of the tagging protocol in the DSA switch tree")
> 
> Signed-off-by: George McCollister <george.mccollister@gmail.com>
> ---

Reviewed-by: Vladimir Oltean <olteanv@gmail.com>

Just FYI, new lines aren't typically added between the various tags.

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

* Re: [PATCH net] net: dsa: don't assign an error value to tag_ops
  2021-03-22 20:26 [PATCH net] net: dsa: don't assign an error value to tag_ops George McCollister
  2021-03-22 20:46 ` Vladimir Oltean
  2021-03-22 20:52 ` Vladimir Oltean
@ 2021-03-22 23:22 ` Florian Fainelli
  2021-03-23  0:30 ` patchwork-bot+netdevbpf
  3 siblings, 0 replies; 6+ messages in thread
From: Florian Fainelli @ 2021-03-22 23:22 UTC (permalink / raw)
  To: George McCollister
  Cc: Andrew Lunn, Vivien Didelot, Vladimir Oltean, David S. Miller,
	Jakub Kicinski, netdev, linux-kernel



On 3/22/2021 1:26 PM, George McCollister wrote:
> Use a temporary variable to hold the return value from
> dsa_tag_driver_get() instead of assigning it to dst->tag_ops. Leaving
> an error value in dst->tag_ops can result in deferencing an invalid
> pointer when a deferred switch configuration happens later.
> 
> Fixes: 357f203bb3b5 ("net: dsa: keep a copy of the tagging protocol in the DSA switch tree")
> 
> Signed-off-by: George McCollister <george.mccollister@gmail.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

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

* Re: [PATCH net] net: dsa: don't assign an error value to tag_ops
  2021-03-22 20:26 [PATCH net] net: dsa: don't assign an error value to tag_ops George McCollister
                   ` (2 preceding siblings ...)
  2021-03-22 23:22 ` Florian Fainelli
@ 2021-03-23  0:30 ` patchwork-bot+netdevbpf
  3 siblings, 0 replies; 6+ messages in thread
From: patchwork-bot+netdevbpf @ 2021-03-23  0:30 UTC (permalink / raw)
  To: George McCollister
  Cc: andrew, vivien.didelot, f.fainelli, olteanv, davem, kuba, netdev,
	linux-kernel

Hello:

This patch was applied to netdev/net.git (refs/heads/master):

On Mon, 22 Mar 2021 15:26:50 -0500 you wrote:
> Use a temporary variable to hold the return value from
> dsa_tag_driver_get() instead of assigning it to dst->tag_ops. Leaving
> an error value in dst->tag_ops can result in deferencing an invalid
> pointer when a deferred switch configuration happens later.
> 
> Fixes: 357f203bb3b5 ("net: dsa: keep a copy of the tagging protocol in the DSA switch tree")
> 
> [...]

Here is the summary with links:
  - [net] net: dsa: don't assign an error value to tag_ops
    https://git.kernel.org/netdev/net/c/e0c755a45f6f

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:[~2021-03-23  0:31 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-22 20:26 [PATCH net] net: dsa: don't assign an error value to tag_ops George McCollister
2021-03-22 20:46 ` Vladimir Oltean
2021-03-22 20:51   ` George McCollister
2021-03-22 20:52 ` Vladimir Oltean
2021-03-22 23:22 ` Florian Fainelli
2021-03-23  0:30 ` patchwork-bot+netdevbpf

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