netdev - Re: [PATCH net-next 5/5] udp: add recvmmsg implementation

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1165706e-b828-cb12-4bea-b77ccca1cb95@stressinduktion.org>
Date:   Fri, 25 Nov 2016 18:09:00 +0100
From:   Hannes Frederic Sowa <hannes@...essinduktion.org>
To:     Paolo Abeni <pabeni@...hat.com>, netdev@...r.kernel.org
Cc:     "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jesper Dangaard Brouer <brouer@...hat.com>,
        Sabrina Dubroca <sd@...asysnail.net>
Subject: Re: [PATCH net-next 5/5] udp: add recvmmsg implementation

On 25.11.2016 16:39, Paolo Abeni wrote:
> skbs are extracted from the receive queue in burts, and a single
> sk_rmem_alloc/forward allocated memory update is performed for
> each burst.
> MSG_PEEK and MSG_ERRQUEUE are not supported to keep the implementation
> as simple as possible.
> 
> Signed-off-by: Sabrina Dubroca <sd@...asysnail.net>
> Signed-off-by: Paolo Abeni <pabeni@...hat.com>

During review we discussed on how to handle major errors in the kernel:

The old code and the new code still can report back success even though
the kernel got back an EFAULT while copying from kernel space to user
space (due to bad pointers).

I favor that we drop all packets (also the already received batches) in
this case and let the code report -EFAULT and increase sk_drops for all
dropped packets from the queue.

Currently sk_err is set so the next syscall would get an -EFAULT, which
seems very bad and can also be overwritten by incoming icmp packets, so
we never get a notification that we actually had a bad pointer somewhere
in the mmsghdr. Also delivering -EFAULT on the follow-up syscalls really
will make people confused that use strace.

If people would like to know the amount of packets dropped we can make
sk_drops readable by an getsockopt.

Thoughts?

Unfortunately the interface doesn't allow for better error handling.