[PATCH] m32r: Fix clearing of thread info fault code

[PATCH] m32r: Fix clearing of thread info fault code

[Michael Büsch]

[-- Attachment #1: Type: text/plain, Size: 1192 bytes --]

The expression (~0 >> x) will always yield all-ones, because the right
shift is an arithmetic right shift that will always shift ones in.
Hence the old fault code bits will not be cleared before being ORed
with the new fault code.

Fix this by forcing a logical right shift instead of an arithmetic
right shift by using an unsigned long constant.

Reported-by: Ilia Mirkin <imirkin@alum.mit.edu>
Signed-off-by: Michael Buesch <m@bues.ch>

---

The code also assumes sizeof(ti->flags) == 4. But that probably is ok for this arch.

This patch is untested, because I do not have the hardware.


Index: linux/arch/m32r/include/asm/thread_info.h
===================================================================
--- linux.orig/arch/m32r/include/asm/thread_info.h
+++ linux/arch/m32r/include/asm/thread_info.h
@@ -77,7 +77,7 @@ static inline struct thread_info *curren
 static inline void set_thread_fault_code(unsigned int val)
 {
 	struct thread_info *ti = current_thread_info();
-	ti->flags = (ti->flags & (~0 >> (32 - TI_FLAG_FAULT_CODE_SHIFT)))
+	ti->flags = (ti->flags & (~0UL >> (32 - TI_FLAG_FAULT_CODE_SHIFT)))
 		| (val << TI_FLAG_FAULT_CODE_SHIFT);
 }
 

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH] m32r: Fix clearing of thread info fault code

[Michael Büsch]

[-- Attachment #1: Type: text/plain, Size: 669 bytes --]

The expression (~0 >> x) will always yield all-ones, because the right
shift is an arithmetic right shift that will always shift ones in.
Hence the old fault code bits will not be cleared before being ORed
with the new fault code.

Fix this by forcing a logical right shift instead of an arithmetic
right shift by using an unsigned long constant.

Reported-by: Ilia Mirkin <imirkin@alum.mit.edu>
Signed-off-by: Michael Buesch <m@bues.ch>

---

The code also assumes sizeof(ti->flags) == 4. But that probably is ok
for this arch.

This patch is untested, because I do not have the hardware.

Resend: Patch was originally sent on Wed, 18 Jun 2015.



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH] m32r: Fix clearing of thread info fault code

[Michael Büsch]

[-- Attachment #1: Type: text/plain, Size: 1321 bytes --]

The expression (~0 >> x) will always yield all-ones, because the right
shift is an arithmetic right shift that will always shift ones in.
Hence the old fault code bits will not be cleared before being ORed
with the new fault code.

Fix this by forcing a logical right shift instead of an arithmetic
right shift by using an unsigned long constant.

Reported-by: Ilia Mirkin <imirkin@alum.mit.edu>
Signed-off-by: Michael Buesch <m@bues.ch>

---

The code also assumes sizeof(ti->flags) == 4. But that probably is ok
for this arch.

This patch is untested, because I do not have the hardware.

Resend: Patch was originally sent on Wed, 18 Jun 2015.

(Sorry, hit the send button early, so here goes the actual patch.)


Index: linux/arch/m32r/include/asm/thread_info.h
===================================================================
--- linux.orig/arch/m32r/include/asm/thread_info.h
+++ linux/arch/m32r/include/asm/thread_info.h
@@ -77,7 +77,7 @@ static inline struct thread_info *curren
 static inline void set_thread_fault_code(unsigned int val)
 {
 	struct thread_info *ti = current_thread_info();
-	ti->flags = (ti->flags & (~0 >> (32 - TI_FLAG_FAULT_CODE_SHIFT)))
+	ti->flags = (ti->flags & (~0UL >> (32 - TI_FLAG_FAULT_CODE_SHIFT)))
 		| (val << TI_FLAG_FAULT_CODE_SHIFT);
 }
 

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] m32r: Fix clearing of thread info fault code

[Andrew Morton]

On Thu, 19 Nov 2015 21:13:19 +0100 Michael B__sch <m@bues.ch> wrote:

> The expression (~0 >> x) will always yield all-ones, because the right
> shift is an arithmetic right shift that will always shift ones in.
> Hence the old fault code bits will not be cleared before being ORed
> with the new fault code.
> 
> Fix this by forcing a logical right shift instead of an arithmetic
> right shift by using an unsigned long constant.
> 
> Reported-by: Ilia Mirkin <imirkin@alum.mit.edu>
> Signed-off-by: Michael Buesch <m@bues.ch>
> 
> ---
> 
> The code also assumes sizeof(ti->flags) = 4. But that probably is ok
> for this arch.
> 
> This patch is untested, because I do not have the hardware.
> 
> Resend: Patch was originally sent on Wed, 18 Jun 2015.
> 
> (Sorry, hit the send button early, so here goes the actual patch.)
>
> ...
>
> --- linux.orig/arch/m32r/include/asm/thread_info.h
> +++ linux/arch/m32r/include/asm/thread_info.h
> @@ -77,7 +77,7 @@ static inline struct thread_info *curren
>  static inline void set_thread_fault_code(unsigned int val)
>  {
>  	struct thread_info *ti = current_thread_info();
> -	ti->flags = (ti->flags & (~0 >> (32 - TI_FLAG_FAULT_CODE_SHIFT)))
> +	ti->flags = (ti->flags & (~0UL >> (32 - TI_FLAG_FAULT_CODE_SHIFT)))
>  		| (val << TI_FLAG_FAULT_CODE_SHIFT);
>  }

I don't think we should apply this unless someone can runtime test it. 
Presumably the current code works OK, but we just don't know what
nasties the fixed version might expose.

The best I can think of is to put a big FIXME comment in there, so
perhaps one day if someone is working on m32r stuff, they may try
fixing it.

Re: [PATCH] m32r: Fix clearing of thread info fault code

[Michael Büsch]

[-- Attachment #1: Type: text/plain, Size: 542 bytes --]

On Thu, 19 Nov 2015 15:08:32 -0800
Andrew Morton <akpm@linux-foundation.org> wrote:

> I don't think we should apply this unless someone can runtime test it. 
> Presumably the current code works OK, but we just don't know what
> nasties the fixed version might expose.

I fully agree. But who can test it?

> The best I can think of is to put a big FIXME comment in there, so
> perhaps one day if someone is working on m32r stuff, they may try
> fixing it.

Or remove the architecture, if nobody is using it? :)

-- 
Michael

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]