| Message ID | 20201118154335.1189-1-ceggers@arri.de |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [net-next] net: dsa: avoid potential use-after-free error | expand |
On Wed, Nov 18, 2020 at 04:43:35PM +0100, Christian Eggers wrote: > If dsa_switch_ops::port_txtstamp() returns false, clone will be freed > immediately. Storing the pointer in DSA_SKB_CB(skb)->clone anyway, > supports annoying use-after-free bugs. Like what? > Signed-off-by: Christian Eggers <ceggers@arri.de> > Fixes 146d442c2357 ("net: dsa: Keep a pointer to the skb clone for TX timestamping") Not the right format for a Fixes: tag, please try to set up your .gitconfig to automate the creation of this tag.
On Thursday, 19 November 2020, 00:33:57 CET, Vladimir Oltean wrote: > On Wed, Nov 18, 2020 at 04:43:35PM +0100, Christian Eggers wrote: > > If dsa_switch_ops::port_txtstamp() returns false, clone will be freed > > immediately. Storing the pointer in DSA_SKB_CB(skb)->clone anyway, > > supports annoying use-after-free bugs. > > Like what? In my own code. I test for DSA_SKB_CB(skb)->clone in order to determine whether a skb has been selected for TX time stamping by ksz9477_ptp_port_txtstamp(). > > > Signed-off-by: Christian Eggers <ceggers@arri.de> > > Fixes 146d442c2357 ("net: dsa: Keep a pointer to the skb clone for TX > > timestamping") > Not the right format for a Fixes: tag, please try to set up your > .gitconfig to automate the creation of this tag. I think you (or somebody else) already noted this. Sorry for postponing. regards Christian
diff --git a/net/dsa/slave.c b/net/dsa/slave.c index ff2266d2b998..7efc753e4d9d 100644 --- a/net/dsa/slave.c +++ b/net/dsa/slave.c @@ -522,10 +522,10 @@ static void dsa_skb_tx_timestamp(struct dsa_slave_priv *p, if (!clone) return; - DSA_SKB_CB(skb)->clone = clone; - - if (ds->ops->port_txtstamp(ds, p->dp->index, clone, type)) + if (ds->ops->port_txtstamp(ds, p->dp->index, clone, type)) { + DSA_SKB_CB(skb)->clone = clone; return; + } kfree_skb(clone); }
If dsa_switch_ops::port_txtstamp() returns false, clone will be freed immediately. Storing the pointer in DSA_SKB_CB(skb)->clone anyway, supports annoying use-after-free bugs. Signed-off-by: Christian Eggers <ceggers@arri.de> Fixes 146d442c2357 ("net: dsa: Keep a pointer to the skb clone for TX timestamping") --- net/dsa/slave.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)