All of lore.kernel.org
 help / color / mirror / Atom feed
From: Saeed Mahameed <saeed@kernel.org>
To: Vladimir Oltean <vladimir.oltean@nxp.com>
Cc: netdev@vger.kernel.org, Andrew Lunn <andrew@lunn.ch>,
	Florian Fainelli <f.fainelli@gmail.com>,
	Vladimir Oltean <olteanv@gmail.com>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>
Subject: Re: [PATCH net] net: dsa: don't leak tagger-owned storage on switch driver unbind
Date: Tue, 15 Nov 2022 13:57:33 -0800	[thread overview]
Message-ID: <Y3QLTSeHa8l+nCm9@x130.lan> (raw)
In-Reply-To: <20221114143551.1906361-1-vladimir.oltean@nxp.com>

On 14 Nov 16:35, Vladimir Oltean wrote:
>In the initial commit dc452a471dba ("net: dsa: introduce tagger-owned
>storage for private and shared data"), we had a call to
>tag_ops->disconnect(dst) issued from dsa_tree_free(), which is called at
>tree teardown time.
>
>There were problems with connecting to a switch tree as a whole, so this
>got reworked to connecting to individual switches within the tree. In
>this process, tag_ops->disconnect(ds) was made to be called only from
>switch.c (cross-chip notifiers emitted as a result of dynamic tag proto
>changes), but the normal driver teardown code path wasn't replaced with
>anything.
>
>Solve this problem by adding a function that does the opposite of
>dsa_switch_setup_tag_protocol(), which is called from the equivalent
>spot in dsa_switch_teardown(). The positioning here also ensures that we
>won't have any use-after-free in tagging protocol (*rcv) ops, since the
>teardown sequence is as follows:
>
>dsa_tree_teardown
>-> dsa_tree_teardown_master
>   -> dsa_master_teardown
>      -> unsets master->dsa_ptr, making no further packets match the
>         ETH_P_XDSA packet type handler
>-> dsa_tree_teardown_ports
>   -> dsa_port_teardown
>      -> dsa_slave_destroy
>         -> unregisters DSA net devices, there is even a synchronize_net()
>            in unregister_netdevice_many()
>-> dsa_tree_teardown_switches
>   -> dsa_switch_teardown
>      -> dsa_switch_teardown_tag_protocol
>         -> finally frees the tagger-owned storage
>
>Fixes: 7f2973149c22 ("net: dsa: make tagging protocols connect to individual switches from a tree")
>Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Reviewed-by: Saeed Mahameed <saeed@kernel.org>


  reply	other threads:[~2022-11-15 21:57 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-14 14:35 [PATCH net] net: dsa: don't leak tagger-owned storage on switch driver unbind Vladimir Oltean
2022-11-15 21:57 ` Saeed Mahameed [this message]
2022-11-15 22:52 ` Florian Fainelli
2022-11-16  5:10 ` patchwork-bot+netdevbpf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Y3QLTSeHa8l+nCm9@x130.lan \
    --to=saeed@kernel.org \
    --cc=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=f.fainelli@gmail.com \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=olteanv@gmail.com \
    --cc=pabeni@redhat.com \
    --cc=vladimir.oltean@nxp.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.