netdev - Re: [PATCH] can: ems_usb: Fix possible tx overflow

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: <56CAFC9E.7050306@pengutronix.de>
Date:	Mon, 22 Feb 2016 13:18:38 +0100
From:	Marc Kleine-Budde <mkl@...gutronix.de>
To:	Sergei Shtylyov <sergei.shtylyov@...entembedded.com>,
	netdev@...r.kernel.org
Cc:	davem@...emloft.net, linux-can@...r.kernel.org,
	kernel@...gutronix.de,
	Gerhard Uttenthaler <uttenthaler@...-wuensche.com>,
	linux-stable <stable@...r.kernel.org>
Subject: Re: [PATCH] can: ems_usb: Fix possible tx overflow

On 02/22/2016 01:11 PM, Sergei Shtylyov wrote:
> Hello.
> 
> On 2/21/2016 6:27 PM, Marc Kleine-Budde wrote:
> 
>> From: Gerhard Uttenthaler <uttenthaler@...-wuensche.com>
>>
>> This patch fixes the problem that more CAN messages could be sent to the
>> interface as could be send on the CAN bus. This was more likely for slow baud
>> rates. The sleeping _start_xmit was woken up in the _write_bulk_callback. Under
>> heavy TX load this produced another bulk transfer without checking the
>> free_slots variable and hence caused the overflow in the interface.
>>
>> Signed-off-by: Gerhard Uttenthaler <uttenthaler@...-wuensche.com>
>> Cc: linux-stable <stable@...r.kernel.org>
>> Signed-off-by: Marc Kleine-Budde <mkl@...gutronix.de>
>> ---
>>   drivers/net/can/usb/ems_usb.c | 14 ++++++++++----
>>   1 file changed, 10 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/net/can/usb/ems_usb.c b/drivers/net/can/usb/ems_usb.c
>> index fc5b75675cd8..eb7192fab593 100644
>> --- a/drivers/net/can/usb/ems_usb.c
>> +++ b/drivers/net/can/usb/ems_usb.c
>> @@ -117,6 +117,9 @@ MODULE_LICENSE("GPL v2");
>>    */
>>   #define EMS_USB_ARM7_CLOCK 8000000
>>
>> +#define CPC_TX_QUEUE_TRIGGER_LOW	25
>> +#define CPC_TX_QUEUE_TRIGGER_HIGH	35
>> +
>>   /*
>>    * CAN-Message representation in a CPC_MSG. Message object type is
>>    * CPC_MSG_TYPE_CAN_FRAME or CPC_MSG_TYPE_RTR_FRAME or
>> @@ -278,6 +281,11 @@ static void ems_usb_read_interrupt_callback(struct urb *urb)
>>   	switch (urb->status) {
>>   	case 0:
>>   		dev->free_slots = dev->intr_in_buffer[1];
>> +		if(dev->free_slots > CPC_TX_QUEUE_TRIGGER_HIGH){
>> +			if (netif_queue_stopped(netdev)){
>> +				netif_wake_queue(netdev);
>> +			}
>> +		}
> 
>     Hm, did anyone run scripts/checkpatch.pl on this patch?

Obviously not. :(

David, I can send a patch to clean up the coding style. Do you want it
for net or via net-next?

Marc
-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


Download attachment "signature.asc" of type "application/pgp-signature" (456 bytes)