On 6/23/20 10:03 AM, Yonghong Song wrote: > > > On 6/23/20 9:27 AM, Eric Dumazet wrote: >> >> >> On 6/22/20 7:22 PM, Yonghong Song wrote: >>> >>> >>> On 6/22/20 6:47 PM, Eric Dumazet wrote: >> & >>>> >>>> Why is the sk_fullsock(sk) needed ? >>> >>> The parameter 'sk' could be a sock_common. That is why the >>> helper name bpf_skc_to_udp6_sock implies. The sock_common cannot >>> access sk_protocol, hence we requires sk_fullsock(sk) here. >>> Did I miss anything? >> >> OK, if arbitrary sockets can land here, you need also to check sk_type > > The current check is: > if (sk_fullsock(sk) && sk->sk_protocol == IPPROTO_UDP && > sk->sk_family == AF_INET6) > return (unsigned long)sk; > it checks to ensure it is full socket, it is a ipv6 socket and then check protocol. > > Are you suggesting to add the following check? > sk->sk_type == SOCK_DGRAM > > Not a networking expert. Maybe you can explain when we could have > protocol is IPPROTO_UDP and sk_type not SOCK_DGRAM? RAW sockets for instance. Look at : commit 940ba14986657a50c15f694efca1beba31fa568f Author: Eric Dumazet <edumazet@xxxxxxxxxx> Date: Tue Jan 21 23:17:14 2020 -0800 gtp: make sure only SOCK_DGRAM UDP sockets are accepted A malicious user could use RAW sockets and fool GTP using them as standard SOCK_DGRAM UDP sockets.