From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S968260AbcHBRzc (ORCPT ); Tue, 2 Aug 2016 13:55:32 -0400 Received: from shards.monkeyblade.net ([184.105.139.130]:50292 "EHLO shards.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S966923AbcHBRzO (ORCPT ); Tue, 2 Aug 2016 13:55:14 -0400 Date: Tue, 02 Aug 2016 10:47:52 -0700 (PDT) Message-Id: <20160802.104752.527337990768683986.davem@davemloft.net> To: mpatocka@redhat.com Cc: sparclinux@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 1/2] sparc: fix incorrect value returned by copy_from_user_fixup From: David Miller In-Reply-To: References: <20160801.213312.1130341044241125265.davem@davemloft.net> X-Mailer: Mew version 6.7 on Emacs 24.5 / Mule 6.0 (HANACHIRUSATO) Mime-Version: 1.0 Content-Type: Text/Plain; charset=us-ascii Content-Transfer-Encoding: 7bit X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.5.12 (shards.monkeyblade.net [149.20.54.216]); Tue, 02 Aug 2016 10:47:53 -0700 (PDT) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org From: Mikulas Patocka Date: Tue, 2 Aug 2016 08:20:15 -0400 (EDT) > On Mon, 1 Aug 2016, David Miller wrote: > >> From: Mikulas Patocka >> Date: Sun, 31 Jul 2016 19:50:57 -0400 (EDT) >> >> > @@ -18,9 +25,9 @@ >> > * of the cases, just fix things up simply here. >> > */ >> > >> > -static unsigned long compute_size(unsigned long start, unsigned long size, unsigned long *offset) >> > +static unsigned long compute_size(unsigned long start, unsigned long size, unsigned long *offset, unsigned long prefetch) >> > { >> > - unsigned long fault_addr = current_thread_info()->fault_address; >> > + unsigned long fault_addr = current_thread_info()->fault_address - prefetch; >> > unsigned long end = start + size; >> > >> > if (fault_addr < start || fault_addr >= end) { >> > @@ -36,7 +43,7 @@ unsigned long copy_from_user_fixup(void >> > { >> > unsigned long offset; >> > >> > - size = compute_size((unsigned long) from, size, &offset); >> > + size = compute_size((unsigned long) from, size, &offset, COPY_FROM_USER_PREFETCH); >> > if (likely(size)) >> > memset(to + offset, 0, size); >> > >> >> I think this might cause a problem. Assume we are not in one of those >> prefetching loops and are just doing a byte at a time, and therefore >> hit the fault exactly at the beginning of the missing page. >> >> You will rewind 0x100 bytes and the caller will restart the copy at >> "faulting address - 0x100". >> >> If someone is using atomic user copies, and using the returned length >> to determine which page in userspace needs to be faulted in, and >> then restart the copy, then we will loop forever. > > This isn't guaranteed on x86 neither. > > __copy_user_intel reads and writes 64 bytes in one loop iteration (and it > prefetches the data for the next iteration with "movl 64(%4), %%eax". If > it fails, it reports the amount of remaining data at the start of the loop > iteration. The reported value may be 67 bytes lower than the fault > location. That's very interesting, let me do some research into this, as I was pretty sure something like futexes or similar had some requirement in this area. Thanks.