Bug 1477491 - tc filter does not fail on malformed bytecode file
tc filter does not fail on malformed bytecode file
Status: CLOSED ERRATA
Product: Red Hat Enterprise Linux 7
Classification: Red Hat
Component: iproute (Show other bugs)
7.4-Alt
Unspecified Unspecified
high Severity high
: rc
: ---
Assigned To: Phil Sutter
Jaroslav Aster
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2017-08-02 05:09 EDT by Jaroslav Aster
Modified: 2017-11-09 06:25 EST (History)
2 users (show)

See Also:
Fixed In Version: iproute-4.11.0-3.el7a
Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2017-11-09 06:25:03 EST
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
Regression reproducer. (738 bytes, application/x-shellscript)
2017-08-28 11:04 EDT, Jaroslav Aster
no flags Details

  None (edit)
Description Jaroslav Aster 2017-08-02 05:09:49 EDT
Description of problem:

tc filter does not fail on malformed bytecode file. It reads only first line, creates filter and ignores other line.


Version-Release number of selected component (if applicable):

iproute-4.11.0-1.el7a


How reproducible:

100%


Steps to Reproduce:

iproute-4.11.0-1.el7a

# cat /tmp/tmp.fejlMbWcco
7,40 0 0 12,21 0 4 2048,37 3 0 1500,48 0 0 14,21 0 1 66,6 0 0 262144,6 0 0 0,
12,12 13,13 0 4
blabla

# tc filter add dev TestIface parent 1:0 bpf run bytecode-file /tmp/tmp.fejlMbWcco flowid 1:13

# tc filter show dev TestIface | grep 'flowid 1:13'
filter parent 1: protocol all pref 49152 bpf handle 0x1 flowid 1:13 bytecode '7,40 0 0 12,21 0 4 2048,37 3 0 1500,48 0 0 14,21 0 1 66,6 0 0 262144,6 0 0 0'


iproute-3.10.0-87.el7

# cat /tmp/tmp.BgIfvpoxVL
7,40 0 0 12,21 0 4 2048,37 3 0 1500,48 0 0 14,21 0 1 66,6 0 0 262144,6 0 0 0,
12,12 13,13 0 4
blabla

# tc filter add dev TestIface parent 1:0 bpf run bytecode-file /tmp/tmp.BgIfvpoxVL flowid 1:13'
Real program length exceeds encoded length parameter!
Illegal "bytecode"

# tc filter show dev TestIface | grep 'flowid 1:13'
<nothing>


Actual results:

tc filter reads malformed bytecode file and creates filter instead of fails and creates nothing.


Expected results:

tc filter reports error, returns non-zero return code and creates nothing.
Comment 2 Phil Sutter 2017-08-02 09:23:33 EDT
In fact, this is not a regression: The test case previously failed not because
of the invalid content on the second line, but because of the trailing newline
on the first one - the BPF parser never read more than a single line from the
provided bytecode file.

What has changed with the rebase is that the parser now removes any trailing
newline from that first line. This is a good thing per se, becase editors
commonly add that when editing a file (even if it contains just a single line)
anyway.

So in this particular case, the test case should be adjusted to append the
invalid bytecode part to the same line, not write a new line to the file.

Nevertheless I have submitted a patch upstream which enables the parser to
detect consecutive lines of bytecode and treat them correctly (which
implicitly also allows to split bytecode files over multiple lines):

http://marc.info/?l=linux-netdev&m=150167869213237&w=2

Cheers, Phil
Comment 3 Phil Sutter 2017-08-04 03:04:47 EDT
Upstream accepted my patch:

commit 3da3ebfca85b8f1e8252b898453d8cb383c5c398
Author: Phil Sutter <phil@nwl.cc>
Date:   Wed Aug 2 14:57:56 2017 +0200

    bpf: Make bytecode-file reading a little more robust
    
    bpf_parse_string() will now correctly handle:
    
    - Extraneous whitespace,
    - OPs on multiple lines and
    - overlong file names.
    
    The added feature of allowing to have OPs on multiple lines (like e.g.
    tcpdump prints them) is rather a side effect of fixing detection of
    malformed bytecode files having random content on a second line, like
    e.g.:
    
    | 4,40 0 0 12,21 0 1 2048,6 0 0 262144,6 0 0 0
    | foobar
    
    Cc: Daniel Borkmann <daniel@iogearbox.net>
    Signed-off-by: Phil Sutter <phil@nwl.cc>
    Acked-by: Daniel Borkmann <daniel@iogearbox.net>
Comment 5 Jaroslav Aster 2017-08-28 11:03:25 EDT
Hi Phil,

unfortunately I found a regression in the patch. It has stopped to work on correct bytecode file. See the reproducer I have attached.

iproute-4.11.0-1.el7a (old)

# cat /tmp/bytecode.txt 
7,40 0 0 12,21 0 4 2048,37 3 0 1500,48 0 0 14,21 0 1 66,6 0 0 262144,6 0 0 0,
# tc filter add dev TestIface parent 1:0 bpf run bytecode "$(cat /tmp/bytecode.txt)" flowid 1:11
# tc filter add dev TestIface parent 1:0 bpf run bytecode-file /tmp/bytecode.txt flowid 1:13


iproute-4.11.0-2.el7a (new)

# cat /tmp/bytecode.txt 
7,40 0 0 12,21 0 4 2048,37 3 0 1500,48 0 0 14,21 0 1 66,6 0 0 262144,6 0 0 0,
# tc filter add dev TestIface parent 1:0 bpf run bytecode "$(cat /tmp/bytecode.txt)" flowid 1:11
# tc filter add dev TestIface parent 1:0 bpf run bytecode-file /tmp/bytecode.txt flowid 1:13
Real program length exceeds encoded length parameter!
Comment 6 Jaroslav Aster 2017-08-28 11:04 EDT
Created attachment 1319118 [details]
Regression reproducer.
Comment 7 Phil Sutter 2017-08-29 11:15:21 EDT
Bug found, seems to happen on certain architectures only. Fix submitted upstream:
https://marc.info/?l=linux-netdev&m=150401943515102&w=2
Comment 10 errata-xmlrpc 2017-11-09 06:25:03 EST
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, 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/RHEA-2017:3172

Note You need to log in before you can comment on or make changes to this bug.