Bug 2033505
| Summary: | tc u32 ematch configuration might fail even nexthdr offset is aligned to 4 | ||
|---|---|---|---|
| Product: | Red Hat Enterprise Linux 8 | Reporter: | Alfred Yang <alf.redyoung> |
| Component: | iproute | Assignee: | Andrea Claudi <aclaudi> |
| Status: | CLOSED ERRATA | QA Contact: | liujian <jianliu> |
| Severity: | unspecified | Docs Contact: | |
| Priority: | unspecified | ||
| Version: | 8.7 | CC: | jiji, mleitner, network-qe, shuali |
| Target Milestone: | rc | Keywords: | Triaged |
| Target Release: | 8.7 | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
| Whiteboard: | |||
| Fixed In Version: | iproute-5.18.0-1.el8 | Doc Type: | If docs needed, set a value |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2022-11-08 10:52:53 UTC | Type: | Bug |
| Regression: | --- | Mount Type: | --- |
| Documentation: | --- | CRM: | |
| Verified Versions: | Category: | --- | |
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
| Cloudforms Team: | --- | Target Upstream Version: | |
| Embargoed: | |||
| Bug Depends On: | 2074607 | ||
| Bug Blocks: | |||
From investigation and run by valgrind with debug info, we can see below output, of coz, if it is run by valgrind, the issue cannot be reproduced.
==29412== Use of uninitialised value of size 8
==29412== at 0x53786CD: ____strtoul_l_internal (strtol_l.c:461)
==29412== by 0x43246C: u32_parse_eopt (em_u32.c:89)
==29412== by 0x412921: parse_tree (m_ematch.c:216)
==29412== by 0x412921: parse_ematch (m_ematch.c:352)
==29412== by 0x42197D: basic_parse_opt (f_basic.c:65)
==29412== by 0x40B915: tc_filter_modify (tc_filter.c:196)
==29412== by 0x407B3F: main (tc.c:348)
the codes are in em_u32.c, the function u32_parse_eopt()
83 nh_len = strlen("nexthdr+");
84 if (a->len > nh_len && !memcmp(a->data, "nexthdr+", nh_len)) {
85 char buf[a->len - nh_len + 1];
86 memset(buf, 0, a->len - nh_len + 1); <<<- this line is added by me to fix the issue
87 offmask = -1;
88 memcpy(buf, a->data + nh_len, a->len - nh_len);
89 offset = strtoul(buf, NULL, 0);
90 } else if (!bstrcmp(a, "nexthdr+")) {
We can see with line 86, buf might contain dirty data, line 88 only copy string but not the null terminator. This causes reading uninitialized data.
Oh, just find a simpler and more efficient fix might be explicitly set buf[a-len - nh_len] = 0 before strtoul.
This issue is still present on rhel-8.7, but is not critical enough to qualify for a rhel-7 fix. I'm moving this to rhel-8.7 to have this fixed there. Patch submitted upstream: https://patchwork.kernel.org/project/netdevbpf/patch/5ceaf48253d515b8c8e0902d939471b3a95f5407.1651867575.git.aclaudi@redhat.com/ A TC-related bug, Liu Jian will help to deal with it. Hi Liu Jian, Sorry for the late reaction, please set an acceptable ITM for you. Since the problem described in this bug report should be resolved in a recent advisory, it has been closed with a resolution of ERRATA. For information on the advisory (iproute bug fix and enhancement update), and where to find the updated files, follow the link below. If the solution does not work for you, open a new bug report. https://access.redhat.com/errata/RHBA-2022:7752 |
Description of problem: tc u32 ematch configuration might fail even nexthdr offset is aligned to 4 Version-Release number of selected component (if applicable): We've tested iproute-4.11.0-30.el7.x86_64 and -25 and -14 version. How reproducible: I use a simple script to constantly reproduce it. (Note: it is a dead loop) #!/bin/sh tc qdisc del dev dummy0 root tc qdisc add dev dummy0 root handle 1: htb r2q 1 default 1 tc class add dev dummy0 parent 1:1 classid 1:108 htb quantum 1000000 rate 1.00mbit ceil 10.00mbit burst 6k TC="valgrind -q tc" TC=tc #TC=./tc c=0 while true; do if ! $TC filter add dev dummy0 protocol all parent 1: prio 1 basic match "meta(vlan mask 0xfff eq 1)" and "u32(u32 0x20011002 0xffffffff at nexthdr+8)" flowid 1:108; then echo add $c exit 0 fi if ! $TC filter del dev dummy0 protocol all parent 1: prio 1 basic match "meta(vlan mask 0xfff eq 1)" and "u32(u32 0x20011002 0xffffffff at nexthdr+8)" flowid 1:108; then echo del $c exit 0 fi c=$(expr $c + 1) done Steps to Reproduce: 1. modprobe dummy 2. run above script Actual results: script will fail and exit. Expected results: no fail, run forever. Additional info: Error output is: u32: invalid offset alignment, must be aligned to 4. ... meta(vlan mask 0xfff eq 1) and >>u32(u32 0x20011002 0xffffffff at nexthdr+8)<< ... ... u32(u32 0x20011002 0xffffffff at >>nexthdr+8<<)... Usage: u32(ALIGN VALUE MASK at [ nexthdr+ ] OFFSET) where: ALIGN := { u8 | u16 | u32 } Example: u32(u16 0x1122 0xffff at nexthdr+4) Illegal "ematch"