From: Robin Murphy <robin.murphy@arm.com>
To: Will Deacon <will@kernel.org>,
catalin.marinas@arm.com, guodeqing <geffrey.guo@huawei.com>
Cc: luke.starrett@broadcom.com, kernel-team@android.com,
linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH,v2] arm64: fix the illegal address access in some cases
Date: Tue, 28 Jul 2020 15:30:50 +0100 [thread overview]
Message-ID: <4b66d792-4c8a-a500-6f81-8e8f78e99b82@arm.com> (raw)
In-Reply-To: <159593323394.1061330.12501304112140193783.b4-ty@kernel.org>
Hi Will,
On 2020-07-28 14:03, Will Deacon wrote:
> On Sat, 25 Jul 2020 10:08:06 +0800, guodeqing wrote:
>> 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
next prev parent reply other threads:[~2020-07-28 14:32 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-25 2:08 [PATCH,v2] arm64: fix the illegal address access in some cases guodeqing
2020-07-27 11:47 ` Catalin Marinas
2020-07-27 13:29 ` 答复: " Guodeqing (A)
2020-07-28 9:53 ` Catalin Marinas
2020-07-28 13:03 ` Will Deacon
2020-07-28 14:30 ` Robin Murphy [this message]
2020-07-28 15:35 ` Will Deacon
2020-07-29 7:05 ` 答复: " Guodeqing (A)
2020-07-30 8:44 ` Will Deacon
2020-07-30 9:56 ` Robin Murphy
2020-07-30 16:03 ` Will Deacon
2020-07-31 3:04 ` 答复: " Guodeqing (A)
2020-07-30 10:49 ` Guodeqing (A)
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4b66d792-4c8a-a500-6f81-8e8f78e99b82@arm.com \
--to=robin.murphy@arm.com \
--cc=catalin.marinas@arm.com \
--cc=geffrey.guo@huawei.com \
--cc=kernel-team@android.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=luke.starrett@broadcom.com \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).