netdev - Re: af_packet / TX_RING not fully non-blocking (w/ MSG

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:	Sat, 4 Apr 2015 10:36:55 +0200
From:	Mathias Kretschmer <mathias.kretschmer@...us.fraunhofer.de>
To:	Daniel Borkmann <daniel@...earbox.net>
CC:	<netdev@...r.kernel.org>, <willemb@...gle.com>
Subject: Re: af_packet / TX_RING not fully non-blocking (w/ MSG_DONTWAIT)

Hi Daniel,

you are right, EAGAIN seems more appropriate.

I thought packet_snd() would return this - it rather seems that I need 
glasses :)

Anyway, new patch attached. (Hopefully without spaces this time).

Cheers,

Mathias

On 04/04/2015 12:22 AM, Daniel Borkmann wrote:
> Hi Mathias,
>
> On 04/02/2015 12:52 PM, Mathias Kretschmer wrote:
>> Dear all,
>>
>> we have encountered a problem where the send(MSG_DONTWAIT) call on a 
>> TX_RING is not fully non-blocking in cases where the device's sndBuf 
>> is full (i.e. we are trying to write faster than the device can handle).
>>
>> This is on a WLAN radio (so it's not that hard to achieve :).
>>
>> Comparing the TX_RING send() handler to the regular send() handler, 
>> the difference seems to be in the sock_alloc_send_skb() call where, 
>> the regular handler passes a (flags & MSG_DONTWAIT), while the 
>> TX_RING handler always passes a 0 (block).
>>
>> The attached patch changes this behavior by
>>
>> a) also passing (flags & MSG_DONTWAIT)
>> b) adjusting the return code so that -ENOBUFS is returned if no frame 
>> could be sent or to return the number of bytes sent, if frame(s) 
>> could be sent within this call.
>>
>> The proposed modification works fine for us and has been tested 
>> extensively with WLAN and Ethernet device.
>>
>> Feel free to apply this patch if you agree with this solution.
>> Of course, we're also open to other solutions / proposals / ideas.
>
> Please send a proper patch with SOB, and no white space corruption
> (there are spaces instead of tabs).
>
> +        if (skb == NULL) {
> +                    /* we assume the socket was initially writeable 
> ... */
> +                        if (likely(len_sum > 0))
> +                            err = len_sum;
> +                    else
> +                            err = -ENOBUFS;
>              goto out_status;
>
> What I'm a bit worried about is, if existing applications would be
> able to handle -ENOBUFS? Any reason you don't let -EAGAIN from the
> sock_alloc_send_skb() not pass through?
>
> Well, man 2 sendmsg clearly describes the -EAGAIN possibility as
> "the socket is marked nonblocking and the requested operation would
> block". So far it was apparently not returned since here we'd just
> have blocked, but strictly speaking non-blocking applications would
> need to be aware and should handle -EAGAIN, that awareness might be
> more likely than -ENOBUFS, imho. What do you think?
>
> Cheers,
> Daniel


-- 
Dr. Mathias Kretschmer, Head of Competence Center
Fraunhofer FOKUS Network Research
A Schloss Birlinghoven, 53754 Sankt Augustin, Germany
T +49-2241-14-3466, F +49-2241-14-1050
E mathias.kretschmer@...us.fraunhofer.de


View attachment "005-af_packet_no_block_tx.patch" of type "text/x-patch" (793 bytes)