linux-kernel - RE: [PATCH v9 5/6] usb:cdns3 Add Cadence USB3 DRD Driver

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: <BYAPR07MB4709E993145DE80AD24D2E36DDD30@BYAPR07MB4709.namprd07.prod.outlook.com>
Date:   Mon, 12 Aug 2019 04:43:16 +0000
From:   Pawel Laszczak <pawell@...ence.com>
To:     Peter Chen <peter.chen@....com>,
        Felipe Balbi <felipe.balbi@...ux.intel.com>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>
CC:     "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
        "hdegoede@...hat.com" <hdegoede@...hat.com>,
        "heikki.krogerus@...ux.intel.com" <heikki.krogerus@...ux.intel.com>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>,
        "rogerq@...com" <rogerq@...com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "jbergsagel@...com" <jbergsagel@...com>,
        "nsekhar@...com" <nsekhar@...com>, "nm@...com" <nm@...com>,
        Suresh Punnoose <sureshp@...ence.com>,
        Jayshri Dajiram Pawar <jpawar@...ence.com>,
        Rahul Kumar <kurahul@...ence.com>
Subject: RE: [PATCH v9 5/6] usb:cdns3 Add Cadence USB3 DRD Driver


>
>>
>> Yes, driver frees not used buffers here.
>> I think that it's the safest place for this purpose.
>>
>> >
>> >>>> +				dma_free_coherent(priv_dev->sysdev, buf-
>> >size,
>> >>>> +						  buf->buf,
>> >>>> +						  buf->dma);
>> >>>> +				spin_lock_irqsave(&priv_dev->lock, flags);
>> >>>> +
>> >>>> +				kfree(buf);
>> >>>
>> >>>why do you even need this "garbage collector"?
>> >>
>> >> I need to free not used memory. The once allocated buffer will be
>> >> associated with request, but if request.length will be increased in
>> >> usb_request then driver will must allocate the  bigger buffer. As I
>> >> remember I couldn't call dma_free_coherent in interrupt context so I
>> >> had to move it to thread handled. This flag was used to avoid going through
>> whole  aligned_buf_list  every time.
>> >> In most cases this part will never called int this place
>> >
>> >Did you try, btw, setting the quirk flag which tells gadget drivers to
>> >always allocate buffers aligned to MaxPacketSize? Wouldn't that be enough?
>>
>> If found only  quirk_ep_out_aligned_size flag, but it align only buffer size.
>>
>> DMA used by this controller must have buffer address aligned to 8.
>> I think that on most architecture kmalloc should guarantee such aligned.
>> The problem was detected on NXP testing board.
>> On my board all buffer address are alignment at least to 8.
>>
>
>This un-aligned request buffer address for 8 occurs for Ethernet Gadget (eg, NCM),
>it allocates socket buffer with NET_IP_ALIGN, so the last byte of buffer address
>is always 2. Although this can be workaround by setting quirk_avoids_skb_reserve,
>but we are not sure if all gadget request buffers can be 8 or Max Packet Size aligned.
>

Thanks Peter for explanation. 
I will add quirk_avoids_skb_reserve to avoid using this extra buffers, but I leave this 
code for safety. 

--
Cheers,
Pawell