Skip to content

Commit 39aa692

Browse files
Florian Westphalklassert
authored andcommitted
xfrm: policy: fix netlink/pf_key policy lookups
Colin Ian King says: Static analysis with CoverityScan found a potential issue [..] It seems that pointer pol is set to NULL and then a check to see if it is non-null is used to set pol to tmp; howeverm this check is always going to be false because pol is always NULL. Fix this and update test script to catch this. Updated script only: ./xfrm_policy.sh ; echo $? RTNETLINK answers: No such file or directory FAIL: ip -net ns3 xfrm policy get src 10.0.1.0/24 dst 10.0.2.0/24 dir out RTNETLINK answers: No such file or directory [..] PASS: policy before exception matches PASS: ping to .254 bypassed ipsec tunnel PASS: direct policy matches PASS: policy matches 1 Fixes: 6be3b0d ("xfrm: policy: add inexact policy search tree infrastructure") Reported-by: Colin Ian King <[email protected]> Signed-off-by: Florian Westphal <[email protected]> Signed-off-by: Steffen Klassert <[email protected]>
1 parent 7759d6a commit 39aa692

File tree

2 files changed

+36
-7
lines changed

2 files changed

+36
-7
lines changed

net/xfrm/xfrm_policy.c

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1663,7 +1663,10 @@ struct xfrm_policy *xfrm_policy_bysel_ctx(struct net *net, u32 mark, u32 if_id,
16631663
tmp = __xfrm_policy_bysel_ctx(cand.res[i], mark,
16641664
if_id, type, dir,
16651665
sel, ctx);
1666-
if (tmp && pol && tmp->pos < pol->pos)
1666+
if (!tmp)
1667+
continue;
1668+
1669+
if (!pol || tmp->pos < pol->pos)
16671670
pol = tmp;
16681671
}
16691672
} else {

tools/testing/selftests/net/xfrm_policy.sh

Lines changed: 32 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
# Kselftest framework requirement - SKIP code is 4.
2222
ksft_skip=4
2323
ret=0
24+
policy_checks_ok=1
2425

2526
KEY_SHA=0xdeadbeef1234567890abcdefabcdefabcdefabcd
2627
KEY_AES=0x0123456789abcdef0123456789012345
@@ -45,6 +46,26 @@ do_esp() {
4546
ip -net $ns xfrm policy add src $rnet dst $lnet dir fwd tmpl src $remote dst $me proto esp mode tunnel priority 100 action allow
4647
}
4748

49+
do_esp_policy_get_check() {
50+
local ns=$1
51+
local lnet=$2
52+
local rnet=$3
53+
54+
ip -net $ns xfrm policy get src $lnet dst $rnet dir out > /dev/null
55+
if [ $? -ne 0 ] && [ $policy_checks_ok -eq 1 ] ;then
56+
policy_checks_ok=0
57+
echo "FAIL: ip -net $ns xfrm policy get src $lnet dst $rnet dir out"
58+
ret=1
59+
fi
60+
61+
ip -net $ns xfrm policy get src $rnet dst $lnet dir fwd > /dev/null
62+
if [ $? -ne 0 ] && [ $policy_checks_ok -eq 1 ] ;then
63+
policy_checks_ok=0
64+
echo "FAIL: ip -net $ns xfrm policy get src $rnet dst $lnet dir fwd"
65+
ret=1
66+
fi
67+
}
68+
4869
do_exception() {
4970
local ns=$1
5071
local me=$2
@@ -112,31 +133,31 @@ check_xfrm() {
112133
# 1: iptables -m policy rule count != 0
113134
rval=$1
114135
ip=$2
115-
ret=0
136+
lret=0
116137

117138
ip netns exec ns1 ping -q -c 1 10.0.2.$ip > /dev/null
118139

119140
check_ipt_policy_count ns3
120141
if [ $? -ne $rval ] ; then
121-
ret=1
142+
lret=1
122143
fi
123144
check_ipt_policy_count ns4
124145
if [ $? -ne $rval ] ; then
125-
ret=1
146+
lret=1
126147
fi
127148

128149
ip netns exec ns2 ping -q -c 1 10.0.1.$ip > /dev/null
129150

130151
check_ipt_policy_count ns3
131152
if [ $? -ne $rval ] ; then
132-
ret=1
153+
lret=1
133154
fi
134155
check_ipt_policy_count ns4
135156
if [ $? -ne $rval ] ; then
136-
ret=1
157+
lret=1
137158
fi
138159

139-
return $ret
160+
return $lret
140161
}
141162

142163
#check for needed privileges
@@ -227,6 +248,11 @@ do_esp ns4 dead:3::10 dead:3::1 dead:2::/64 dead:1::/64 $SPI2 $SPI1
227248
do_dummies4 ns3
228249
do_dummies6 ns4
229250

251+
do_esp_policy_get_check ns3 10.0.1.0/24 10.0.2.0/24
252+
do_esp_policy_get_check ns4 10.0.2.0/24 10.0.1.0/24
253+
do_esp_policy_get_check ns3 dead:1::/64 dead:2::/64
254+
do_esp_policy_get_check ns4 dead:2::/64 dead:1::/64
255+
230256
# ping to .254 should use ipsec, exception is not installed.
231257
check_xfrm 1 254
232258
if [ $? -ne 0 ]; then

0 commit comments

Comments
 (0)