[PATCH] sparc32: unaligned memory access (MNA) trap handler bug

[PATCH] sparc32: unaligned memory access (MNA) trap handler bug

[Daniel Hellstrom]

Since the merge process of the sparc and sparc64 the sparc32
MNA trap handler does not emulate stores to unaligned addresses
correctly. MNA operation from both from kernel and user space
are affected.

A typical effect of this bug is nr_frags in skbs are overwritten
during buffer copying/checksum-calculation, or maximally 6 bytes
of data in the network buffer will be overwitten with garbage.

Signed-off-by: Daniel Hellstrom <daniel@gaisler.com>
---
 arch/sparc/kernel/una_asm_32.S |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/sparc/kernel/una_asm_32.S b/arch/sparc/kernel/una_asm_32.S
index 8cc0345..8f096e8 100644
--- a/arch/sparc/kernel/una_asm_32.S
+++ b/arch/sparc/kernel/una_asm_32.S
@@ -24,9 +24,9 @@ retl_efault:
 	.globl	__do_int_store
 __do_int_store:
 	ld	[%o2], %g1
-	cmp	%1, 2
+	cmp	%o1, 2
 	be	2f
-	 cmp	%1, 4
+	 cmp	%o1, 4
 	be	1f
 	 srl	%g1, 24, %g2
 	srl	%g1, 16, %g7
-- 
1.5.4

Re: [PATCH] sparc32: unaligned memory access (MNA) trap handler bug

[Sam Ravnborg]

On Tue, Feb 01, 2011 at 07:03:43PM +0100, Daniel Hellstrom wrote:
> Since the merge process of the sparc and sparc64 the sparc32
> MNA trap handler does not emulate stores to unaligned addresses
> correctly. MNA operation from both from kernel and user space
> are affected.

Well spotted!

This bug was actually introduced by:
f0e98c387e61de00646be31fab4c2fa0224e1efb "[SPARC]: Fix link errors with gcc-4.3"

I wanted to check if there were similar bugs introduced, but
this looks like a solo incident.

I grepped for "%1" in the other *.S files, no hits.
[Likewise for "%2", "%3", "%4"]

Patch looks good to me.
Reluctant with an "Acked-by" only because I do not feel confident
with the code in question. The change looks obviously correct.

I noticed you copied Greg on this. I guess you did so because you
consider this -stable material.

A better way to do so is to add:

Cc: <stable@kernel.org>

Then the stable team(s) will all be notified when this patch is applied
by Linus to mainline.

This is also a strong hint for David to apply this to sparc-2.6.git and not
sparc-next-2.6.git
But being explicit is always preferred.

	Sam

Re: [PATCH] sparc32: unaligned memory access (MNA) trap handler bug

[David Miller]

From: Sam Ravnborg <sam@ravnborg.org>
Date: Tue, 1 Feb 2011 20:59:26 +0100

> On Tue, Feb 01, 2011 at 07:03:43PM +0100, Daniel Hellstrom wrote:
>> Since the merge process of the sparc and sparc64 the sparc32
>> MNA trap handler does not emulate stores to unaligned addresses
>> correctly. MNA operation from both from kernel and user space
>> are affected.
> 
> Well spotted!

Indeed.

> This bug was actually introduced by:
> f0e98c387e61de00646be31fab4c2fa0224e1efb "[SPARC]: Fix link errors with gcc-4.3"

I'll make a note of this in the commit message.

> A better way to do so is to add:
> 
> Cc: <stable@kernel.org>
> 
> Then the stable team(s) will all be notified when this patch is applied
> by Linus to mainline.

I'll take care of the -stable submission.

I wish binutils wouldn't accept %1 as a register, nobody sane uses that
format for register specifications and when it does happen (like here)
it's a typo and a bug.

Thanks guys, I'll apply this and queue it up for -stable!

Re: [PATCH] sparc32: unaligned memory access (MNA) trap handler bug

[Daniel Hellstrom]

David Miller wrote:

>From: Sam Ravnborg <sam@ravnborg.org>
>Date: Tue, 1 Feb 2011 20:59:26 +0100
>
>  
>
>>On Tue, Feb 01, 2011 at 07:03:43PM +0100, Daniel Hellstrom wrote:
>>    
>>
>>>Since the merge process of the sparc and sparc64 the sparc32
>>>MNA trap handler does not emulate stores to unaligned addresses
>>>correctly. MNA operation from both from kernel and user space
>>>are affected.
>>>      
>>>
>>Well spotted!
>>    
>>
>
>Indeed.
>  
>
Thanks, we have seen this problem pop up now and then since nov/dec but 
unable to trigger it stable. Debugging such a problem often requires a 
quite long time and since I can not work fulltime with Linux it took 
some time before I could fully focus on it. Of course at first I thought 
if was a LEON problem, then a GRETH network driver problem, then SMP 
related... Last week I spent a lot time tracing, and thanks to a great 
HW team here at Gaisler that made me a system with better hardware 
debugging support and the excellent GRMON I was able to find it at last.

I really hope that the sparc community can take benefit of the time we 
invested in this. Now that we are more synced with the official kernel 
and hopefully SMP will work better, I think it will be much easier for 
us to spot and fix generic problems as they appear in the future. Thanks!

>  
>
>>This bug was actually introduced by:
>>f0e98c387e61de00646be31fab4c2fa0224e1efb "[SPARC]: Fix link errors with gcc-4.3"
>>    
>>
>
>I'll make a note of this in the commit message.
>
>  
>
>>A better way to do so is to add:
>>
>>Cc: <stable@kernel.org>
>>
>>Then the stable team(s) will all be notified when this patch is applied
>>by Linus to mainline.
>>    
>>
>
>I'll take care of the -stable submission.
>  
>
Good, thanks, that was my intention but didn't know there was a 
procedure like this. Will stick to that in the future if more problems 
like this arise.

>I wish binutils wouldn't accept %1 as a register, nobody sane uses that
>format for register specifications and when it does happen (like here)
>it's a typo and a bug.
>  
>
I agree.

>Thanks guys, I'll apply this and queue it up for -stable!
>  
>
nice.