linux-kernel - [PATCH] m68k: Patch for broken lsl64() "old code" with incorrect shift

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <BANLkTimUSvQi9CPgO+cCUrEdQq-+ohbzFg@mail.gmail.com>
Date:	Fri, 10 Jun 2011 23:50:05 -0400
From:	Jonathan Elchison <JElchison@...il.com>
To:	Geert Uytterhoeven <geert@...ux-m68k.org>,
	linux-m68k@...ts.linux-m68k.org, Jiri Kosina <trivial@...nel.org>
Cc:	linux-kernel@...r.kernel.org
Subject: [PATCH] m68k: Patch for broken lsl64() "old code" with incorrect shift

[1.] One line summary of the problem:

[PATCH] m68k: Patch for broken lsl64() "old code" with incorrect shift


[2.] Full description of the problem/report:

In arch/m68k/math-emu/multi_arith.h, lsl64() doesn't calculate a
correct HI_WORD(*dest) when count < 32, due to an incorrect shift
operation.  This line:

    HI_WORD(*dest) = (HI_WORD(*dest) << count) | (LO_WORD(*dest) >> count);

...should be...

    HI_WORD(*dest) = (HI_WORD(*dest) << count) | (LO_WORD(*dest) >>
(32 - count));

This function is ifdef'd out, marked as "old code".  However, I fell
into the same trap into which I suspect others have fallen (or will
fall):  On rare occasion when I need to implement my own library
functions, I depend on the Linux kernel to provide solid code.  This
bug, albeit in old code, led me astray.

I'm asking that this old code (lsl64) be corrected or removed.


[3.] Keywords (i.e., modules, networking, kernel):

kernel, arch, m68k, math-emu, multi_arith.h, lsl64


[4.] Kernel information

2.6.39.1


[5.] Most recent kernel version which did not have the bug:

Unknown


[6.] Output of Oops.. message (if applicable) with symbolic
information resolved (see Documentation/oops-tracing.txt)

N/A


[7.] A small shell script or example program which triggers the
problem (if possible)

-----------------------------------------------------------------
#include <stdio.h>

#define LO_WORD(ll) (((unsigned int *) &ll)[1])
#define HI_WORD(ll) (((unsigned int *) &ll)[0])

static inline void lsl64_broken(int count, unsigned long long *dest)
{
	if (count < 32) {
		HI_WORD(*dest) = (HI_WORD(*dest) << count)
		    | (LO_WORD(*dest) >> count);
		LO_WORD(*dest) <<= count;
		return;
	}
	count -= 32;
	HI_WORD(*dest) = LO_WORD(*dest) << count;
	LO_WORD(*dest) = 0;
}

static inline void lsl64_fixed(int count, unsigned long long *dest)
{
	if (count < 32) {
		HI_WORD(*dest) = (HI_WORD(*dest) << count)
		    | (LO_WORD(*dest) >> (32 - count));
		LO_WORD(*dest) <<= count;
		return;
	}
	count -= 32;
	HI_WORD(*dest) = LO_WORD(*dest) << count;
	LO_WORD(*dest) = 0;
}

static inline void lsr64(int count, unsigned long long *dest)
{
	if (count < 32) {
		LO_WORD(*dest) = (LO_WORD(*dest) >> count)
		    | (HI_WORD(*dest) << (32 - count));
		HI_WORD(*dest) >>= count;
		return;
	}
	count -= 32;
	LO_WORD(*dest) = HI_WORD(*dest) >> count;
	HI_WORD(*dest) = 0;
}

int main( void )
{
	long long testVal = 0x0000100000001000L;
	long long testVal_broken = testVal;
	long long testVal_fixed = testVal;
	
	printf( "begin testVal_broken =\t\t0x%016llx\n", testVal_broken );
	lsr64( 1, &testVal_broken );
	printf( "intermediate testVal_broken =\t0x%016llx\n", testVal_broken );
	lsl64_broken( 1, &testVal_broken );
	printf( "final testVal_broken =\t\t0x%016llx\n", testVal_broken );
	printf( "final matches begin?\t\t%s\n", testVal_broken == testVal ?
"TRUE" : "FALSE" );

	printf( "\nbegin testVal_fixed =\t\t0x%016llx\n", testVal_fixed );
	lsr64( 1, &testVal_fixed );
	printf( "intermediate testVal_fixed =\t0x%016llx\n", testVal_fixed );
	lsl64_fixed( 1, &testVal_fixed );
	printf( "final testVal_fixed =\t\t0x%016llx\n", testVal_fixed );
	printf( "final matches begin?\t\t%s\n", testVal_fixed == testVal ?
"TRUE" : "FALSE" );

	return 0;
}
-----------------------------------------------------------------


[8.] Environment

Found via code inspection


[9.] Patch

Signed-off-by: Jonathan Elchison <jelchison@...il.com>

-----------------------------------------------------------------
diff -up linux-2.6/arch/m68k/math-emu/multi_arith.h
linux-2.6_orig/arch/m68k/math-emu/multi_arith.h
--- linux-2.6/arch/m68k/math-emu/multi_arith.h	2011-06-10
22:50:32.538711320 -0400
+++ linux-2.6_orig/arch/m68k/math-emu/multi_arith.h	2011-06-10
22:47:43.407285452 -0400
@@ -236,7 +236,7 @@ static inline void lsl64(int count, unsi
 {
 	if (count < 32) {
 		HI_WORD(*dest) = (HI_WORD(*dest) << count)
-		    | (LO_WORD(*dest) >> (32 - count));
+		    | (LO_WORD(*dest) >> count);
 		LO_WORD(*dest) <<= count;
 		return;
 	}
-----------------------------------------------------------------


[10.] Responses

Please personally CC me on responses to this post.

--
Jonathan Elchison
jelchison@...il.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/