netdev - Re: [PATCH] USBNET: fix handling padding packet

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: <be5ecff3-27e1-4ab7-b3af-9d7c4376140f@email.android.com>
Date:	Wed, 18 Sep 2013 20:56:15 +0200
From:	Bjørn Mork <bjorn@...k.no>
To:	Oliver Neukum <oneukum@...e.de>
CC:	Ming Lei <ming.lei@...onical.com>,
	"David S. Miller" <davem@...emloft.net>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Network Development <netdev@...r.kernel.org>,
	linux-usb <linux-usb@...r.kernel.org>
Subject: Re: [PATCH] USBNET: fix handling padding packet

Oliver Neukum <oneukum@...e.de> wrote:
>On Wed, 2013-09-18 at 17:52 +0200, Bjørn Mork wrote:
>
>> No modern device should need the padding.  No old device will be able
>to
>> use the SG feature as implemented. You only enable it on USB3, don't
>
>On XHCI.
>
>> you? If this feature is restricted to USB3 capable devices, then it
>most
>> certainly can be restricted to ZLP capable devices with absolutely no
>> difference in the resulting set of supported devices.
>
>No, USB 3.0 uses no companion controllers, so you can have devices
>of any speed connected to it.
>

Ah, right. I don't own such modern hardware, but I should have known this anyway. 

This still doesn't change the fact that the driver is brand new for brand new devices. I believe we should assume such devices will support ZLPs unless we have documentation stating anything else.

>> Anyway, if you want to keep the padding for SG then maybe this will
>work
>> and allow you to drop the extra struct usbnet field and allocation:
>> 
>>                         if (skb_tailroom(skb) && !dev->can_dma_sg) {
>>                                skb->data[skb->len] = 0;
>>                                __skb_put(skb, 1);
>>                         } else if (dev->can_dma_sg) {
>>                               sg_set_buf(&urb->sg[urb->num_sgs++],
>skb->data, 1);
>>                         }
>> 
>> I.e. cheat and use the skb->data buffer twice, if that is allowed? 
>The
>> actual value of the padding byte should not matter, I believe?
>
>That makes me immediately suspect a violation of the DMA rules.

Sounds likely. And it's an ugly hack in any case. Probably not a good idea. Just one of the many random thoughts I should have kept to myself :-)


Bjørn 


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html