Skip to content

net: permit skb_segment on head_frag frag_list skb

Yonghong Song netdev 6 versions – In review

Get series with b4
b4 am [email protected]

Versions newest first · linked by author and title

  1. v6 Latest2 patches 2 repliesIn review
  2. v52 patches 2 repliesSuperseded
  3. v42 patches 3 repliesSuperseded
Earlier versions · 3
VersionPostedPatchesRepliesStatus
v32 patches6 repliesSuperseded
v22 patches2 repliesSuperseded
v12 patches3 repliesSuperseded

Changes in v6 from the cover letter

. Added back missed BUG_ON(!nfrags) for zero
skb_headlen(skb) case, plus a couple of
cosmetic changes, from Alexander.

Patches in v6 No applies recorded yet

#Subject and filesSizeRepliesApplied
1/2net: permit skb_segment on head_frag frag_list skbnet/core/skbuff.c+22 -51 reply
2/2net: bpf: add a test for skb_segment in test_bpf modulelib/test_bpf.c+91 -20 replies

Reviews trailers per patch

1 of 2 patches has a Reviewed-by or Acked-by.

PatchAlexander Duyck
1/2R-b
2/2

Trailers given in replies are filled; those carried in the patches from earlier versions are outlined with dashes.

Read the v6 thread

Cover letter [PATCH net-next v6 0/2] net: permit skb_segment on head_frag frag_list skb

One of our in-house projects, bpf-based NAT, hits a kernel BUG_ON at function skb_segment(), line 3667. The bpf program attaches to clsact ingress, calls bpf_skb_change_proto to change protocol from ipv4 to ipv6 or from ipv6 to ipv4, and then calls bpf_redirect to send the changed packet out.
...

    3665                 while (pos < offset + len) {
    3666                         if (i >= nfrags) {
    3667                                 BUG_ON(skb_headlen(list_skb));

...

The triggering input skb has the following properties:

    list_skb = skb->frag_list;
    skb->nfrags != NULL && skb_headlen(list_skb) != 0

and skb_segment() is not able to handle a frag_list skb if its headlen (list_skb->len - list_skb->data_len) is not 0.

Patch #1 provides a simple solution to avoid BUG_ON. If list_skb->head_frag is true, its page-backed frag will be processed before the list_skb->frags.
Patch #2 provides a test case in test_bpf module which constructs a skb and calls skb_segment() directly. The test case is able to trigger the BUG_ON without Patch #1.

The patch has been tested in the following setup:
ipv6_host <-> nat_server <-> ipv4_host
where nat_server has a bpf program doing ipv4<->ipv6 translation and forwarding through clsact hook bpf_skb_change_proto.

Changelog:
v5 -> v6:
. Added back missed BUG_ON(!nfrags) for zero
skb_headlen(skb) case, plus a couple of
cosmetic changes, from Alexander.
v4 -> v5:
. Replace local variable head_frag with
a static inline function skb_head_frag_to_page_desc which gets the head_frag on-demand. This makes code more readable and also does not increase the stack size, from Alexander.
. Remove the "if(nfrags)" guard for skb_orphan_frags
and skb_zerocopy_clone as I found that they can handle zero-frag skb (with non-zero skb_headlen(skb)) properly.
. Properly release segment list from skb_segment()
in the test, from Eric.
v3 -> v4:
. Remove dynamic memory allocation and use rewinding
for both index and frag to remove one branch in fast path, from Alexander.
. Fix a bunch of issues in test_bpf skb_segment() test,
including proper way to allocate skb, proper function argument for skb_add_rx_frag and not freeint skb, etc., from Eric.
v2 -> v3:
. Use starting frag index -1 (instead of 0) to
special process head_frag before other frags in the skb, from Alexander Duyck.
v1 -> v2:
. Removed never-hit BUG_ON, spotted by Linyu Yuan.

Yonghong Song (2):
net: permit skb_segment on head_frag frag_list skb
net: bpf: add a test for skb_segment in test_bpf module

 lib/test_bpf.c    | 93 +++++++++++++++++++++++++++++++++++++++++++++++++++++--
 net/core/skbuff.c | 27 +++++++++++++---
 2 files changed, 113 insertions(+), 7 deletions(-)
Signature · 1 line
2.9.5