Skip to content

Commit e94ee17

Browse files
herbertxklassert
authored andcommitted
xfrm: Use correct address family in xfrm_state_find
The struct flowi must never be interpreted by itself as its size depends on the address family. Therefore it must always be grouped with its original family value. In this particular instance, the original family value is lost in the function xfrm_state_find. Therefore we get a bogus read when it's coupled with the wrong family which would occur with inter- family xfrm states. This patch fixes it by keeping the original family value. Note that the same bug could potentially occur in LSM through the xfrm_state_pol_flow_match hook. I checked the current code there and it seems to be safe for now as only secid is used which is part of struct flowi_common. But that API should be changed so that so that we don't get new bugs in the future. We could do that by replacing fl with just secid or adding a family field. Reported-by: syzbot+577fbac3145a6eb2e7a5@syzkaller.appspotmail.com Fixes: 48b8d78 ("[XFRM]: State selection update to use inner...") Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au> Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
1 parent 8366685 commit e94ee17

File tree

1 file changed

+7
-4
lines changed

1 file changed

+7
-4
lines changed

net/xfrm/xfrm_state.c

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1019,7 +1019,8 @@ static void xfrm_state_look_at(struct xfrm_policy *pol, struct xfrm_state *x,
10191019
*/
10201020
if (x->km.state == XFRM_STATE_VALID) {
10211021
if ((x->sel.family &&
1022-
!xfrm_selector_match(&x->sel, fl, x->sel.family)) ||
1022+
(x->sel.family != family ||
1023+
!xfrm_selector_match(&x->sel, fl, family))) ||
10231024
!security_xfrm_state_pol_flow_match(x, pol, fl))
10241025
return;
10251026

@@ -1032,7 +1033,9 @@ static void xfrm_state_look_at(struct xfrm_policy *pol, struct xfrm_state *x,
10321033
*acq_in_progress = 1;
10331034
} else if (x->km.state == XFRM_STATE_ERROR ||
10341035
x->km.state == XFRM_STATE_EXPIRED) {
1035-
if (xfrm_selector_match(&x->sel, fl, x->sel.family) &&
1036+
if ((!x->sel.family ||
1037+
(x->sel.family == family &&
1038+
xfrm_selector_match(&x->sel, fl, family))) &&
10361039
security_xfrm_state_pol_flow_match(x, pol, fl))
10371040
*error = -ESRCH;
10381041
}
@@ -1072,7 +1075,7 @@ xfrm_state_find(const xfrm_address_t *daddr, const xfrm_address_t *saddr,
10721075
tmpl->mode == x->props.mode &&
10731076
tmpl->id.proto == x->id.proto &&
10741077
(tmpl->id.spi == x->id.spi || !tmpl->id.spi))
1075-
xfrm_state_look_at(pol, x, fl, encap_family,
1078+
xfrm_state_look_at(pol, x, fl, family,
10761079
&best, &acquire_in_progress, &error);
10771080
}
10781081
if (best || acquire_in_progress)
@@ -1089,7 +1092,7 @@ xfrm_state_find(const xfrm_address_t *daddr, const xfrm_address_t *saddr,
10891092
tmpl->mode == x->props.mode &&
10901093
tmpl->id.proto == x->id.proto &&
10911094
(tmpl->id.spi == x->id.spi || !tmpl->id.spi))
1092-
xfrm_state_look_at(pol, x, fl, encap_family,
1095+
xfrm_state_look_at(pol, x, fl, family,
10931096
&best, &acquire_in_progress, &error);
10941097
}
10951098

0 commit comments

Comments
 (0)