netdev - Re: [PATCH RFC net-next 1/6] sock: MSG_PEEK support for sk_error

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: <CAF=yD-KkL+rnSOvYMFKmeaJoD1zLj1mxtroccTNUJGrhm4SyKA@mail.gmail.com>
Date:   Wed, 17 Jan 2018 18:50:56 -0500
From:   Willem de Bruijn <willemdebruijn.kernel@...il.com>
To:     Sowmini Varadhan <sowmini.varadhan@...cle.com>
Cc:     Network Development <netdev@...r.kernel.org>,
        David Miller <davem@...emloft.net>, rds-devel@....oracle.com,
        santosh.shilimkar@...cle.com
Subject: Re: [PATCH RFC net-next 1/6] sock: MSG_PEEK support for sk_error_queue

On Wed, Jan 17, 2018 at 7:19 AM, Sowmini Varadhan
<sowmini.varadhan@...cle.com> wrote:
> Allow the application the ability to use MSG_PEEK with sk_error_queue
> so that it can peek and re-read message in cases where MSG_TRUNC
> may be encountered.
>
> Signed-off-by: Sowmini Varadhan <sowmini.varadhan@...cle.com>

>  int sock_recv_errqueue(struct sock *sk, struct msghdr *msg, int len,
> -                      int level, int type)
> +                      int level, int type, int flags)
>  {
>         struct sock_exterr_skb *serr;
>         struct sk_buff *skb;
> @@ -2916,7 +2916,10 @@ int sock_recv_errqueue(struct sock *sk, struct msghdr *msg, int len,
>         err = copied;
>
>  out_free_skb:
> -       kfree_skb(skb);
> +       if (likely(!(flags & MSG_PEEK)))
> +               kfree_skb(skb);
> +       else
> +               skb_queue_head(&sk->sk_error_queue, skb);

This can cause reordering with parallel readers. Can we avoid the need
for peeking? It also caused a slew of subtle bugs previously.

How about just define a max number of cookies and require the caller
to always read with sufficient room to hold them?