netdev - TC filter rule not doing an atomic replace for tc-matchall filter type

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [day] [month] [year] [list]
Message-ID: <CAFxY_EsE6t_ezpQ5HyiTc_PhLAkX=hbEO1b77JLKihuEOm7VZA@mail.gmail.com>
Date:   Tue, 8 Jun 2021 10:39:41 -0700
From:   Lalitha Sahitya Maruvada <sahitya.lalitha@...il.com>
To:     netdev@...r.kernel.org
Subject: TC filter rule not doing an atomic replace for tc-matchall filter type

I have a tc-matchall filter rule attached to a qdisc associated with a
wireguard device. When no tc filter rules are present, executing a tc
replace command, creates a new rule as expected. But if I try to
re-run the replace command without any changes (or with any changes),
it errors out with EEXIST. Ideally it should perform a nearly atomic
remove/add on an existing node id, or add it if it doesn't already
exist.

I also have a tc-u32 rule associated with the same qdisc, this rule
does atomic replace, as expected, without any errors.

I am suspecting it is specific to the matchall filter, There is a bug
reported with the u32 filter that is close to this, but not quite the
same.

 I have tried the matchall rule below on tc versions 5.4 and the
latest TC version 5.10 as well, but it errors out with File exists, we
have an error talking to the kernel.

Here is the set of commands used for replicating the scenario.

$ ip link add dev wg1 type wireguard
$ tc qdisc add dev wg1 root handle ffff: prio bands 2 priomap  0 0 0 0
0 0 0 0 0 0 0 0 0 0 0 0
$ tc filter replace dev wg1 parent ffff: prio 99 handle 800::800
protocol ip u32 match ip src 192.168.8.0/16 classid 1:1
$ tc filter replace dev wg1 parent ffff: prio 300  matchall action
sample rate 100 group 12 action mirred egress redirect dev ifb0

Output of tc filter show command after executing the above commands
looks like this

$ tc filter show dev wg1
filter parent ffff: protocol ip pref 99 u32 chain 0
filter parent ffff: protocol ip pref 99 u32 chain 0 fh 800: ht divisor 1
filter parent ffff: protocol ip pref 99 u32 chain 0 fh 800::800 order
2048 key ht 800 bkt 0 flowid 1:1 not_in_hw
  match c0a80000/ffff0000 at 12
filter parent ffff: protocol all pref 300 matchall chain 0
filter parent ffff: protocol all pref 300 matchall chain 0 handle 0x1
  not_in_hw
    action order 1: sample rate 1/100 group 12 pipe
     index 1 ref 1 bind 1

    action order 2: mirred (Egress Redirect to device ifb0) stolen
    index 6 ref 1 bind 1

Re-running the filter replace commands should ideally do an atomic
replace without any errors. The u32 filter behaves as expected, but
not the matchall filter. Here is the output when I re-run the
commands. I have a service that re-runs these commands when a few
conditions are met and it is essential that these commands shouldn't
error out for the successful functioning of the service.

$ tc filter replace dev wg1 parent ffff: prio 99 handle 800::800
protocol ip u32 match ip src 192.168.8.0/24 classid 1:1

# running this command again fails with the error below.
$ tc filter replace dev wg1 parent ffff: prio 300  matchall action
sample rate 100 group 12 action mirred egress redirect dev ifb0
RTNETLINK answers: File exists
We have an error talking to the kernel

On a side note, one other thing that is observed is, if I run a
replace without specifying the prio, it ends up creating a new rule
with a different prio

$ tc filter replace dev wg1 parent ffff: matchall action sample rate
100 group 12 action mirred egress redirect dev ifb0

$ tc filter replace dev wg1 parent ffff: matchall action sample rate
100 group 12 action mirred egress redirect dev ifb0

Output of tc filter show dev wg1 command, after running replace
without specifying a prio. This really isn't my concern right now, my
problem is replace erroring out with EEXIST when prio is specified.

$tc filter show dev wg1

filter parent ffff: protocol ip pref 99 u32 chain 0
filter parent ffff: protocol ip pref 99 u32 chain 0 fh 800: ht divisor 1
filter parent ffff: protocol ip pref 99 u32 chain 0 fh 800::800 order
2048 key ht 800 bkt 0 flowid 1:1 not_in_hw
  match c0a80000/ffff0000 at 12
filter parent ffff: protocol all pref 300 matchall chain 0
filter parent ffff: protocol all pref 300 matchall chain 0 handle 0x1
  not_in_hw
action order 1: sample rate 1/100 group 12 pipe
index 1 ref 1 bind 1

action order 2: mirred (Egress Redirect to device ifb0) stolen
index 6 ref 1 bind 1

filter parent ffff: protocol all pref 49151 matchall chain 0
filter parent ffff: protocol all pref 49151 matchall chain 0 handle 0x1
  not_in_hw
action order 1: sample rate 1/100 group 12 pipe
index 4 ref 1 bind 1

action order 2: mirred (Egress Redirect to device ifb0) stolen
index 10 ref 1 bind 1

filter parent ffff: protocol all pref 49152 matchall chain 0
filter parent ffff: protocol all pref 49152 matchall chain 0 handle 0x1
  not_in_hw
action order 1: sample rate 1/100 group 12 pipe
index 3 ref 1 bind 1

action order 2: mirred (Egress Redirect to device ifb0) stolen
index 9 ref 1 bind 1


This is the first time I am reporting a bug, apologies if I am not
clear. My previous mails with formatting were marked as spam by the
mail Delivery
system and hence sending it in plain text. Happy to clarify if
required.

Thank You!

Best,
Sahitya Maruvada.