Re: [PATCH,v2] arm64: fix the illegal address access in some cases
From: Robin Murphy <robin.murphy@arm.com>
Date: 2020-07-28 14:32:14
Hi Will, On 2020-07-28 14:03, Will Deacon wrote:
On Sat, 25 Jul 2020 10:08:06 +0800, guodeqing wrote:quoted
The ihl value of ip header is smaller than 5 in some cases, if the ihl value is smaller than 5, then the next code will access the illegal address, and the system will panic. ip_fast_csum() must be able to handle any value that could fit in the ihl field of the ip protocol header. Here I add the check of the ihl value to solve this problem.Applied to arm64 (for-next/fixes), thanks! [1/1] arm64: csum: Reject IP headers with 'ihl' field smaller than five https://git.kernel.org/arm64/c/09aaef1c5f50
I'm not sure your commit message is entirely right there. AFAICS it's not "the same way as x86" at all - x86 dereferences the first word of iph and returns that as the sum if ihl <= 4 (and thus is still capable of crashing given sufficiently bogus data). I'm not sure where "return 1" came from - if we're going to return nonsense then the mildly more efficient choice of 0 seems just as good. Otherwise it would seem reasonable to jump straight into the word-at-a-time loop if ip_fast_csum() is really expected to cope with more than just genuine IP headers (which should be backed by at least 20 bytes of valid memory regardless of what ihl says). I still think this smells of papering over some other bug that led to a bogus skb getting that far into the transmit stack in the first place - presumably it's all wasted effort anyway since a "header" with no space for a destination address and a deliberately wrong checksum seems unlikely to go very far... On a quick look there appear to be quite a few implementations dereferencing up to 5 words unconditionally, so it's not like this is arm64's own bug. Robin. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel