lkml.org 
[lkml]   [2019]   [May]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 4.19 086/114] xfrm: clean up xfrm protocol checks
    Date
    [ Upstream commit dbb2483b2a46fbaf833cfb5deb5ed9cace9c7399 ]

    In commit 6a53b7593233 ("xfrm: check id proto in validate_tmpl()")
    I introduced a check for xfrm protocol, but according to Herbert
    IPSEC_PROTO_ANY should only be used as a wildcard for lookup, so
    it should be removed from validate_tmpl().

    And, IPSEC_PROTO_ANY is expected to only match 3 IPSec-specific
    protocols, this is why xfrm_state_flush() could still miss
    IPPROTO_ROUTING, which leads that those entries are left in
    net->xfrm.state_all before exit net. Fix this by replacing
    IPSEC_PROTO_ANY with zero.

    This patch also extracts the check from validate_tmpl() to
    xfrm_id_proto_valid() and uses it in parse_ipsecrequest().
    With this, no other protocols should be added into xfrm.

    Fixes: 6a53b7593233 ("xfrm: check id proto in validate_tmpl()")
    Reported-by: syzbot+0bf0519d6e0de15914fe@syzkaller.appspotmail.com
    Cc: Steffen Klassert <steffen.klassert@secunet.com>
    Cc: Herbert Xu <herbert@gondor.apana.org.au>
    Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
    Acked-by: Herbert Xu <herbert@gondor.apana.org.au>
    Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
    Signed-off-by: Sasha Levin <sashal@kernel.org>
    ---
    include/net/xfrm.h | 17 +++++++++++++++++
    net/ipv6/xfrm6_tunnel.c | 2 +-
    net/key/af_key.c | 4 +++-
    net/xfrm/xfrm_state.c | 2 +-
    net/xfrm/xfrm_user.c | 14 +-------------
    5 files changed, 23 insertions(+), 16 deletions(-)

    diff --git a/include/net/xfrm.h b/include/net/xfrm.h
    index 5e3daf53b3d1e..3e966c632f3b2 100644
    --- a/include/net/xfrm.h
    +++ b/include/net/xfrm.h
    @@ -1430,6 +1430,23 @@ static inline int xfrm_state_kern(const struct xfrm_state *x)
    return atomic_read(&x->tunnel_users);
    }

    +static inline bool xfrm_id_proto_valid(u8 proto)
    +{
    + switch (proto) {
    + case IPPROTO_AH:
    + case IPPROTO_ESP:
    + case IPPROTO_COMP:
    +#if IS_ENABLED(CONFIG_IPV6)
    + case IPPROTO_ROUTING:
    + case IPPROTO_DSTOPTS:
    +#endif
    + return true;
    + default:
    + return false;
    + }
    +}
    +
    +/* IPSEC_PROTO_ANY only matches 3 IPsec protocols, 0 could match all. */
    static inline int xfrm_id_proto_match(u8 proto, u8 userproto)
    {
    return (!userproto || proto == userproto ||
    diff --git a/net/ipv6/xfrm6_tunnel.c b/net/ipv6/xfrm6_tunnel.c
    index 12cb3aa990af4..d9e5f6808811a 100644
    --- a/net/ipv6/xfrm6_tunnel.c
    +++ b/net/ipv6/xfrm6_tunnel.c
    @@ -345,7 +345,7 @@ static void __net_exit xfrm6_tunnel_net_exit(struct net *net)
    unsigned int i;

    xfrm_flush_gc();
    - xfrm_state_flush(net, IPSEC_PROTO_ANY, false, true);
    + xfrm_state_flush(net, 0, false, true);

    for (i = 0; i < XFRM6_TUNNEL_SPI_BYADDR_HSIZE; i++)
    WARN_ON_ONCE(!hlist_empty(&xfrm6_tn->spi_byaddr[i]));
    diff --git a/net/key/af_key.c b/net/key/af_key.c
    index 7d4bed9550605..0b79c9aa8eb1f 100644
    --- a/net/key/af_key.c
    +++ b/net/key/af_key.c
    @@ -1951,8 +1951,10 @@ parse_ipsecrequest(struct xfrm_policy *xp, struct sadb_x_ipsecrequest *rq)

    if (rq->sadb_x_ipsecrequest_mode == 0)
    return -EINVAL;
    + if (!xfrm_id_proto_valid(rq->sadb_x_ipsecrequest_proto))
    + return -EINVAL;

    - t->id.proto = rq->sadb_x_ipsecrequest_proto; /* XXX check proto */
    + t->id.proto = rq->sadb_x_ipsecrequest_proto;
    if ((mode = pfkey_mode_to_xfrm(rq->sadb_x_ipsecrequest_mode)) < 0)
    return -EINVAL;
    t->mode = mode;
    diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
    index 3f729cd512aff..11e09eb138d60 100644
    --- a/net/xfrm/xfrm_state.c
    +++ b/net/xfrm/xfrm_state.c
    @@ -2386,7 +2386,7 @@ void xfrm_state_fini(struct net *net)

    flush_work(&net->xfrm.state_hash_work);
    flush_work(&xfrm_state_gc_work);
    - xfrm_state_flush(net, IPSEC_PROTO_ANY, false, true);
    + xfrm_state_flush(net, 0, false, true);

    WARN_ON(!list_empty(&net->xfrm.state_all));

    diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
    index 060afc4ffd958..2122f89f61555 100644
    --- a/net/xfrm/xfrm_user.c
    +++ b/net/xfrm/xfrm_user.c
    @@ -1513,20 +1513,8 @@ static int validate_tmpl(int nr, struct xfrm_user_tmpl *ut, u16 family)
    return -EINVAL;
    }

    - switch (ut[i].id.proto) {
    - case IPPROTO_AH:
    - case IPPROTO_ESP:
    - case IPPROTO_COMP:
    -#if IS_ENABLED(CONFIG_IPV6)
    - case IPPROTO_ROUTING:
    - case IPPROTO_DSTOPTS:
    -#endif
    - case IPSEC_PROTO_ANY:
    - break;
    - default:
    + if (!xfrm_id_proto_valid(ut[i].id.proto))
    return -EINVAL;
    - }
    -
    }

    return 0;
    --
    2.20.1


    \
     
     \ /
      Last update: 2019-05-23 21:43    [W:4.249 / U:0.628 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site