• Soheil Hassas Yeganeh's avatar
    sock: do not set sk_err in sock_dequeue_err_skb · f5f99309
    Soheil Hassas Yeganeh authored
    Do not set sk_err when dequeuing errors from the error queue.
    Doing so results in:
    a) Bugs: By overwriting existing sk_err values, it possibly
       hides legitimate errors. It is also incorrect when local
       errors are queued with ip_local_error. That happens in the
       context of a system call, which already returns the error
       code.
    b) Inconsistent behavior: When there are pending errors on
       the error queue, sk_err is sometimes 0 (e.g., for
       the first timestamp on the error queue) and sometimes
       set to an error code (after dequeuing the first
       timestamp).
    c) Suboptimality: Setting sk_err to ENOMSG on simple
       TX timestamps can abort parallel reads and writes.
    
    Removing this line doesn't break userspace. This is because
    userspace code cannot rely on sk_err for detecting whether
    there is something on the error queue. Except for ICMP messages
    received for UDP and RAW, sk_err is not set at enqueue time,
    and as a result sk_err can be 0 while there are plenty of
    errors on the error queue.
    
    For ICMP packets in UDP and RAW, sk_err is set when they are
    enqueued on the error queue, but that does not result in aborting
    reads and writes. For such cases, sk_err is only readable via
    getsockopt(SO_ERROR) which will reset the value of sk_err on
    its own. More importantly, prior to this patch,
    recvmsg(MSG_ERRQUEUE) has a race on setting sk_err (i.e.,
    sk_err is set by sock_dequeue_err_skb without atomic ops or
    locks) which can store 0 in sk_err even when we have ICMP
    messages pending. Removing this line from sock_dequeue_err_skb
    eliminates that race.
    Signed-off-by: default avatarSoheil Hassas Yeganeh <soheil@google.com>
    Signed-off-by: default avatarEric Dumazet <edumazet@google.com>
    Signed-off-by: default avatarWillem de Bruijn <willemb@google.com>
    Signed-off-by: default avatarNeal Cardwell <ncardwell@google.com>
    Acked-by: default avatarHannes Frederic Sowa <hannes@stressinduktion.org>
    Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
    f5f99309
skbuff.c 121 KB