From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757413AbcFAGor (ORCPT ); Wed, 1 Jun 2016 02:44:47 -0400 Received: from szxga03-in.huawei.com ([119.145.14.66]:47132 "EHLO szxga03-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751197AbcFAGop (ORCPT ); Wed, 1 Jun 2016 02:44:45 -0400 Subject: Re: [PATCH] drivers: char: mem: Check {read,write}_kmem() addresses To: Robin Murphy , Russell King - ARM Linux References: <5e293c103f0a86f512d71f17091a55e9746e711f.1464698496.git.robin.murphy@arm.com> <20160531130821.GV19428@n2100.arm.linux.org.uk> <574D9448.7080206@arm.com> CC: , , , , "Guohanjun (Hanjun Guo)" From: Kefeng Wang Message-ID: <574E83CB.5040404@huawei.com> Date: Wed, 1 Jun 2016 14:42:19 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-Version: 1.0 In-Reply-To: <574D9448.7080206@arm.com> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit X-Originating-IP: [10.177.19.180] X-CFilter-Loop: Reflected X-Mirapoint-Virus-RAPID-Raw: score=unknown(0), refid=str=0001.0A090201.574E83F2.0016,ss=1,re=0.000,recu=0.000,reip=0.000,cl=1,cld=1,fgs=0, ip=0.0.0.0, so=2013-05-26 15:14:31, dmn=2013-03-21 17:37:32 X-Mirapoint-Loop-Id: be5baf7f4fa9d68ddc29574e6d513996 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2016/5/31 21:40, Robin Murphy wrote: > On 31/05/16 14:08, Russell King - ARM Linux wrote: >> On Tue, May 31, 2016 at 01:52:45PM +0100, Robin Murphy wrote: >>> Arriving at read_kmem() with an offset representing a bogus kernel >>> address (e.g. 0 from a simple "cat /dev/kmem") leads to copy_to_user >>> faulting on the kernel-space read. >>> >>> x86_64 happens to get away with this since the optimised implementation >>> uses "rep movs*", thus the user write (which is allowed to fault) and >>> the kernel read are the same instruction, the kernel-side fault falls >>> into the userspace fixup handler and a chain of events transpires >>> leading to returning the expected -EFAULT. On other architectures, >>> though, the read is not covered by the fixup entry for the write, and we >>> get a straightforward "Unable to hande kernel paging request..." dump. >>> >>> The more typical use-case of mmap_kmem() already validates the address >>> with pfn_valid() as one might expect, so let's make that consistent >>> across {read,write}_kem() too. >>> >>> Reported-by: Kefeng Wang >>> Signed-off-by: Robin Murphy >>> --- >>> >>> I'm not sure if this warrants going to stable or not, as it's really >>> just making an existing failure case more graceful and less confusing. >> >> Returning -EFAULT because the kernel-side address (iow, file offset) is >> invalid is not particularly nice: >> >> NAME >> read - read from a file descriptor >> >> ERRORS >> EFAULT buf is outside your accessible address space. >> >> Latest POSIX has: >> >> ENXIO >> A request was made of a nonexistent device, or the >> request was outside the capabilities of the device. >> >> which to me looks like a better error code to return, as file offsets >> which are not valid can be interpreted as being "outside the >> capabilities of the device". EFAULT has always on Linux meant that >> the user passed an invalid userspace buffer. > > Good point - seems I failed to twig that the error code in the x86 case is still effectively falling out of the "fault with the user address" path. ENXIO indeed sounds more reasonable, thanks. Hi Robin, thanks for you fix, return -EIO like mmap_kmem() is better to me. BRs, Kefeng > > Robin. > > . >