Fix oops in skb_segment for Bionic series

Bug #1915552 reported by Guilherme G. Piccoli on 2021-02-12
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
linux (Ubuntu)
Medium
Guilherme G. Piccoli
Bionic
Medium
Guilherme G. Piccoli

Bug Description

[Impact]
* It was reported upstream [0] that an eBPF NAT64 filter caused an oops due to bad handling of GRO headers length on SKB segmentation path; the discussion is rich in details, and eventually the reporter sent a fix patch for that [1], as well as a test scenario in test_bpf kernel module that reproduces the issue.

[0] https://<email address hidden>/
[1] https://<email address hidden>/

* The fix patch landed on v4.17 and for some reason didn't reach the stable kernels; by testing our Bionic v4.15 kernel I was able to reproduce the issue, observing the following stack trace (details in the testing section below):

kernel BUG at net/core/skbuff.c:3703!
Modules linked in: test_bpf(E+) isofs nls_iso8859_1 dm_multipath scsi_dh_rdac scsi_dh_emc ...
RIP: 0010:skb_segment+0xa34/0xce0
[...]
Call Trace:
 test_bpf_init.part.7+0x767/0x7d1 [test_bpf]
 test_bpf_init+0xfc/0x82f [test_bpf]
 do_one_initcall+0x52/0x19f
[...]

* Interesting to mention that this fix is not complete in the sense there was another corner case reported after that [2], which was fixed by another patch [3], this one released in kernel v5.3 and present in the stable tree (hence backported to our Bionic 4.15 kernels).

[2] https://lore.kernel.org/netdev/20190826170724.25ff616f@pixies/
[3] http://git.kernel.org/linus/3dcbdb134f32 ("net: gso: Fix skb_segment splat when splitting gso_size mangled skb having linear-headed frag_list")

* So we are hereby backporting both the original fix patch [4] as well as the test_bpf patch (and a fix for it) [5] [6] for Ubuntu Bionic v4.15-based kernels

[4] http://git.kernel.org/linus/13acc94eff12 ("net: permit skb_segment on head_frag frag_list skb")
[5] http://git.kernel.org/linus/76db8087c4c9 ("net: bpf: add a test for skb_segment in test_bpf module")
[6] http://git.kernel.org/linus/99fe29d3a25f ("test_bpf: Fix NULL vs IS_ERR() check in test_skb_segment()")

[Test Case]
* One could use a NAT64 filter, but with the aforementioned patches [5] [6] hereby backported, one can also use the kernel infrastructure, by loading the test_bpf module:

insmod /lib/modules/$(uname -r)/kernel/lib/test_bpf.ko

If patches [5] [6] are included and kernel doesn't contain the fix [4], an oops will be observed.

[Where problems could occur]
* The backported patches are present upstream since v4.17, and no fixes were released for them (other than [6], included here), so from the testing point-of-view, these patches are being exercised for a while with no issues.

* That said, if a problem would be triggered by these patches, hypothetically it would affect SKB segmentation, the net/core code - a bad check could case an oops in this code or they could present a pretty small overhead due to more checks in the hot path.

Changed in linux-azure (Ubuntu Bionic):
status: New → In Progress
assignee: nobody → Guilherme G. Piccoli (gpiccoli)
importance: Undecided → Medium
Stefan Bader (smb) wrote :

The patches were submitted to Bionic main.

affects: linux-azure (Ubuntu) → linux (Ubuntu)
Changed in linux (Ubuntu):
status: In Progress → Invalid
Stefan Bader (smb) on 2021-02-15
Changed in linux (Ubuntu Bionic):
status: In Progress → Fix Committed

This bug is awaiting verification that the kernel in -proposed solves the problem. Please test the kernel and update this bug with the results. If the problem is solved, change the tag 'verification-needed-bionic' to 'verification-done-bionic'. If the problem still exists, change the tag 'verification-needed-bionic' to 'verification-failed-bionic'.

If verification is not done by 5 working days from today, this fix will be dropped from the source code, and this bug will be closed.

See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Thank you!

tags: added: verification-needed-bionic
Guilherme G. Piccoli (gpiccoli) wrote :

An user who reported the issue confirmed the kernel with the patches hereby proposed is not reproducing anymore; also, I checked in the Bionic git tree, patches are indeed there.
Cheers,

Guilherme

tags: added: verification-done-bionic
removed: verification-needed-bionic
Launchpad Janitor (janitor) wrote :
Download full text (11.7 KiB)

This bug was fixed in the package linux - 4.15.0-137.141

---------------
linux (4.15.0-137.141) bionic; urgency=medium

  * bionic/linux: 4.15.0-137.141 -proposed tracker (LP: #1916199)

  * Fix oops in skb_segment for Bionic series (LP: #1915552)
    - net: permit skb_segment on head_frag frag_list skb
    - net: bpf: add a test for skb_segment in test_bpf module
    - test_bpf: Fix NULL vs IS_ERR() check in test_skb_segment()

  * Bionic update: upstream stable patchset 2021-02-10 (LP: #1915328)
    - net: cdc_ncm: correct overhead in delayed_ndp_size
    - net: vlan: avoid leaks on register_vlan_dev() failures
    - net: ip: always refragment ip defragmented packets
    - net: fix pmtu check in nopmtudisc mode
    - x86/resctrl: Use an IPI instead of task_work_add() to update PQR_ASSOC MSR
    - x86/resctrl: Don't move a task to the same resource group
    - vmlinux.lds.h: Add PGO and AutoFDO input sections
    - drm/i915: Fix mismatch between misplaced vma check and vma insert
    - spi: pxa2xx: Fix use-after-free on unbind
    - iio: imu: st_lsm6dsx: flip irq return logic
    - iio: imu: st_lsm6dsx: fix edge-trigger interrupts
    - ARM: OMAP2+: omap_device: fix idling of devices during probe
    - i2c: sprd: use a specific timeout to avoid system hang up issue
    - cpufreq: powernow-k8: pass policy rather than use cpufreq_cpu_get()
    - spi: stm32: FIFO threshold level - fix align packet size
    - dmaengine: xilinx_dma: check dma_async_device_register return value
    - dmaengine: xilinx_dma: fix mixed_enum_type coverity warning
    - wil6210: select CONFIG_CRC32
    - block: rsxx: select CONFIG_CRC32
    - iommu/intel: Fix memleak in intel_irq_remapping_alloc
    - net/mlx5e: Fix memleak in mlx5e_create_l2_table_groups
    - net/mlx5e: Fix two double free cases
    - wan: ds26522: select CONFIG_BITREVERSE
    - KVM: arm64: Don't access PMCR_EL0 when no PMU is available
    - block: fix use-after-free in disk_part_iter_next
    - net: drop bogus skb with CHECKSUM_PARTIAL and offset beyond end of trimmed
      packet
    - net: hns3: fix the number of queues actually used by ARQ
    - net: stmmac: dwmac-sun8i: Balance internal PHY resource references
    - net: stmmac: dwmac-sun8i: Balance internal PHY power
    - net/sonic: Fix some resource leaks in error handling paths
    - net: ipv6: fib: flush exceptions when purging route
    - dmaengine: xilinx_dma: fix incompatible param warning in _child_probe()
    - lightnvm: select CONFIG_CRC32
    - ASoC: dapm: remove widget from dirty list on free
    - MIPS: boot: Fix unaligned access with CONFIG_MIPS_RAW_APPENDED_DTB
    - MIPS: relocatable: fix possible boot hangup with KASLR enabled
    - ACPI: scan: Harden acpi_device_add() against device ID overflows
    - mm/hugetlb: fix potential missing huge page size info
    - dm snapshot: flush merged data before committing metadata
    - r8152: Add Lenovo Powered USB-C Travel Hub
    - ext4: fix bug for rename with RENAME_WHITEOUT
    - ARC: build: remove non-existing bootpImage from KBUILD_IMAGE
    - ARC: build: add uImage.lzma to the top-level target
    - ARC: build: add boot_targets to PHONY
    - btrfs: fix transaction leak and crash...

Changed in linux (Ubuntu Bionic):
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers