On Mon, Mar 18, 2019 at 10:08:24PM -0700, Cong Wang wrote: > > +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 > + case IPSEC_PROTO_ANY: > + return true; > + default: > + return false; > + } > +} > + > static inline int xfrm_id_proto_match(u8 proto, u8 userproto) > { > return (!userproto || proto == userproto || > - (userproto == IPSEC_PROTO_ANY && (proto == IPPROTO_AH || > - proto == IPPROTO_ESP || > - proto == IPPROTO_COMP))); > + (userproto == IPSEC_PROTO_ANY && xfrm_id_proto_valid(proto))); > }
This does not look right. IPSEC_PROTO_ANY should only be allowed in userproto and your patch is going to let it pass when it's in proto. Whether IPPROTO_ROUTING/IPPROTO_DSTOPTS should be allowed in this context is also not obvious. Cheers, -- Email: Herbert Xu <herb...@gondor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt