SPARC32: Fixed unaligned memory copying in function __csum_partial_copy_sparc_generic

Hello, Dave!

I'm sorry, there is a error in my previous letter. I added excess annul field to bge.
Final patch from me is following:

When we are in the label cc_dword_align, registers %o0 and %o1 have the same last 2 bits,
but it's not guaranteed one of they is zero. So we can get unaligned memory access
in label ccte. Example of parameters which lead to this:
%o0=0x7ff183e9, %o1=0x8e709e7d, %g1=3

With the parameters I had a memory corruption, when the additional 5 bytes were rewritten.
This patch corrects the error.

One comment to the patch. We don't care about the third bit in %o1, because cc_end_cruft
stores word or less.

Thanks. Kirill.

Signed-off-by: Tkhai Kirill <tkhai@yandex.ru>
---

--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

From: Tkhai Kirill <tkhai@yandex.ru>
Date: Tue, 10 May 2011 16:31:41 +0400

>  	/* Also, handle the alignment code out of band. */
>  cc_dword_align:
> -	cmp	%g1, 6
> -	bl,a	ccte
> +	cmp	%g1, 16
> +	bge	1f
> +	 srl	%g1, 1, %o3
> +2:	cmp	%o3, 0
> +	be,a	ccte
>  	 andcc	%g1, 0xf, %o3
> -	andcc	%o0, 0x1, %g0
> +	andcc	%o3, %o0, %g0	! Check %o0 only (%o1 has the same last 2 bits)
> +	be,a	2b
> +	 srl	%o3, 1, %o3
> +1:	andcc	%o0, 0x1, %g0

I don't understand why you're testing "andcc %o3, %o0, %g0", as %o3 has
the value computed in "srl %g1, 1, %o3" here.

What is so interesting about "ptr & (length >> 1)"?
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

11.05.11, 00:28, "David Miller" <davem@davemloft.net>:

>I don't understand why you're testing "andcc %o3, %o0, %g0", as %o3 has
>the value computed in "srl %g1, 1, %o3" here.

>What is so interesting about "ptr & (length >> 1)"?

1)Label ccte (rather, cc_end_cruft) copies data sequentially by pieces of length 2^x bytes from biggest to smallest. For example, if %g1 is 15, then the label copies 8+4+2+1=15, and %o0 must be aligned to 8.

Let alignment a = 2^x. Pointer ptr % a == 0 when:
ptr & (a >> 1) == 0
ptr & (a >> 2) == 0
..

Or, what is the same:
ptr & (length >> 1) == 0
ptr & (length >> 2) == 0
..

I.e. we are looking for right alignment, if another case don't call ccte.

2)We check %o0 only, because %o1 has the same last two bits, and cc_end_cruft writes 4 bytes or less.
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

From: Tkhai Kirill <tkhai@yandex.ru>
Date: Wed, 11 May 2011 03:21:49 +0400

> 11.05.11, 00:28, "David Miller" <davem@davemloft.net>:
> 
>>I don't understand why you're testing "andcc %o3, %o0, %g0", as %o3 has
>>the value computed in "srl %g1, 1, %o3" here.
> 
>>What is so interesting about "ptr & (length >> 1)"?
> 
> 1)Label ccte (rather, cc_end_cruft) copies data sequentially by pieces of length 2^x bytes from biggest to smallest. For example, if %g1 is 15, then the label copies 8+4+2+1=15, and %o0 must be aligned to 8.
> 
> Let alignment a = 2^x. Pointer ptr % a == 0 when:
> ptr & (a >> 1) == 0
> ptr & (a >> 2) == 0
> ..
> 
> Or, what is the same:
> ptr & (length >> 1) == 0
> ptr & (length >> 2) == 0
> ..
> 
> I.e. we are looking for right alignment, if another case don't call ccte.
> 
> 2)We check %o0 only, because %o1 has the same last two bits, and cc_end_cruft writes 4 bytes or less.

Now I understand, thanks for explaining.

Applied.
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html