From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-10.5 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,MAILING_LIST_MULTI,MENTIONS_GIT_HOSTING,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 879F6C433E5 for ; Tue, 28 Jul 2020 15:37:11 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 58533206D8 for ; Tue, 28 Jul 2020 15:37:11 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="QcitlZjw"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=kernel.org header.i=@kernel.org header.b="XZhWZm4T" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 58533206D8 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:Message-ID: Subject:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=a0KpgtdAFfRHdKT2VqHMy0VJ3mUfE1zpHzlXevPYRME=; b=QcitlZjwEa5S+cLh+v8vknY3a PYiDEKyHCWy+/a0xDDH0eKB9eazQ501S5t3xGZatdtPrqqSbuw7sfbQhnsfvFO9ByjZ5j4nV88aTl DZdc2lCLKeG87OtuTwdpIwcqKB8quAHRlzjXD4oYI2CneBhOX8VU/RD3O851Z5/pXWv3/0DLN5sXu RgZbBI0JtX3d8vwpMyBZVxZZs942ucSOMxowdPQOofZSezsLs9LbvfmgAxlA458VOZ5JZpKuzotKj 6bwYiF5OlTZOOlIy3qS3wpm/ik25dAUR3Apa666/YquAU2F5fRO47H2L/RsVva/4RkkqpcRFVdk32 VPU75cJCw==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1k0Rdr-00037E-U8; Tue, 28 Jul 2020 15:35:47 +0000 Received: from mail.kernel.org ([198.145.29.99]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1k0Rdd-00032t-U9 for linux-arm-kernel@lists.infradead.org; Tue, 28 Jul 2020 15:35:36 +0000 Received: from willie-the-truck (236.31.169.217.in-addr.arpa [217.169.31.236]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 017D5206D8; Tue, 28 Jul 2020 15:35:31 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1595950533; bh=uV0pM7wGUMP9Qv9q4fwl/U3ueICuDigmkumUnpSkQpY=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=XZhWZm4T8C7H8OPny9xJnEb3l4+An0Kh+D+gSTwV9eNvZHMGFkXZ4aHdKd5QeU8t8 qgyQ3wWIZ+MMfiWwX4Wc7je79joGFIaU7wS2G9yw8Mb3FsR90cvb483KxRaRP9MLX5 rEz9X6mpRfEbljXQvbnHD9etIayr3amUb2HPJLp4= Date: Tue, 28 Jul 2020 16:35:28 +0100 From: Will Deacon To: Robin Murphy Subject: Re: [PATCH,v2] arm64: fix the illegal address access in some cases Message-ID: <20200728153528.GA22361@willie-the-truck> References: <1595642886-78334-1-git-send-email-geffrey.guo@huawei.com> <159593323394.1061330.12501304112140193783.b4-ty@kernel.org> <4b66d792-4c8a-a500-6f81-8e8f78e99b82@arm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <4b66d792-4c8a-a500-6f81-8e8f78e99b82@arm.com> User-Agent: Mutt/1.10.1 (2018-07-13) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200728_113534_264850_8C73EE7B X-CRM114-Status: GOOD ( 29.94 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: catalin.marinas@arm.com, luke.starrett@broadcom.com, kernel-team@android.com, linux-arm-kernel@lists.infradead.org, guodeqing Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Robin, On Tue, Jul 28, 2020 at 03:30:50PM +0100, Robin Murphy wrote: > 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. Argh, yes, that's %1 not $1, so I don't know where the 1 comes from either. Geffrey? > 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). Either copying the x86 behaviour or WARN_ON_ONCE() and assuming and ihl of 5 would be my preference, because I agree with you that this feels like it shouldn't be happening to start with. > 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... Looking at the ipvlan_start_xmit() path from the backtrace, it looks to me like ipvlan_get_L3_hdr() returns NULL if the header length is invalid, but then ipvlan_xmit_mode_l3() ends up calling ipvlan_process_outbound() anyway. Hmm. I really don't know enough about VLANs to know what the right behaviour is here and I guess just returning NET_XMIT_DROP will break something. > 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. I'll drop the patch, but we are apparently open to a crash here, so if you have time to figure out what's going on, that would be great. The reproducer didn't work for me (I guess I'm missing some utils) and sending bogus header lengths with a raw socket worked fine (i.e. didn't crash either). I guess the vlan is an important piece of the picture. Will _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel