lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAL=9YSUt2j6tgP2OXntHn-_TcFz4i_=WQ9fo86sEZBLW9afbvw@mail.gmail.com>
Date:	Mon, 12 Oct 2015 00:14:46 +0100
From:	Ben Cox <ben@...jojo.co.uk>
To:	Eric Dumazet <eric.dumazet@...il.com>
Cc:	Ben Cox <ben@...jojo.co.uk>, davem@...emloft.net,
	netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
	ben+patches@...jojo.co.uk, David L Stevens <dlstevens@...ibm.com>
Subject: Re: [PATCH] icmp: Fixed bug in raw sockets causing incorrect ICMP
 SNMP counter values

Nice!

That works in my head at least, Sorry about not seeing that fairly
glaring memory issue there.

Are you sure " skb->transport_header += iphlen; " won't have a knock
on affect when it's given to NF_HOOK ( as in, would a potential
userspace program get something it does not expect anymore ) ?

How does submission work at this point if the above is not a issue
(apologies if this is already in a FAQ somewhere I missed)?

On Mon, Oct 12, 2015 at 12:09 AM, Eric Dumazet <eric.dumazet@...il.com> wrote:
> On Sun, 2015-10-11 at 15:44 -0700, Eric Dumazet wrote:
>> On Sun, 2015-10-11 at 15:43 -0700, Eric Dumazet wrote:
>>
>> > But your code reads 21th byte.
>>
>> BTW, nice catch !
>
> Maybe the following one.
>
> 1) We properly set transport header
> 2) We use icmp_hdr() helper.
>
> diff --git a/net/ipv4/raw.c b/net/ipv4/raw.c
> index 561cd4b8fc6e..ffe25cd1f0e0 100644
> --- a/net/ipv4/raw.c
> +++ b/net/ipv4/raw.c
> @@ -406,11 +406,11 @@ static int raw_send_hdrinc(struct sock *sk, struct flowi4 *fl4,
>                         ip_select_ident(net, skb, NULL);
>
>                 iph->check = ip_fast_csum((unsigned char *)iph, iph->ihl);
> +               skb->transport_header += iphlen;
> +               if (iph->protocol == IPPROTO_ICMP &&
> +                   length >= iphlen + sizeof(struct icmphdr))
> +                       icmp_out_count(net, icmp_hdr(skb)->type);
>         }
> -       if (iph->protocol == IPPROTO_ICMP)
> -               icmp_out_count(net, ((struct icmphdr *)
> -                       skb_transport_header(skb))->type);
> -
>         err = NF_HOOK(NFPROTO_IPV4, NF_INET_LOCAL_OUT, sk, skb,
>                       NULL, rt->dst.dev, dst_output_sk);
>         if (err > 0)
>
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ