| Message ID | 20201118154335.1189-1-ceggers@arri.de (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] net: dsa: avoid potential use-after-free error | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/subject_prefix | success | Link |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 13 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | Link |
| netdev/stable | success | Stable not CCed |
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(-)