From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754709AbdECSqW (ORCPT ); Wed, 3 May 2017 14:46:22 -0400 Received: from mx1.redhat.com ([209.132.183.28]:59688 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752967AbdECSqN (ORCPT ); Wed, 3 May 2017 14:46:13 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 83005201FB Authentication-Results: ext-mx05.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx05.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=riel@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 83005201FB Message-ID: <1493837167.20270.8.camel@redhat.com> Subject: Re: [RESENT PATCH] x86/mem: fix the offset overflow when read/write mem From: Rik van Riel To: David Rientjes , zhongjiang , Bjorn Helgaas , Yoshinori Sato Cc: Rich Felker , Andrew Morton , arnd@arndb.de, hannes@cmpxchg.org, kirill@shutemov.name, mgorman@techsingularity.net, hughd@google.com, linux-mm@kvack.org, linux-kernel@vger.kernel.org Date: Wed, 03 May 2017 14:46:07 -0400 In-Reply-To: References: <1493293775-57176-1-git-send-email-zhongjiang@huawei.com> Content-Type: multipart/signed; micalg="pgp-sha256"; protocol="application/pgp-signature"; boundary="=-hoXnknDxDS92K+dTIJiC" Mime-Version: 1.0 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.29]); Wed, 03 May 2017 18:46:12 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --=-hoXnknDxDS92K+dTIJiC Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Tue, 2017-05-02 at 13:54 -0700, David Rientjes wrote: > > diff --git a/drivers/char/mem.c b/drivers/char/mem.c > > index 7e4a9d1..3a765e02 100644 > > --- a/drivers/char/mem.c > > +++ b/drivers/char/mem.c > > @@ -55,7 +55,7 @@ static inline int > valid_phys_addr_range(phys_addr_t addr, size_t count) > >=C2=A0=C2=A0 > >=C2=A0 static inline int valid_mmap_phys_addr_range(unsigned long pfn, > size_t size) > >=C2=A0 { > > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0return 1; > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0return (pfn << PAGE_SHIFT) + size <=3D _= _pa(high_memory); > >=C2=A0 } > >=C2=A0 #endif > >=C2=A0=C2=A0 >=20 > I suppose you are correct that there should be some sanity checking > on the=C2=A0 > size used for the mmap(). My apologies for not responding earlier. It may indeed make sense to have a sanity check here. However, it is not as easy as simply checking the end against __pa(high_memory). Some systems have non-contiguous physical memory ranges, with gaps of invalid addresses in-between. You would have to make sure that both the beginning and the end are valid, and that there are no gaps of invalid pfns in the middle... At that point, is the complexity so much that it no longer makes sense to try to protect against root crashing the system? --=20 All rights reversed --=-hoXnknDxDS92K+dTIJiC Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part Content-Transfer-Encoding: 7bit -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQEcBAABCAAGBQJZCiVvAAoJEM553pKExN6DYH8H/0WhU6+eB1BnSdM1Lz8d0VLc NV1qp0758ix0rrG+EbIBfR8QjpcCaIyjtKv2tu/dnvsk2ugP724HAVjDR1AVvFyA 3nw1O2rXnPxbWMkQn40fEqOIFOmguLGbExzCay28lH4gDIUjmFI1ArxlcMBtHSch gnWAke3kDSXmat9jswj493a0WG8w1lJwKdBIe3eqYwRL17ErhiDAqD8YBRSZiZAH 3FqAobI3mlopSFM8rMohRB6MTFy0T1g2vZgzj1SFLdOFaOsYrcfy2QEUm5hgj7oE PFDuzr/06Yv8jzu/yuCZNoz+BSZ3YHXjcbxfameuY7OEIFrlonl05oAUDvKKa6g= =2Cy0 -----END PGP SIGNATURE----- --=-hoXnknDxDS92K+dTIJiC--