linux-kernel - Re: [BUG] lib: zram lz4 compression/decompression still broken on big endian

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-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20160406130911.GA584@swordfish>
Date:	Wed, 6 Apr 2016 22:09:11 +0900
From:	Sergey Senozhatsky <sergey.senozhatsky@...il.com>
To:	Rui Salvaterra <rsalvaterra@...il.com>
Cc:	Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>,
	Greg KH <gregkh@...uxfoundation.org>,
	linux-kernel@...r.kernel.org, eunb.song@...sung.com,
	minchan@...nel.org, linux-mm@...ck.org,
	Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
	Chanho Min <chanho.min@....com>,
	Kyungsik Lee <kyungsik.lee@....com>
Subject: Re: [BUG] lib: zram lz4 compression/decompression still broken on
 big endian

Cc Chanho Min, Kyungsik Lee


Hello,

On (04/06/16 10:39), Rui Salvaterra wrote:
> > may we please ask you to test the patch first? quite possible there
> > is nothing to fix there; I've no access to mips h/w but the patch
> > seems correct to me.
> >
> > LZ4_READ_LITTLEENDIAN_16 does get_unaligned_le16(), so
> > LZ4_WRITE_LITTLEENDIAN_16 must do put_unaligned_le16() /* not put_unaligned() */
> >
[..]
> Consequentially, while I believe the patch will fix the mips case, I'm
> not so sure about ppc (or any other big endian architecture with
> efficient unaligned accesses).

frankly, yes, I took a quick look today (after I sent my initial
message, tho) ... and it is fishy, I agree. was going to followup
on my email but somehow got interrupted, sorry.

so we have, write:
	((U16_S *)(p)) = v    OR    put_unaligned(v, (u16 *)(p))

and only one read:
	get_unaligned_le16(p))

I guess it's either read part also must depend on
HAVE_EFFICIENT_UNALIGNED_ACCESS, or write path
should stop doing so.

I ended up with two patches, NONE was tested (!!!). like at all.

1) provide CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS-dependent
   LZ4_READ_LITTLEENDIAN_16

2) provide common LZ4_WRITE_LITTLEENDIAN_16 and LZ4_READ_LITTLEENDIAN_16
   regardless CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS.


assuming that common LZ4_WRITE_LITTLEENDIAN_16 will somehow hit the
performance, I'd probably prefer option #1.

the patch is below. would be great if you can help testing it.

---

 lib/lz4/lz4defs.h | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/lib/lz4/lz4defs.h b/lib/lz4/lz4defs.h
index abcecdc..a23e6c2 100644
--- a/lib/lz4/lz4defs.h
+++ b/lib/lz4/lz4defs.h
@@ -36,10 +36,14 @@ typedef struct _U64_S { u64 v; } U64_S;
 #define PUT4(s, d) (A32(d) = A32(s))
 #define PUT8(s, d) (A64(d) = A64(s))
 #define LZ4_WRITE_LITTLEENDIAN_16(p, v)	\
-	do {	\
-		A16(p) = v; \
-		p += 2; \
+	do {					\
+		A16(p) = v; 			\
+		p += 2; 			\
 	} while (0)
+
+#define LZ4_READ_LITTLEENDIAN_16(d, s, p)	\
+	(d = s - A16(p))
+
 #else /* CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS */
 
 #define A64(x) get_unaligned((u64 *)&(((U16_S *)(x))->v))
@@ -52,10 +56,13 @@ typedef struct _U64_S { u64 v; } U64_S;
 	put_unaligned(get_unaligned((const u64 *) s), (u64 *) d)
 
 #define LZ4_WRITE_LITTLEENDIAN_16(p, v)	\
-	do {	\
-		put_unaligned(v, (u16 *)(p)); \
-		p += 2; \
+	do {						\
+		put_unaligned_le16(v, (u16 *)(p));	\
+		p += 2; 				\
 	} while (0)
+
+#define LZ4_READ_LITTLEENDIAN_16(d, s, p) 		\
+	(d = s - get_unaligned_le16(p))
 #endif
 
 #define COPYLENGTH 8
@@ -140,9 +147,6 @@ typedef struct _U64_S { u64 v; } U64_S;
 
 #endif
 
-#define LZ4_READ_LITTLEENDIAN_16(d, s, p) \
-	(d = s - get_unaligned_le16(p))
-
 #define LZ4_WILDCOPY(s, d, e)		\
 	do {				\
 		LZ4_COPYPACKET(s, d);	\