netdev - Re: RE packet: fix reserve calculation

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>] [day] [month] [year] [list]

Date:   Fri, 7 Sep 2018 22:59:54 -0400
From:   James Sakalaukus <james@...alaukus.com>
To:     Willem de Bruijn <willemb@...gle.com>
Cc:     David Miller <davem@...emloft.net>, netdev@...r.kernel.org
Subject: Re: RE packet: fix reserve calculation - net/packet/af_packet.c

On Fri, Sep 7, 2018 at 11:50 AM, Willem de Bruijn <willemb@...gle.com> wrote:
> Hi James,
>
> Thanks for the report. In the future please always include
> netdev@...r.kernel.org in technical discussions.
>
> On Fri, Sep 7, 2018 at 1:00 AM James Sakalaukus <james@...alaukus.com> wrote:
>>
>> Hello Willem and David,
>>
>> I have an unpolished Ethernet driver for a PCIe FPGA subsystem, and
>> the following commit has the side effect of moving the data alignment
>> for SOCK_RAW packets.
>>
>>
>> commit b84bbaf7a6c8cca24f8acf25a2c8e46913a947ba
>>     net/packet/af_packet.c
>>
>> These changes to packet_snd() moves the data and tail pointers
>> backwards by net_device->hard_header_len, which is nominally ETH_HLEN.
>> The .ndo_start_xmit driver function now gets a struct sk_buff with
>> data alignment on a 2-byte boundary.  My DMA core is not happy about
>> it.
>>
>>
>> commit 9aad13b087ab0a588cd68259de618f100053360e
>>
>> This commit changed the previous fix from a skb_push to skb_reserve.
>> The functionality from my end did not change though.  .ndo_start_xmit
>> still gets a struct sk_buff with 2 byte alignment.
>>
>>
>> This may be causing problems for other network drivers with DMA
>> alignment requirements, but maybe its just me.
>
> This is the crux of the question.
>
>
> Did the PACKET_TX_RING variant work with your device?
>
> A quick scan seems to indicate that it is common to allocate a linear
> buffer and then reserve hard_header_len aligned up to 16B
> (HH_MOD_LEN). The actual alignment of both link layer and network
> header then depends on the alignment with which kmalloc returned. It
> is probably safe to assume that the buddy allocator returns a multiple
> of 4B at least for allocations of this size. Then the network layer
> header is 4B aligned. But for Ethernet, the skb_push in eth_header()
> would make the link layer header 2B aligned.
>
> If you are not seeing these problems with other protocols, I must be
> misreading that code.
>
> I will take a closer look.
>

I actually have not tested the device with any protocols other than
SOCK_RAW.  This device is on a real time network that does not use
standard network layer protocols.
PACKET_TX_RING is something I haven't had time to delve into.

It may be the case that other protocols always hand over a 2-byte
aligned buffer.  All I know for sure is that I was getting a 4-byte
aligned buffer before the update.


>> I've only been working
>> on the driver for about 6 months, so its not released, though I would
>> like to put it out there for others.  I updated my distro kernel this
>> week, and then proceeded to beat my head against the wall trying to
>> figure out why my driver mysteriously stopped working.
>>
>>
>> Thanks for your time,
>>
>> James Sakalaukus
>> james@...alaukus.com