Skip to content

Commit 179f185

Browse files
V4belgregkh
authored andcommitted
net: skbuff: propagate shared-frag marker through frag-transfer helpers
commit 48f6a53 upstream. Two frag-transfer helpers (__pskb_copy_fclone() and skb_shift()) fail to propagate the SKBFL_SHARED_FRAG bit in skb_shinfo()->flags when moving frags from source to destination. __pskb_copy_fclone() defers the rest of the shinfo metadata to skb_copy_header() after copying frag descriptors, but that helper only carries over gso_{size,segs, type} and never touches skb_shinfo()->flags; skb_shift() moves frag descriptors directly and leaves flags untouched. As a result, the destination skb keeps a reference to the same externally-owned or page-cache-backed pages while reporting skb_has_shared_frag() as false. The mismatch is harmful in any in-place writer that uses skb_has_shared_frag() to decide whether shared pages must be detoured through skb_cow_data(). ESP input is one such writer (esp4.c, esp6.c), and a single nft 'dup to <local>' rule -- or any other nf_dup_ipv4() / xt_TEE caller -- is enough to land a pskb_copy()'d skb in esp_input() with the marker stripped, letting an unprivileged user write into the page cache of a root-owned read-only file via authencesn-ESN stray writes. Set SKBFL_SHARED_FRAG on the destination whenever frag descriptors were actually moved from the source. skb_copy() and skb_copy_expand() share skb_copy_header() too but linearize all paged data into freshly allocated head storage and emerge with nr_frags == 0, so skb_has_shared_frag() returns false on its own; they need no change. The same omission exists in skb_gro_receive() and skb_gro_receive_list(). The former moves the incoming skb's frag descriptors into the accumulator's last sub-skb via two paths (a direct frag-move loop and the head_frag + memcpy path); the latter chains the incoming skb whole onto p's frag_list. Downstream skb_segment() reads only skb_shinfo(p)->flags, and skb_segment_list() reuses each sub-skb's shinfo as the nskb -- both p and lp must carry the marker. The same omission also exists in tcp_clone_payload(), which builds an MTU probe skb by moving frag descriptors from skbs on sk_write_queue into a freshly allocated nskb. The helper falls into the same family and warrants the same fix for consistency; no TCP TX-side in-place writer is currently known to reach a user page through this gap, but a future consumer depending on the marker would regress silently. The same omission exists in skb_segment(): the per-iteration flag merge takes only head_skb's flag, and the inner switch that rebinds frag_skb to list_skb on head_skb-frags exhaustion does not fold the new frag_skb's flag into nskb. Fold frag_skb's flag at both sites so segments drawing frags from frag_list members carry the marker. Fixes: cef401d ("net: fix possible wrong checksum generation") Fixes: f4c50a4 ("xfrm: esp: avoid in-place decrypt on shared skb frags") Suggested-by: Sabrina Dubroca <sd@queasysnail.net> Suggested-by: Sultan Alsawaf <sultan@kerneltoast.com> Suggested-by: Ben Hutchings <ben@decadent.org.uk> Suggested-by: Lin Ma <malin89@huawei.com> Suggested-by: Jingguo Tan <tanjingguo@huawei.com> Suggested-by: Aaron Esau <aaron1esau@gmail.com> Cc: stable@vger.kernel.org Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com> Tested-by: Rajat Gupta <rajat.gupta@oss.qualcomm.com> Link: https://patch.msgid.link/ageeJfJHwgzmKXbh@v4bel Signed-off-by: Paolo Abeni <pabeni@redhat.com> [bwh: Backported to 5.15: - skb_gro_receive() and skb_gro_receive_list() are in skbuff.c here - Drop change to tcp_clone_payload(), which does not exist here - Adjust context in skb_shift() ] Signed-off-by: Ben Hutchings <benh@debian.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent 2f2b160 commit 179f185

1 file changed

Lines changed: 12 additions & 1 deletion

File tree

net/core/skbuff.c

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1661,6 +1661,7 @@ struct sk_buff *__pskb_copy_fclone(struct sk_buff *skb, int headroom,
16611661
skb_frag_ref(skb, i);
16621662
}
16631663
skb_shinfo(n)->nr_frags = i;
1664+
skb_shinfo(n)->flags |= skb_shinfo(skb)->flags & SKBFL_SHARED_FRAG;
16641665
}
16651666

16661667
if (skb_has_frag_list(skb)) {
@@ -3650,6 +3651,8 @@ int skb_shift(struct sk_buff *tgt, struct sk_buff *skb, int shiftlen)
36503651
tgt->ip_summed = CHECKSUM_PARTIAL;
36513652
skb->ip_summed = CHECKSUM_PARTIAL;
36523653

3654+
skb_shinfo(tgt)->flags |= skb_shinfo(skb)->flags & SKBFL_SHARED_FRAG;
3655+
36533656
/* Yak, is it really working this way? Some helper please? */
36543657
skb->len -= shiftlen;
36553658
skb->data_len -= shiftlen;
@@ -4017,6 +4020,8 @@ int skb_gro_receive_list(struct sk_buff *p, struct sk_buff *skb)
40174020
p->truesize += skb->truesize;
40184021
p->len += skb->len;
40194022

4023+
skb_shinfo(p)->flags |= skb_shinfo(skb)->flags & SKBFL_SHARED_FRAG;
4024+
40204025
NAPI_GRO_CB(skb)->same_flow = 1;
40214026

40224027
return 0;
@@ -4251,7 +4256,8 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
42514256
skb_copy_from_linear_data_offset(head_skb, offset,
42524257
skb_put(nskb, hsize), hsize);
42534258

4254-
skb_shinfo(nskb)->flags |= skb_shinfo(head_skb)->flags &
4259+
skb_shinfo(nskb)->flags |= (skb_shinfo(head_skb)->flags |
4260+
skb_shinfo(frag_skb)->flags) &
42554261
SKBFL_SHARED_FRAG;
42564262

42574263
if (skb_zerocopy_clone(nskb, frag_skb, GFP_ATOMIC))
@@ -4268,6 +4274,9 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
42684274
nfrags = skb_shinfo(list_skb)->nr_frags;
42694275
frag = skb_shinfo(list_skb)->frags;
42704276
frag_skb = list_skb;
4277+
4278+
skb_shinfo(nskb)->flags |= skb_shinfo(frag_skb)->flags & SKBFL_SHARED_FRAG;
4279+
42714280
if (!skb_headlen(list_skb)) {
42724281
BUG_ON(!nfrags);
42734282
} else {
@@ -4490,10 +4499,12 @@ int skb_gro_receive(struct sk_buff *p, struct sk_buff *skb)
44904499
p->data_len += len;
44914500
p->truesize += delta_truesize;
44924501
p->len += len;
4502+
skb_shinfo(p)->flags |= skbinfo->flags & SKBFL_SHARED_FRAG;
44934503
if (lp != p) {
44944504
lp->data_len += len;
44954505
lp->truesize += delta_truesize;
44964506
lp->len += len;
4507+
skb_shinfo(lp)->flags |= skbinfo->flags & SKBFL_SHARED_FRAG;
44974508
}
44984509
NAPI_GRO_CB(skb)->same_flow = 1;
44994510
return 0;

0 commit comments

Comments
 (0)