| Message ID | 20230406031136.2814421-1-william.xuanziyang@huawei.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] ipv4: Fix potential uninit variable access buf in __ip_make_skb() | expand |
On Thu, Apr 6, 2023 at 5:11 AM Ziyang Xuan <william.xuanziyang@huawei.com> wrote: > > Like commit ea30388baebc ("ipv6: Fix an uninit variable access bug in > __ip6_make_skb()"). icmphdr does not in skb linear region under the > scenario of SOCK_RAW socket. Access icmp_hdr(skb)->type directly will > trigger the uninit variable access bug. > > Use a local variable icmp_type to carry the correct value in different > scenarios. > > Fixes: 96793b482540 ("[IPV4]: Add ICMPMsgStats MIB (RFC 4293)") > Signed-off-by: Ziyang Xuan <william.xuanziyang@huawei.com> > --- > net/ipv4/ip_output.c | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c > index 4e4e308c3230..57921b297a8e 100644 > --- a/net/ipv4/ip_output.c > +++ b/net/ipv4/ip_output.c > @@ -1570,9 +1570,15 @@ struct sk_buff *__ip_make_skb(struct sock *sk, > cork->dst = NULL; > skb_dst_set(skb, &rt->dst); > > - if (iph->protocol == IPPROTO_ICMP) > - icmp_out_count(net, ((struct icmphdr *) > - skb_transport_header(skb))->type); > + if (iph->protocol == IPPROTO_ICMP) { > + u8 icmp_type; > + > + if (sk->sk_socket->type == SOCK_RAW && !inet_sk(sk)->hdrincl) What is the reason for not using sk->sk_type ? > + icmp_type = fl4->fl4_icmp_type; > + else > + icmp_type = icmp_hdr(skb)->type; > + icmp_out_count(net, icmp_type); > + } > > ip_cork_release(cork); > out: > -- > 2.25.1 >
> On Thu, Apr 6, 2023 at 5:11 AM Ziyang Xuan > <william.xuanziyang@huawei.com> wrote: >> >> Like commit ea30388baebc ("ipv6: Fix an uninit variable access bug in >> __ip6_make_skb()"). icmphdr does not in skb linear region under the >> scenario of SOCK_RAW socket. Access icmp_hdr(skb)->type directly will >> trigger the uninit variable access bug. >> >> Use a local variable icmp_type to carry the correct value in different >> scenarios. >> >> Fixes: 96793b482540 ("[IPV4]: Add ICMPMsgStats MIB (RFC 4293)") >> Signed-off-by: Ziyang Xuan <william.xuanziyang@huawei.com> >> --- >> net/ipv4/ip_output.c | 12 +++++++++--- >> 1 file changed, 9 insertions(+), 3 deletions(-) >> >> diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c >> index 4e4e308c3230..57921b297a8e 100644 >> --- a/net/ipv4/ip_output.c >> +++ b/net/ipv4/ip_output.c >> @@ -1570,9 +1570,15 @@ struct sk_buff *__ip_make_skb(struct sock *sk, >> cork->dst = NULL; >> skb_dst_set(skb, &rt->dst); >> >> - if (iph->protocol == IPPROTO_ICMP) >> - icmp_out_count(net, ((struct icmphdr *) >> - skb_transport_header(skb))->type); >> + if (iph->protocol == IPPROTO_ICMP) { >> + u8 icmp_type; >> + >> + if (sk->sk_socket->type == SOCK_RAW && !inet_sk(sk)->hdrincl) > > What is the reason for not using sk->sk_type ? sk->sk_type is more concise. Thank you! > >> + icmp_type = fl4->fl4_icmp_type; >> + else >> + icmp_type = icmp_hdr(skb)->type; >> + icmp_out_count(net, icmp_type); >> + } > > >> >> ip_cork_release(cork); >> out: >> -- >> 2.25.1 >> > . >
diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c index 4e4e308c3230..57921b297a8e 100644 --- a/net/ipv4/ip_output.c +++ b/net/ipv4/ip_output.c @@ -1570,9 +1570,15 @@ struct sk_buff *__ip_make_skb(struct sock *sk, cork->dst = NULL; skb_dst_set(skb, &rt->dst); - if (iph->protocol == IPPROTO_ICMP) - icmp_out_count(net, ((struct icmphdr *) - skb_transport_header(skb))->type); + if (iph->protocol == IPPROTO_ICMP) { + u8 icmp_type; + + if (sk->sk_socket->type == SOCK_RAW && !inet_sk(sk)->hdrincl) + icmp_type = fl4->fl4_icmp_type; + else + icmp_type = icmp_hdr(skb)->type; + icmp_out_count(net, icmp_type); + } ip_cork_release(cork); out:
Like commit ea30388baebc ("ipv6: Fix an uninit variable access bug in __ip6_make_skb()"). icmphdr does not in skb linear region under the scenario of SOCK_RAW socket. Access icmp_hdr(skb)->type directly will trigger the uninit variable access bug. Use a local variable icmp_type to carry the correct value in different scenarios. Fixes: 96793b482540 ("[IPV4]: Add ICMPMsgStats MIB (RFC 4293)") Signed-off-by: Ziyang Xuan <william.xuanziyang@huawei.com> --- net/ipv4/ip_output.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)