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=ham 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 4E10FC433DF for ; Thu, 30 Jul 2020 08:46:16 +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 1B4C4206E6 for ; Thu, 30 Jul 2020 08:46:15 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="q9G4khkG"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=kernel.org header.i=@kernel.org header.b="c/Idy1oP" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 1B4C4206E6 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=QmfxW2OkMnPY8yYxI8rqlUMvieQSmIEdkrfitRTxf3E=; b=q9G4khkGsBanJv3ihd8Ja2Gli w0RIK7clwUmqpjlSHdkecQ2wHtUZ6S8kaLiMBM3eHRa4rSRtyL2oC6lgECkMMl46UnN1cLXPfCS+S xh1hG8QVmlEUnaSFiNZtS+TaBHUOLD2vUL649Ftl1YQb0wWQoWjmIUggVMH6jrotHMf7LaElXLmt6 3Bj21xM9aWqa9JTWLDNU/g6k7PmsDw82fZkVw2oTbZqwWIl9sQ+wfIkeC45BRByYVLXATJChhns39 fkAv7fgkQDquc6HDowVokE/+1EkyqIXjx3STstjgapTLtPI18uZM2UoastL0p76nfuV8iaJk3gAnv WM6xErnTg==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1k14Aq-0007fl-VZ; Thu, 30 Jul 2020 08:44:25 +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 1k14An-0007fQ-6F for linux-arm-kernel@lists.infradead.org; Thu, 30 Jul 2020 08:44:22 +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 23C6C206E6; Thu, 30 Jul 2020 08:44:19 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1596098660; bh=mYYnI//stojJsDzYoTEPsJLvH33sRUW7DJr/GpLzES8=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=c/Idy1oPegcc2Ocm9owDRg8pE5LqUKjOZXo9kM79rWyYH++sN2FLarZxWJhW/NM0g mT06yIMTbNAZbT/COa5dnQXZoWk3Ig1sma3yYjB9hzKB3NJhePNE/pJTwDSx3/Ha+a Ahxqi2Gq5EW8/PGdlRvYmme8Id+1UbwQYENAeGHY= Date: Thu, 30 Jul 2020 09:44:16 +0100 From: Will Deacon To: "Guodeqing (A)" Subject: Re: =?utf-8?B?562U5aSNOiBbUEFUQ0gsdjI=?= =?utf-8?Q?=5D?= arm64: fix the illegal address access in some cases Message-ID: <20200730084415.GA24410@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> <20200728153528.GA22361@willie-the-truck> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: 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-20200730_044422_105944_FA008034 X-CRM114-Status: GOOD ( 36.36 ) 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" , Robin Murphy , "linux-arm-kernel@lists.infradead.org" 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 On Wed, Jul 29, 2020 at 07:05:09AM +0000, Guodeqing (A) wrote: > > On Tue, Jul 28, 2020 at 03:30:50PM +0100, Robin Murphy wrote: > > > On 2020-07-28 14:03, Will Deacon wrote: > > > > 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? > > > > The return 1 is just the report of ip checksum error, the return value 0 means the ip > checksum correct. x86 dereferences the first word of iph and returns that as the sum, > this may be just the report of ip checksum error too. On the receive path, sure, but the crash was on the transmit path where we're computing the checksum to insert into the header, no? > > > 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. > > How about modify the patch like this? > > static inline __sum16 ip_fast_csum(const void *iph, unsigned int ihl) > { > __uint128_t tmp; > u64 sum; > > if (unlikely(ihl < 5)) > ihl = 5; I'd probably do: /* Callers should really be checking this first */ if (WARN_ON_ONCE(ihl < 5)) ihl = 5; because I'd still like to understand what the vlan code is up to. > > > 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. > > The network maintainer has replied to me, > " ip_fast_csum() must be able to handle any value that could fit in the > ihl field of the ip protocol header. That's not only the most correct > logic, but also the most robust." Is that on a public list somewhere? Would be a good link for the commit message. > This is a fault injection test, the corrupt function of netem is the > emulation of random noise introducing an error in a random position for a > chosen percent of packets to test the network module.the netem will modify > the packet randomly,so the ihl value of ip header may be modified to 1. Ok, but netem is running in userspace (right?) and so I still think the network layer can reject the invalid ihl before calling into the checksum code. Will _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel