netdev - Re: traceroute failure in kernel 6.1 and 6.2

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: <71a0050a-2656-1d5b-0302-65000cf854cb@gmail.com>
Date:   Tue, 24 Jan 2023 15:27:15 +0000
From:   Pavel Begunkov <asml.silence@...il.com>
To:     Eric Dumazet <edumazet@...gle.com>,
        Mantas Mikulėnas <grawity@...il.com>
Cc:     netdev@...r.kernel.org
Subject: Re: traceroute failure in kernel 6.1 and 6.2

On 1/24/23 08:57, Eric Dumazet wrote:
> On Tue, Jan 24, 2023 at 7:03 AM Eric Dumazet <edumazet@...gle.com> wrote:
[...]
>>> It doesn't solve the issue; I tried bumping it from the default of
>>> 212992 to 4096-times-that, with exactly the same results.
>>>
>>> The amount of packets it's able to send is variable, For example, right
>>> now, on my regular VM (which is smaller than the PC that yesterday's
>>> trace was done on), the program very consistently fails on the *second*
>>> sendto() call -- I don't think two packets is an unreasonable amount.
>>>
>>> The program has -q and -N options to reduce the number of simultaneous
>>> probes, but the only effect it has is if I reduce the packets all the
>>> way down to just one at a time.
>>
>> Problem is : if we revert the patch, unpriv users can trivially crash a host.
>>
>> Also, sent ICMP packets  look just fine to me, and the patch is
>> changing tx path.
>>
>> The reported issue seems more like rx path related to me.
>> Like IP_RECVERR being not handled correctly.
>>
>> I think more investigations are needed. Maybe contact Pavel Begunkov
>> <asml.silence@...il.com>
>> because the initial crash issue came with
>> 47cf88993c91 ("net: unify alloclen calculation for paged requests")
> 
> I am reasonably confident this is a bug in this traceroute binary.
> 
> It sets
>   setsockopt(3, SOL_IP, IP_RECVERR, [1], 4) = 0
> 
> So a sendto() can absolutely return the error set by last received
> ICMP (cf ping_err()) on the socket,
> as per RFC1122 4.1.3.3
> 
>   4.1.3.3  ICMP Messages
> 
>              UDP MUST pass to the application layer all ICMP error
>              messages that it receives from the IP layer.  Conceptually
>              at least, this may be accomplished with an upcall to the
>              ERROR_REPORT routine (see Section 4.2.4.1).
> 
>              DISCUSSION:
>                   Note that ICMP error messages resulting from sending a
>                   UDP datagram are received asynchronously.  A UDP-based
>                   application that wants to receive ICMP error messages
>                   is responsible for maintaining the state necessary to
>                   demultiplex these messages when they arrive; for
>                   example, the application may keep a pending receive
>                   operation for this purpose.  The application is also
>                   responsible to avoid confusion from a delayed ICMP
>                   error message resulting from an earlier use of the same
> 
> 
> Fix would be
> 
> diff traceroute/traceroute.c.orig traceroute/traceroute.c
> 1657c1657
> <     if (errno == EMSGSIZE)
> ---
>>      if (errno == EMSGSIZE || errno == EHOSTUNREACH)
> 
> or to collect a pending socket error (but that would be racy), using
> SO_ERROR getsockopt()

If it doesn't help I'll take a look, perfectly reproducible for me.

-- 
Pavel Begunkov