linux-kernel - Re: commit f5f99309 (sock: do not set sk_err in sock_dequeue_err

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]

Date:   Thu, 1 Jun 2017 12:42:18 -0400
From:   Soheil Hassas Yeganeh <soheil@...gle.com>
To:     Cyril Hrubis <chrubis@...e.cz>
Cc:     "David S. Miller" <davem@...emloft.net>,
        Shmulik Ladkani <shmulik.ladkani@...il.com>,
        Marcelo Ricardo Leitner <marcelo.leitner@...il.com>,
        Pravin B Shelar <pshelar@....org>,
        Eric Dumazet <edumazet@...gle.com>,
        WANG Cong <xiyou.wangcong@...il.com>,
        Yaogong Wang <wygivan@...gle.com>,
        Steffen Klassert <steffen.klassert@...unet.com>,
        Al Viro <viro@...iv.linux.org.uk>,
        netdev <netdev@...r.kernel.org>, linux-kernel@...r.kernel.org
Subject: Re: commit f5f99309 (sock: do not set sk_err in sock_dequeue_err_skb)
 has broken ping

On Thu, Jun 1, 2017 at 11:36 AM, Cyril Hrubis <chrubis@...e.cz> wrote:
> It seems to repeatedly produce (until I plug the cable back):
>
> ee_errno = 113 ee_origin = 2 ee_type = 3 ee_code = 1 ee_info = 0 ee_data = 0
>
> So we get EHOSTUNREACH on SO_EE_ORIGIN_ICMP.

Thank you very much! I have a wild guess that, when we
have a train of skbs on the error queue starting from a local error,
we will see this issue.

Ping (without my patch) considers EAGAIN on a normal read as an
indication that there is nothing on the error queue, but that's a
flawed assumption.

Would you mind trying another shot in the darkness please? Thanks!

diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 5a726161f4e4..097152a03c74 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -3742,7 +3742,8 @@ EXPORT_SYMBOL(sock_queue_err_skb);
 static bool is_icmp_err_skb(const struct sk_buff *skb)
 {
        return skb && (SKB_EXT_ERR(skb)->ee.ee_origin == SO_EE_ORIGIN_ICMP ||
-                      SKB_EXT_ERR(skb)->ee.ee_origin == SO_EE_ORIGIN_ICMP6);
+                      SKB_EXT_ERR(skb)->ee.ee_origin == SO_EE_ORIGIN_ICMP6 ||
+                      SKB_EXT_ERR(skb)->ee.ee_origin == SO_EE_ORIGIN_LOCAL);
 }

 struct sk_buff *sock_dequeue_err_skb(struct sock *sk)
@@ -3751,14 +3752,19 @@ struct sk_buff *sock_dequeue_err_skb(struct sock *sk)
        struct sk_buff *skb, *skb_next = NULL;
        bool icmp_next = false;
        unsigned long flags;
+       int err = 0;

        spin_lock_irqsave(&q->lock, flags);
        skb = __skb_dequeue(q);
-       if (skb && (skb_next = skb_peek(q)))
+       if (skb && (skb_next = skb_peek(q))) {
                icmp_next = is_icmp_err_skb(skb_next);
+               err = SKB_EXT_ERR(skb_next)->ee.ee_origin;
+       }
        spin_unlock_irqrestore(&q->lock, flags);

-       if (is_icmp_err_skb(skb) && !icmp_next)
+       if (icmp_next)
+               sk->sk_err = err;
+       else if (is_icmp_err_skb(skb) && !icmp_next)
                sk->sk_err = 0;

        if (skb_next)