From 5eee7bd7e245914e4e050c413dfe864e31805207 Mon Sep 17 00:00:00 2001 From: "Jason A. Donenfeld" Date: Mon, 13 Jan 2020 18:42:26 -0500 Subject: [PATCH 1/8] net: skbuff: disambiguate argument and member for skb_list_walk_safe helper This worked before, because we made all callers name their next pointer "next". But in trying to be more "drop-in" ready, the silliness here is revealed. This commit fixes the problem by making the macro argument and the member use different names. Signed-off-by: Jason A. Donenfeld Signed-off-by: David S. Miller --- include/linux/skbuff.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h index 016b3c4ab99a..aaf73b34f72f 100644 --- a/include/linux/skbuff.h +++ b/include/linux/skbuff.h @@ -1479,9 +1479,9 @@ static inline void skb_mark_not_on_list(struct sk_buff *skb) } /* Iterate through singly-linked GSO fragments of an skb. */ -#define skb_list_walk_safe(first, skb, next) \ - for ((skb) = (first), (next) = (skb) ? (skb)->next : NULL; (skb); \ - (skb) = (next), (next) = (skb) ? (skb)->next : NULL) +#define skb_list_walk_safe(first, skb, next_skb) \ + for ((skb) = (first), (next_skb) = (skb) ? (skb)->next : NULL; (skb); \ + (skb) = (next_skb), (next_skb) = (skb) ? (skb)->next : NULL) static inline void skb_list_del_init(struct sk_buff *skb) { From 1a186c14cedbf3364634f2dfa9302ed31b8fab19 Mon Sep 17 00:00:00 2001 From: "Jason A. Donenfeld" Date: Mon, 13 Jan 2020 18:42:27 -0500 Subject: [PATCH 2/8] net: udp: use skb_list_walk_safe helper for gso segments This is a straight-forward conversion case for the new function, iterating over the return value from udp_rcv_segment, which actually is a wrapper around skb_gso_segment. Signed-off-by: Jason A. Donenfeld Signed-off-by: David S. Miller --- net/ipv4/udp.c | 3 +-- net/ipv6/udp.c | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c index 93a355b6b092..208da0917469 100644 --- a/net/ipv4/udp.c +++ b/net/ipv4/udp.c @@ -2104,8 +2104,7 @@ static int udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb) BUILD_BUG_ON(sizeof(struct udp_skb_cb) > SKB_SGO_CB_OFFSET); __skb_push(skb, -skb_mac_offset(skb)); segs = udp_rcv_segment(sk, skb, true); - for (skb = segs; skb; skb = next) { - next = skb->next; + skb_list_walk_safe(segs, skb, next) { __skb_pull(skb, skb_transport_offset(skb)); ret = udp_queue_rcv_one_skb(sk, skb); if (ret > 0) diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c index 9fec580c968e..5dc439a391fe 100644 --- a/net/ipv6/udp.c +++ b/net/ipv6/udp.c @@ -690,8 +690,7 @@ static int udpv6_queue_rcv_skb(struct sock *sk, struct sk_buff *skb) __skb_push(skb, -skb_mac_offset(skb)); segs = udp_rcv_segment(sk, skb, false); - for (skb = segs; skb; skb = next) { - next = skb->next; + skb_list_walk_safe(segs, skb, next) { __skb_pull(skb, skb_transport_offset(skb)); ret = udpv6_queue_rcv_one_skb(sk, skb); From c3b18e0d925439043b5887c9a6129ff2ffaf58b0 Mon Sep 17 00:00:00 2001 From: "Jason A. Donenfeld" Date: Mon, 13 Jan 2020 18:42:28 -0500 Subject: [PATCH 3/8] net: xfrm: use skb_list_walk_safe helper for gso segments This is converts xfrm segment iteration to use the new function, keeping the flow of the existing code as intact as possible. One case is very straight-forward, whereas the other case has some more subtle code that likes to peak at ->next and relink skbs. By keeping the variables the same as before, we can upgrade this code with minimal surgery required. Signed-off-by: Jason A. Donenfeld Signed-off-by: David S. Miller --- net/xfrm/xfrm_device.c | 15 ++++----------- net/xfrm/xfrm_output.c | 9 +++------ 2 files changed, 7 insertions(+), 17 deletions(-) diff --git a/net/xfrm/xfrm_device.c b/net/xfrm/xfrm_device.c index 189ef15acbbc..50f567a88f45 100644 --- a/net/xfrm/xfrm_device.c +++ b/net/xfrm/xfrm_device.c @@ -78,7 +78,7 @@ struct sk_buff *validate_xmit_xfrm(struct sk_buff *skb, netdev_features_t featur int err; unsigned long flags; struct xfrm_state *x; - struct sk_buff *skb2; + struct sk_buff *skb2, *nskb; struct softnet_data *sd; netdev_features_t esp_features = features; struct xfrm_offload *xo = xfrm_offload(skb); @@ -148,11 +148,7 @@ struct sk_buff *validate_xmit_xfrm(struct sk_buff *skb, netdev_features_t featur return skb; } - skb2 = skb; - - do { - struct sk_buff *nskb = skb2->next; - + skb_list_walk_safe(skb, skb2, nskb) { esp_features |= skb->dev->gso_partial_features; skb_mark_not_on_list(skb2); @@ -176,14 +172,11 @@ struct sk_buff *validate_xmit_xfrm(struct sk_buff *skb, netdev_features_t featur if (!skb) return NULL; - goto skip_push; + continue; } skb_push(skb2, skb2->data - skb_mac_header(skb2)); - -skip_push: - skb2 = nskb; - } while (skb2); + } return skb; } diff --git a/net/xfrm/xfrm_output.c b/net/xfrm/xfrm_output.c index b1db55b50ba1..fafc7aba705f 100644 --- a/net/xfrm/xfrm_output.c +++ b/net/xfrm/xfrm_output.c @@ -533,7 +533,7 @@ static int xfrm_output2(struct net *net, struct sock *sk, struct sk_buff *skb) static int xfrm_output_gso(struct net *net, struct sock *sk, struct sk_buff *skb) { - struct sk_buff *segs; + struct sk_buff *segs, *nskb; BUILD_BUG_ON(sizeof(*IPCB(skb)) > SKB_SGO_CB_OFFSET); BUILD_BUG_ON(sizeof(*IP6CB(skb)) > SKB_SGO_CB_OFFSET); @@ -544,8 +544,7 @@ static int xfrm_output_gso(struct net *net, struct sock *sk, struct sk_buff *skb if (segs == NULL) return -EINVAL; - do { - struct sk_buff *nskb = segs->next; + skb_list_walk_safe(segs, segs, nskb) { int err; skb_mark_not_on_list(segs); @@ -555,9 +554,7 @@ static int xfrm_output_gso(struct net *net, struct sock *sk, struct sk_buff *skb kfree_skb_list(nskb); return err; } - - segs = nskb; - } while (segs); + } return 0; } From 2cec4448db38758832c2edad439f99584bb8fa0d Mon Sep 17 00:00:00 2001 From: "Jason A. Donenfeld" Date: Mon, 13 Jan 2020 18:42:29 -0500 Subject: [PATCH 4/8] net: openvswitch: use skb_list_walk_safe helper for gso segments This is a straight-forward conversion case for the new function, keeping the flow of the existing code as intact as possible. Signed-off-by: Jason A. Donenfeld Signed-off-by: David S. Miller --- net/openvswitch/datapath.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c index e3a37d22539c..659c2a790fe7 100644 --- a/net/openvswitch/datapath.c +++ b/net/openvswitch/datapath.c @@ -321,8 +321,7 @@ static int queue_gso_packets(struct datapath *dp, struct sk_buff *skb, } /* Queue all of the segments. */ - skb = segs; - do { + skb_list_walk_safe(segs, skb, nskb) { if (gso_type & SKB_GSO_UDP && skb != segs) key = &later_key; @@ -330,17 +329,15 @@ static int queue_gso_packets(struct datapath *dp, struct sk_buff *skb, if (err) break; - } while ((skb = skb->next)); + } /* Free all of the segments. */ - skb = segs; - do { - nskb = skb->next; + skb_list_walk_safe(segs, skb, nskb) { if (err) kfree_skb(skb); else consume_skb(skb); - } while ((skb = nskb)); + } return err; } From b950d8a5b300ceeb2e530291cddc6b3e547291f0 Mon Sep 17 00:00:00 2001 From: "Jason A. Donenfeld" Date: Mon, 13 Jan 2020 18:42:30 -0500 Subject: [PATCH 5/8] net: sched: use skb_list_walk_safe helper for gso segments This is a straight-forward conversion case for the new function, keeping the flow of the existing code as intact as possible. Signed-off-by: Jason A. Donenfeld Signed-off-by: David S. Miller --- net/sched/sch_cake.c | 4 +--- net/sched/sch_tbf.c | 4 +--- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/net/sched/sch_cake.c b/net/sched/sch_cake.c index 90ef7cc79b69..1496e87cd07b 100644 --- a/net/sched/sch_cake.c +++ b/net/sched/sch_cake.c @@ -1682,8 +1682,7 @@ static s32 cake_enqueue(struct sk_buff *skb, struct Qdisc *sch, if (IS_ERR_OR_NULL(segs)) return qdisc_drop(skb, sch, to_free); - while (segs) { - nskb = segs->next; + skb_list_walk_safe(segs, segs, nskb) { skb_mark_not_on_list(segs); qdisc_skb_cb(segs)->pkt_len = segs->len; cobalt_set_enqueue_time(segs, now); @@ -1696,7 +1695,6 @@ static s32 cake_enqueue(struct sk_buff *skb, struct Qdisc *sch, slen += segs->len; q->buffer_used += segs->truesize; b->packets++; - segs = nskb; } /* stats */ diff --git a/net/sched/sch_tbf.c b/net/sched/sch_tbf.c index 5f72f3f916a5..2cd94973795c 100644 --- a/net/sched/sch_tbf.c +++ b/net/sched/sch_tbf.c @@ -155,8 +155,7 @@ static int tbf_segment(struct sk_buff *skb, struct Qdisc *sch, return qdisc_drop(skb, sch, to_free); nb = 0; - while (segs) { - nskb = segs->next; + skb_list_walk_safe(segs, segs, nskb) { skb_mark_not_on_list(segs); qdisc_skb_cb(segs)->pkt_len = segs->len; len += segs->len; @@ -167,7 +166,6 @@ static int tbf_segment(struct sk_buff *skb, struct Qdisc *sch, } else { nb++; } - segs = nskb; } sch->q.qlen += nb; if (nb > 1) From 88bebdf5b26fe9fdeb36f2152978a3bb478b0d4d Mon Sep 17 00:00:00 2001 From: "Jason A. Donenfeld" Date: Mon, 13 Jan 2020 18:42:31 -0500 Subject: [PATCH 6/8] net: ipv4: use skb_list_walk_safe helper for gso segments This is a straight-forward conversion case for the new function, keeping the flow of the existing code as intact as possible. Signed-off-by: Jason A. Donenfeld Signed-off-by: David S. Miller --- net/ipv4/ip_output.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c index 14db1e0b8a6e..d84819893db9 100644 --- a/net/ipv4/ip_output.c +++ b/net/ipv4/ip_output.c @@ -240,8 +240,8 @@ static int ip_finish_output2(struct net *net, struct sock *sk, struct sk_buff *s static int ip_finish_output_gso(struct net *net, struct sock *sk, struct sk_buff *skb, unsigned int mtu) { + struct sk_buff *segs, *nskb; netdev_features_t features; - struct sk_buff *segs; int ret = 0; /* common case: seglen is <= mtu @@ -272,8 +272,7 @@ static int ip_finish_output_gso(struct net *net, struct sock *sk, consume_skb(skb); - do { - struct sk_buff *nskb = segs->next; + skb_list_walk_safe(segs, segs, nskb) { int err; skb_mark_not_on_list(segs); @@ -281,8 +280,7 @@ static int ip_finish_output_gso(struct net *net, struct sock *sk, if (err && ret == 0) ret = err; - segs = nskb; - } while (segs); + } return ret; } From 2670ee77c99c76f46c061caf65daa2e2bfb5dcc5 Mon Sep 17 00:00:00 2001 From: "Jason A. Donenfeld" Date: Mon, 13 Jan 2020 18:42:32 -0500 Subject: [PATCH 7/8] net: netfilter: use skb_list_walk_safe helper for gso segments This is a straight-forward conversion case for the new function, keeping the flow of the existing code as intact as possible. Signed-off-by: Jason A. Donenfeld Signed-off-by: David S. Miller --- net/netfilter/nfnetlink_queue.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/net/netfilter/nfnetlink_queue.c b/net/netfilter/nfnetlink_queue.c index feabdfb22920..76535fd9278c 100644 --- a/net/netfilter/nfnetlink_queue.c +++ b/net/netfilter/nfnetlink_queue.c @@ -778,7 +778,7 @@ nfqnl_enqueue_packet(struct nf_queue_entry *entry, unsigned int queuenum) { unsigned int queued; struct nfqnl_instance *queue; - struct sk_buff *skb, *segs; + struct sk_buff *skb, *segs, *nskb; int err = -ENOBUFS; struct net *net = entry->state.net; struct nfnl_queue_net *q = nfnl_queue_pernet(net); @@ -815,8 +815,7 @@ nfqnl_enqueue_packet(struct nf_queue_entry *entry, unsigned int queuenum) goto out_err; queued = 0; err = 0; - do { - struct sk_buff *nskb = segs->next; + skb_list_walk_safe(segs, segs, nskb) { if (err == 0) err = __nfqnl_enqueue_packet_gso(net, queue, segs, entry); @@ -824,8 +823,7 @@ nfqnl_enqueue_packet(struct nf_queue_entry *entry, unsigned int queuenum) queued++; else kfree_skb(segs); - segs = nskb; - } while (segs); + } if (queued) { if (err) /* some segments are already queued */ From 9f3ef3d7026fe0aef5c2d649e21b8c71c17d8fc2 Mon Sep 17 00:00:00 2001 From: "Jason A. Donenfeld" Date: Mon, 13 Jan 2020 18:42:33 -0500 Subject: [PATCH 8/8] net: mac80211: use skb_list_walk_safe helper for gso segments This is a conversion case for the new function, keeping the flow of the existing code as intact as possible. We also switch over to using skb_mark_not_on_list instead of a null write to skb->next. Finally, this code appeared to have a memory leak in the case where header building fails before the last gso segment. In that case, the remaining segments are not freed. So this commit also adds the proper kfree_skb_list call for the remainder of the skbs. Signed-off-by: Jason A. Donenfeld Signed-off-by: David S. Miller --- net/mac80211/tx.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c index a8a7306a1f56..4bd1faf4f779 100644 --- a/net/mac80211/tx.c +++ b/net/mac80211/tx.c @@ -3949,18 +3949,15 @@ void __ieee80211_subif_start_xmit(struct sk_buff *skb, } } - next = skb; - while (next) { - skb = next; - next = skb->next; - - skb->prev = NULL; - skb->next = NULL; + skb_list_walk_safe(skb, skb, next) { + skb_mark_not_on_list(skb); skb = ieee80211_build_hdr(sdata, skb, info_flags, sta, ctrl_flags); - if (IS_ERR(skb)) + if (IS_ERR(skb)) { + kfree_skb_list(next); goto out; + } ieee80211_tx_stats(dev, skb->len);