[PATCH] Revert "sparc64: Fix __copy_{to,from}_user_inatomic defines."

[PATCH] Revert "sparc64: Fix __copy_{to,from}_user_inatomic defines."

[Dave Kleikamp]

This reverts commit 145e1c0023585e0e8f6df22316308ec61c5066b2.

This commit broke the behavior of __copy_from_user_inatomic when
it is only partially successful. Instead of returning the number
of bytes not copied, it now returns 1. This translates to the
wrong value being returned by iov_iter_copy_from_user_atomic.

xfstests generic/246 and LTP writev01 both fail on btrfs and nfs
because of this.

Signed-off-by: Dave Kleikamp <dave.kleikamp@oracle.com>
Cc: Hugh Dickins <hughd@google.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: sparclinux@vger.kernel.org
---
 arch/sparc/include/asm/uaccess_64.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/sparc/include/asm/uaccess_64.h b/arch/sparc/include/asm/uaccess_64.h
index e562d3c..ad7e178 100644
--- a/arch/sparc/include/asm/uaccess_64.h
+++ b/arch/sparc/include/asm/uaccess_64.h
@@ -262,8 +262,8 @@ extern unsigned long __must_check __clear_user(void __user *, unsigned long);
 extern __must_check long strlen_user(const char __user *str);
 extern __must_check long strnlen_user(const char __user *str, long n);
 
-#define __copy_to_user_inatomic ___copy_to_user
-#define __copy_from_user_inatomic ___copy_from_user
+#define __copy_to_user_inatomic __copy_to_user
+#define __copy_from_user_inatomic __copy_from_user
 
 struct pt_regs;
 extern unsigned long compute_effective_address(struct pt_regs *,
-- 
1.8.5.1

Re: [PATCH] Revert "sparc64: Fix __copy_{to,from}_user_inatomic defines."

[David Miller]

From: Dave Kleikamp <dave.kleikamp@oracle.com>
Date: Mon, 16 Dec 2013 15:01:00 -0600

> This reverts commit 145e1c0023585e0e8f6df22316308ec61c5066b2.
> 
> This commit broke the behavior of __copy_from_user_inatomic when
> it is only partially successful. Instead of returning the number
> of bytes not copied, it now returns 1. This translates to the
> wrong value being returned by iov_iter_copy_from_user_atomic.
> 
> xfstests generic/246 and LTP writev01 both fail on btrfs and nfs
> because of this.
> 
> Signed-off-by: Dave Kleikamp <dave.kleikamp@oracle.com>

Applied and queued up for -stable, thanks.

But I wonder about the original bug that Hugh was trying to
fix :-/

Re: [PATCH] Revert "sparc64: Fix __copy_{to,from}_user_inatomic defines."

[Hugh Dickins]

On Sat, 4 Jan 2014, David Miller wrote:
> From: Dave Kleikamp <dave.kleikamp@oracle.com>
> Date: Mon, 16 Dec 2013 15:01:00 -0600
> 
> > This reverts commit 145e1c0023585e0e8f6df22316308ec61c5066b2.
> > 
> > This commit broke the behavior of __copy_from_user_inatomic when
> > it is only partially successful. Instead of returning the number
> > of bytes not copied, it now returns 1. This translates to the
> > wrong value being returned by iov_iter_copy_from_user_atomic.
> > 
> > xfstests generic/246 and LTP writev01 both fail on btrfs and nfs
> > because of this.
> > 
> > Signed-off-by: Dave Kleikamp <dave.kleikamp@oracle.com>
> 
> Applied and queued up for -stable, thanks.

Good, thank you both.

I'm afraid it's taken me until this evening to set aside
some quiet time to look into this, and the original report.

I now agree that my 2.6.28 patch was completely bogus: relying
upon wishful thinking of what ___copy_from_user() might return,
and not actually addressing the bug in question - the fault in
fixup's __bzero was on the kernel address 0xfffff80037c1c000,
whereas I was fussing over user address faults and atomicity.

I apologize for endangering sparc64 writes over five years.

> 
> But I wonder about the original bug that Hugh was trying to
> fix :-/

That worried me too, but I think you're okay.  I bet your
b270ee8a9fc9 "sparc64: Fix offset calculation in compute_size()"
    
    The fault address is somewhere inside of the buffer, not
    before it.

, also included in 2.6.38, was the actual fix to that bug.

Perhaps that would require CONFIG_DEBUG_PAGEALLOC to have
been in force, and there was no mention of it in the report.
But I see sparc64 was supporting it at the time, and the line
which might have shown it was cut from the report.  And I see
from other reports by Alexander Beregalov that he often had
lockdep on, so there's a good chance he used other such debug
options too.  But it seems a bit late to go asking him now:
let's assume your fix to compute_size() was the answer.

Hugh

Re: [PATCH] Revert "sparc64: Fix __copy_{to,from}_user_inatomic defines."

[David Miller]

From: Hugh Dickins <hughd@google.com>
Date: Mon, 6 Jan 2014 00:42:34 -0800 (PST)

> On Sat, 4 Jan 2014, David Miller wrote:
>> But I wonder about the original bug that Hugh was trying to
>> fix :-/
> 
> That worried me too, but I think you're okay.  I bet your
> b270ee8a9fc9 "sparc64: Fix offset calculation in compute_size()"
>     
>     The fault address is somewhere inside of the buffer, not
>     before it.
> 
> , also included in 2.6.38, was the actual fix to that bug.

That does seem to account for it, thanks Hugh.