linux-kernel - Re: [PATCH] asix: Fix offset calculation in asix_rx

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]

Date:	Tue, 17 May 2016 14:03:11 -0400 (EDT)
From:	David Miller <davem@...emloft.net>
To:	john.stultz@...aro.org
Cc:	linux-kernel@...r.kernel.org, Dean_Jenkins@...tor.com,
	linux@...idrobins.net, Mark_Craske@...tor.com, emilgoode@...il.com,
	yongqin.liu@...aro.org, guodong.xu@...aro.org, ivecera@...hat.com,
	linux-usb@...r.kernel.org, netdev@...r.kernel.org,
	stable@...r.kernel.org
Subject: Re: [PATCH] asix: Fix offset calculation in asix_rx_fixup()
 causing slow transmissions

From: John Stultz <john.stultz@...aro.org>
Date: Mon, 16 May 2016 20:36:15 -0700

> In testing with HiKey, we found that since
> commit 3f30b158eba5 ("asix: On RX avoid creating bad Ethernet
> frames"),
> we're seeing lots of noise during network transfers:
 ...
> And network throughput ends up being pretty bursty and slow with
> a overall throughput of at best ~30kB/s (where as previously we
> got 1.1MB/s with the slower USB1.1 "full speed" host).
> 
> We found the issue also was reproducible on a x86_64 system,
> using a "high-speed" USB2.0 port but the throughput did not
> measurably drop (possibly due to the scp transfer being cpu
> bound on my slow test hardware).
> 
> After lots of debugging, I found the check added in the
> problematic commit seems to be calculating the offset
> incorrectly.
> 
> In the normal case, in the main loop of the function, we do:
> (where offset is zero, or set to "offset += (copy_length + 1) &
> 0xfffe" in the previous loop)
>     rx->header = get_unaligned_le32(skb->data +
>                                     offset);
>     offset += sizeof(u32);
> 
> But the problematic patch calculates:
>     offset = ((rx->remaining + 1) & 0xfffe) + sizeof(u32);
>     rx->header = get_unaligned_le32(skb->data + offset);
> 
> Adding some debug logic to check those offset calculation used
> to find rx->header, the one in problematic code is always too
> large by sizeof(u32).
> 
> Thus, this patch removes the incorrect " + sizeof(u32)" addition
> in the problematic calculation, and resolves the issue.
...
> Reported-by: Yongqin Liu <yongqin.liu@...aro.org>
> Signed-off-by: John Stultz <john.stultz@...aro.org>

Applied.