netdev - Re: [PATCH net-next v3 2/2] usbnet: ipheth: add CDC NCM support

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: <9D068F77-4349-4285-BDE8-3DCFC45DB7E6@gmail.com>
Date: Wed, 31 May 2023 16:14:38 +0300
From: George Valkov <gvalkov@...il.com>
To: Paolo Abeni <pabeni@...hat.com>
Cc: Foster Snowhill <forst@....gy>,
 "David S. Miller" <davem@...emloft.net>,
 Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>,
 Simon Horman <simon.horman@...igine.com>,
 Jan Kiszka <jan.kiszka@...mens.com>,
 linux-usb <linux-usb@...r.kernel.org>,
 Linux Netdev List <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next v3 2/2] usbnet: ipheth: add CDC NCM support


Georgi Valkov
httpstorm.com
nano RTOS



> On 30 May 2023, at 1:58 PM, Paolo Abeni <pabeni@...hat.com> wrote:
> 
> Hi,
> 
> On Sat, 2023-05-27 at 15:03 +0200, Foster Snowhill wrote:
>> @@ -116,12 +124,12 @@ static int ipheth_alloc_urbs(struct ipheth_device *iphone)
>> if (rx_urb == NULL)
>> goto free_tx_urb;
>> 
>> - tx_buf = usb_alloc_coherent(iphone->udev, IPHETH_BUF_SIZE,
>> + tx_buf = usb_alloc_coherent(iphone->udev, IPHETH_TX_BUF_SIZE,
>>    GFP_KERNEL, &tx_urb->transfer_dma);
>> if (tx_buf == NULL)
>> goto free_rx_urb;
>> 
>> - rx_buf = usb_alloc_coherent(iphone->udev, IPHETH_BUF_SIZE + IPHETH_IP_ALIGN,
>> + rx_buf = usb_alloc_coherent(iphone->udev, IPHETH_RX_BUF_SIZE,
>>    GFP_KERNEL, &rx_urb->transfer_dma);
>> if (rx_buf == NULL)
>> goto free_tx_buf;
> 
> Here the driver already knows if the device is in NCM or legacy mode,
> so perhaps we could select the buffer size accordingly? You would
> probably need to store the actual buffer size somewhere to keep the
> buffer handling consistent and simple in later code.
> 
>> @@ -373,12 +489,10 @@ static netdev_tx_t ipheth_tx(struct sk_buff *skb, struct net_device *net)
>> }
>> 
>> memcpy(dev->tx_buf, skb->data, skb->len);
>> - if (skb->len < IPHETH_BUF_SIZE)
>> - memset(dev->tx_buf + skb->len, 0, IPHETH_BUF_SIZE - skb->len);
>> 
>> usb_fill_bulk_urb(dev->tx_urb, udev,
>>  usb_sndbulkpipe(udev, dev->bulk_out),
>> -  dev->tx_buf, IPHETH_BUF_SIZE,
>> +  dev->tx_buf, skb->len,
>>  ipheth_sndbulk_callback,
>>  dev);
>> dev->tx_urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
> 
> This chunk looks unrelated from NCM support, and unconditionally
> changes the established behaviour even with legacy mode, why?
> 
> Does that works even with old(er) devices?

Yes,
Tested-on: iPhone 7 Plus, iOS 15.7.6
Testen-on: iPhone 4s, iOS 8.4
Tested-on: iPhone 3G, iOS 4.2.1

All work without any issues.