netdev - Re: [PATCH net] ipip: only increase err_count for some certain type icmp in ipip

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] [day] [month] [year] [list]

Message-Id: <20171027.234722.793819361242345327.davem@davemloft.net>
Date:   Fri, 27 Oct 2017 23:47:22 +0900 (KST)
From:   David Miller <davem@...emloft.net>
To:     lucien.xin@...il.com
Cc:     netdev@...r.kernel.org, hannes@...essinduktion.org,
        pshelar@...ira.com
Subject: Re: [PATCH net] ipip: only increase err_count for some certain
 type icmp in ipip_err

From: Xin Long <lucien.xin@...il.com>
Date: Thu, 26 Oct 2017 19:19:56 +0800

> t->err_count is used to count the link failure on tunnel and an err
> will be reported to user socket in tx path if t->err_count is not 0.
> udp socket could even return EHOSTUNREACH to users.
> 
> Since commit fd58156e456d ("IPIP: Use ip-tunneling code.") removed
> the 'switch check' for icmp type in ipip_err(), err_count would be
> increased by the icmp packet with ICMP_EXC_FRAGTIME code. an link
> failure would be reported out due to this.
> 
> In Jianlin's case, when receiving ICMP_EXC_FRAGTIME a icmp packet,
> udp netperf failed with the err:
>   send_data: data send error: No route to host (errno 113)
> 
> We expect this error reported from tunnel to socket when receiving
> some certain type icmp, but not ICMP_EXC_FRAGTIME, ICMP_SR_FAILED
> or ICMP_PARAMETERPROB ones.
> 
> This patch is to bring 'switch check' for icmp type back to ipip_err
> so that it only reports link failure for the right type icmp, just as
> in ipgre_err() and ipip6_err().
> 
> Fixes: fd58156e456d ("IPIP: Use ip-tunneling code.")
> Reported-by: Jianlin Shi <jishi@...hat.com>
> Signed-off-by: Xin Long <lucien.xin@...il.com>

Good catch, I have no idea why that logic was removed.  Maybe the
author didn't see the err_count side effect and it's implications.

Applied and queued up for -stable.