Messages in this thread | | | From | Jinmeng Zhou <> | Date | Thu, 16 Sep 2021 16:21:02 +0800 | Subject | A missing check bug before calling ip_route_output_flow() |
| |
Dear maintainers,
hi, our team has found and reported a missing check bug on Linux kernel v5.10.7 using static analysis. We are looking forward to having more experts' eyes on this. Thank you!
Function ipv4_sk_update_pmtu() lacks a LSM check, a.k.a security_sk_classify_flow(), before calling ip_route_output_flow().
Specifically, we find that ip_route_output_flow() is used at 18 places in total. In most cases, either the function is placed behind the security check security_sk_classify_flow() or its last parameter is NULL. (i.e., if the last parameter of ip_route_output_flow() is NULL, usually, there may be no need to do a security check.)
However, we find only two usages in function ipv4_sk_update_pmtu(), the last parameter is not NULL as well as no security check.
1. void ipv4_sk_update_pmtu(struct sk_buff *skb, struct sock *sk, u32 mtu) 2. { 3. ... 4. if (odst->obsolete && !odst->ops->check(odst, 0)) { 5. rt = ip_route_output_flow(sock_net(sk), &fl4, sk); 6. if (IS_ERR(rt)) 7. goto out; 8. 9. new = true; 10. } 11. 12. __ip_rt_update_pmtu((struct rtable *)xfrm_dst_path(&rt->dst), &fl4, mtu); 13. 14. if (!dst_check(&rt->dst, 0)) { 15. if (new) 16. dst_release(&rt->dst); 17. 18. rt = ip_route_output_flow(sock_net(sk), &fl4, sk); 19. if (IS_ERR(rt)) 20. goto out; 21. 22. new = true; 23. } 24. ... 25. }
ipv4_sk_update_pmtu() is called by 3 callers, ping_err(), raw_err(), __udp4_lib_err(). They are likely to do the error handling. Therefore, we think there is a missing check bug before calling ip_route_output_flow(). --------------------------------------------------------------------------------- Original email:
> Thu, 6 May 2021 11:01:24 -0700 Jakub Kicinski <kuba@kernel.org> wrote: > > On Thu, 6 May 2021 15:50:33 +0800 Jinmeng Zhou wrote: > > hi, our team has found a missing check bug on Linux kernel v5.10.7 using > static analysis. > > We think there is a missing check bug in ip_route_output_key() before calling > function ip_route_output_flow(). > > Thank you for the report! > > > There is a check calls to security_sk_classify_flow() in function ip_route_newports(). > > 1. // check security_sk_classify_flow() /////////////// > > 2. static inline struct rtable *ip_route_newports(struct flowi4 *fl4, struct rtable *rt, > > 3. __be16 orig_sport, __be16 orig_dport, > > 4. __be16 sport, __be16 dport, > > 5. struct sock *sk) > > 6. { > > 7. ... > > 8. security_sk_classify_flow(sk, flowi4_to_flowi(fl4)); > > 9. return ip_route_output_flow(sock_net(sk), fl4, sk); > > 10. ... > > 11. } > > > > While, ip_route_output_key() does not have check. > > 1. // no check //////////////////////////////////// > > 2. static inline struct rtable *ip_route_output_key(struct net *net, struct flowi4 *flp) > > 3. { > > 4. return ip_route_output_flow(net, flp, NULL); > > 5. } > > > > On the path from user-reachable function to ip_route_output_key() also does not contain this check. There is a call chain: > > nft_reject_ipv4_eval() => > > nf_send_reset() => > > This path looks like ICMP reject path, so it's not run in a context of > any process, I'm not sure security checks make sense in such context. > But again please circulate the report more widely, add people who have > touched the code in the past and relevant mailing lists. > > > ip_route_me_harder() => > > ip_route_output_key() > > > > 1. static const struct nft_expr_ops nft_reject_ipv4_ops = { > > 2. > > 3. .eval = nft_reject_ipv4_eval, > > 4. > > 5. }; > > 6. static int __init nft_reject_ipv4_module_init(void) > > 7. { > > 8. return nft_register_expr(&nft_reject_ipv4_type); > > 9. } > > 10. module_init(nft_reject_ipv4_module_init); > > > > Therefore we think the buggy function can be triggered. > > > > Thanks! > > > > > > Best regards, > > Jinmeng Zhou
Thanks again!
Best regards, Jinmeng Zhou
| |