netdev - Re: [RFC PATCH] packet: mark ring entry as in-use inside spin

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]

Date:   Wed, 4 Apr 2018 23:44:49 +0200
From:   Willem de Bruijn <willemdebruijn.kernel@...il.com>
To:     "Jon Rosen (jrosen)" <jrosen@...co.com>
Cc:     "David S. Miller" <davem@...emloft.net>,
        Willem de Bruijn <willemb@...gle.com>,
        Eric Dumazet <edumazet@...gle.com>,
        Kees Cook <keescook@...omium.org>,
        David Windsor <dwindsor@...il.com>,
        "Rosen, Rami" <rami.rosen@...el.com>,
        "Reshetova, Elena" <elena.reshetova@...el.com>,
        Mike Maloney <maloney@...gle.com>,
        Benjamin Poirier <bpoirier@...e.com>,
        "open list:NETWORKING [GENERAL]" <netdev@...r.kernel.org>,
        open list <linux-kernel@...r.kernel.org>
Subject: Re: [RFC PATCH] packet: mark ring entry as in-use inside spin_lock to
 prevent RX ring overrun

>> >    One issue with the above proposed change to use TP_STATUS_IN_PROGRESS
>> >    is that the documentation of the tp_status field is somewhat
>> >    inconsistent.  In some places it's described as TP_STATUS_KERNEL(0)
>> >    meaning the entry is owned by the kernel and !TP_STATUS_KERNEL(0)
>> >    meaning the entry is owned by user space.  In other places ownership
>> >    by user space is defined by the TP_STATUS_USER(1) bit being set.
>>
>> But indeed this example in packet_mmap.txt is problematic
>>
>>     if (status == TP_STATUS_KERNEL)
>>         retval = poll(&pfd, 1, timeout);
>>
>> It does not really matter whether the docs are possibly inconsistent and
>> which one is authoritative. Examples like the above make it likely that
>> some user code expects such code to work.
>
> Yes, that's exactly my concern.  Yet another troubling example seems to be
> lipbcap which also is looking specifically for status to be anything other than
> TP_STATUS_KERNEL(0) to indicate a frame is available in user space.

Good catch. If pcap-linux.c relies on this then the status field
cannot be changed. Other fields can be modified freely while tp_status
remains 0, perhaps that's an option.