linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] lzo: fix ip overrun during compress.
@ 2018-11-27 10:34 Yueyi Li
  2018-11-28  4:08 ` Willy Tarreau
  0 siblings, 1 reply; 3+ messages in thread
From: Yueyi Li @ 2018-11-27 10:34 UTC (permalink / raw)
  To: gregkh, w; +Cc: donb, linux-kernel, Yueyi Li

It`s possible ip overrun in lzo1x_1_do_compress() when compressed page is
point to the end of memory and which virtual address is 0xfffffffffffff000.
Leading to a NULL pointer access during the get_unaligned_le32(ip).

Fix this panic:
[ 2738.034508] Unable to handle kernel NULL pointer dereference at virtual address 00000009
[ 2738.034515] Mem abort info:
[ 2738.034518]   Exception class = DABT (current EL), IL = 32 bits
[ 2738.034520]   SET = 0, FnV = 0
[ 2738.034523]   EA = 0, S1PTW = 0
[ 2738.034524]   FSC = 5
[ 2738.034526] Data abort info:
[ 2738.034528]   ISV = 0, ISS = 0x00000005
[ 2738.034530]   CM = 0, WnR = 0
[ 2738.034533] user pgtable: 4k pages, 39-bit VAs, pgd = ffffffff94cee000
[ 2738.034535] [0000000000000009] *pgd=0000000000000000, *pud=0000000000000000
...
[ 2738.034592] pc : lzo1x_1_do_compress+0x198/0x610
[ 2738.034595] lr : lzo1x_1_compress+0x98/0x3d8
[ 2738.034597] sp : ffffff801caa3470 pstate : 00c00145
[ 2738.034598] x29: ffffff801caa3500 x28: 0000000000001000
[ 2738.034601] x27: 0000000000001000 x26: fffffffffffff000
[ 2738.034604] x25: ffffffff4ebc0000 x24: 0000000000000000
[ 2738.034607] x23: 000000000000004c x22: fffffffffffff7b8
[ 2738.034610] x21: ffffffff2e2ee0b3 x20: ffffffff2e2ee0bb
[ 2738.034612] x19: 0000000000000fcc x18: fffffffffffff84a
[ 2738.034615] x17: 00000000801b03d6 x16: 0000000000000782
[ 2738.034618] x15: ffffffff2e2ee0bf x14: fffffffffffffff0
[ 2738.034620] x13: 000000000000000f x12: 0000000000000020
[ 2738.034623] x11: 000000001824429d x10: ffffffffffffffec
[ 2738.034626] x9 : 0000000000000009 x8 : 0000000000000000
[ 2738.034628] x7 : 0000000000000868 x6 : 0000000000000434
[ 2738.034631] x5 : ffffffff4ebc0000 x4 : 0000000000000000
[ 2738.034633] x3 : ffffff801caa3510 x2 : ffffffff2e2ee000
[ 2738.034636] x1 : 0000000000000000 x0 : fffffffffffff000
...
[ 2738.034717] Process kworker/u16:1 (pid: 8705, stack limit = 0xffffff801caa0000)
[ 2738.034720] Call trace:
[ 2738.034722]  lzo1x_1_do_compress+0x198/0x610
[ 2738.034725]  lzo_compress+0x48/0x88
[ 2738.034729]  crypto_compress+0x14/0x20
[ 2738.034733]  zcomp_compress+0x2c/0x38
[ 2738.034736]  zram_bvec_rw+0x3d0/0x860
[ 2738.034738]  zram_rw_page+0x88/0xe0
[ 2738.034742]  bdev_write_page+0x70/0xc0
[ 2738.034745]  __swap_writepage+0x58/0x3f8
[ 2738.034747]  swap_writepage+0x40/0x50
[ 2738.034750]  shrink_page_list+0x4fc/0xe58
[ 2738.034753]  reclaim_pages_from_list+0xa0/0x150
[ 2738.034756]  reclaim_pte_range+0x18c/0x1f8
[ 2738.034759]  __walk_page_range+0xf8/0x1e0
[ 2738.034762]  walk_page_range+0xf8/0x130
[ 2738.034765]  reclaim_task_anon+0xcc/0x168
[ 2738.034767]  swap_fn+0x438/0x668
[ 2738.034771]  process_one_work+0x1fc/0x460
[ 2738.034773]  worker_thread+0x2d0/0x478
[ 2738.034775]  kthread+0x110/0x120
[ 2738.034778]  ret_from_fork+0x10/0x18
[ 2738.034781] Code: 3800167f 54ffffa8 d100066f 14000031 (b9400131)
[ 2738.034784] ---[ end trace 9b5cca106f0e54d1 ]---
[ 2738.035473] Kernel panic - not syncing: Fatal exception

crash> dis lzo1x_1_do_compress+100 3 -l
../kernel/msm-4.14/lib/lzo/lzo1x_compress.c: 44
0xffffff8dec8c6af4 <lzo1x_1_do_compress+100>:   cmp     x9, x10
0xffffff8dec8c6af8 <lzo1x_1_do_compress+104>:   b.cc    0xffffff8dec8c6c28
0xffffff8dec8c6afc <lzo1x_1_do_compress+108>:   b       0xffffff8dec8c7094

crash> dis lzo1x_1_do_compress+0x198
0xffffff8dec8c6c28 <lzo1x_1_do_compress+408>:   ldr     w17, [x9]

ip = x9 = 0x0000000000000009 is overflow.

Signed-off-by: liyueyi <liyueyi@live.com>
---
 lib/lzo/lzo1x_compress.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/lib/lzo/lzo1x_compress.c b/lib/lzo/lzo1x_compress.c
index 236eb21..a0265a6 100644
--- a/lib/lzo/lzo1x_compress.c
+++ b/lib/lzo/lzo1x_compress.c
@@ -17,6 +17,9 @@
 #include <linux/lzo.h>
 #include "lzodefs.h"
 
+#define OVERFLOW_ADD_CHECK(a, b)  \
+	((size_t)~0 - (size_t)(a) < (size_t)(b) ? true : false)
+
 static noinline size_t
 lzo1x_1_do_compress(const unsigned char *in, size_t in_len,
 		    unsigned char *out, size_t *out_len,
@@ -39,6 +42,8 @@ lzo1x_1_do_compress(const unsigned char *in, size_t in_len,
 		size_t t, m_len, m_off;
 		u32 dv;
 literal:
+		if (unlikely(OVERFLOW_ADD_CHECK(ip, 1 + ((ip - ii) >> 5))))
+			break;
 		ip += 1 + ((ip - ii) >> 5);
 next:
 		if (unlikely(ip >= ip_end))
@@ -99,7 +104,8 @@ lzo1x_1_do_compress(const unsigned char *in, size_t in_len,
 				m_len += 8;
 				v = get_unaligned((const u64 *) (ip + m_len)) ^
 				    get_unaligned((const u64 *) (m_pos + m_len));
-				if (unlikely(ip + m_len >= ip_end))
+				if (unlikely(OVERFLOW_ADD_CHECK(ip, m_len)
+						|| (ip + m_len >= ip_end)))
 					goto m_len_done;
 			} while (v == 0);
 		}
@@ -124,7 +130,8 @@ lzo1x_1_do_compress(const unsigned char *in, size_t in_len,
 				m_len += 4;
 				v = get_unaligned((const u32 *) (ip + m_len)) ^
 				    get_unaligned((const u32 *) (m_pos + m_len));
-				if (unlikely(ip + m_len >= ip_end))
+				if (unlikely(OVERFLOW_ADD_CHECK(ip, m_len)
+						|| (ip + m_len >= ip_end)))
 					goto m_len_done;
 			} while (v == 0);
 		}
@@ -160,7 +167,8 @@ lzo1x_1_do_compress(const unsigned char *in, size_t in_len,
 				if (ip[m_len] != m_pos[m_len])
 					break;
 				m_len += 1;
-				if (unlikely(ip + m_len >= ip_end))
+				if (unlikely(OVERFLOW_ADD_CHECK(ip, m_len)
+						|| (ip + m_len >= ip_end)))
 					goto m_len_done;
 			} while (ip[m_len] == m_pos[m_len]);
 		}
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] lzo: fix ip overrun during compress.
  2018-11-27 10:34 [PATCH] lzo: fix ip overrun during compress Yueyi Li
@ 2018-11-28  4:08 ` Willy Tarreau
  2018-11-28  6:16   ` Yueyi Li
  0 siblings, 1 reply; 3+ messages in thread
From: Willy Tarreau @ 2018-11-28  4:08 UTC (permalink / raw)
  To: Yueyi Li; +Cc: gregkh, donb, linux-kernel

Hi Yueyi,

On Tue, Nov 27, 2018 at 10:34:26AM +0000, Yueyi Li wrote:
> It`s possible ip overrun in lzo1x_1_do_compress() when compressed page is
> point to the end of memory and which virtual address is 0xfffffffffffff000.
> Leading to a NULL pointer access during the get_unaligned_le32(ip).

Thanks for this report.

(...)
> diff --git a/lib/lzo/lzo1x_compress.c b/lib/lzo/lzo1x_compress.c
> index 236eb21..a0265a6 100644
> --- a/lib/lzo/lzo1x_compress.c
> +++ b/lib/lzo/lzo1x_compress.c
> @@ -17,6 +17,9 @@
>  #include <linux/lzo.h>
>  #include "lzodefs.h"
>  
> +#define OVERFLOW_ADD_CHECK(a, b)  \
> +	((size_t)~0 - (size_t)(a) < (size_t)(b) ? true : false)
> +

I think the test would be easier to grasp this way :

#define OVERFLOW_ADD_CHECK(a, b)  \
	((size_t)(b) >= (size_t)((void*)0 - (void *)(a)))

But the following should be more efficient since we build with
-fno-strict-overflow :

#define OVERFLOW_ADD_CHECK(a, b)  \
	(((a) + (b)) < (a))

Could you please recheck ?

Thanks,
Willy

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] lzo: fix ip overrun during compress.
  2018-11-28  4:08 ` Willy Tarreau
@ 2018-11-28  6:16   ` Yueyi Li
  0 siblings, 0 replies; 3+ messages in thread
From: Yueyi Li @ 2018-11-28  6:16 UTC (permalink / raw)
  To: Willy Tarreau; +Cc: gregkh, donb, linux-kernel

Hi Willy,


On 2018/11/28 12:08, Willy Tarreau wrote:
> Hi Yueyi,
>
> On Tue, Nov 27, 2018 at 10:34:26AM +0000, Yueyi Li wrote:
>> It`s possible ip overrun in lzo1x_1_do_compress() when compressed page is
>> point to the end of memory and which virtual address is 0xfffffffffffff000.
>> Leading to a NULL pointer access during the get_unaligned_le32(ip).
> Thanks for this report.
>
> (...)
>> diff --git a/lib/lzo/lzo1x_compress.c b/lib/lzo/lzo1x_compress.c
>> index 236eb21..a0265a6 100644
>> --- a/lib/lzo/lzo1x_compress.c
>> +++ b/lib/lzo/lzo1x_compress.c
>> @@ -17,6 +17,9 @@
>>   #include <linux/lzo.h>
>>   #include "lzodefs.h"
>>   
>> +#define OVERFLOW_ADD_CHECK(a, b)  \
>> +	((size_t)~0 - (size_t)(a) < (size_t)(b) ? true : false)
>> +
> I think the test would be easier to grasp this way :
>
> #define OVERFLOW_ADD_CHECK(a, b)  \
> 	((size_t)(b) >= (size_t)((void*)0 - (void *)(a)))
>
> But the following should be more efficient since we build with
> -fno-strict-overflow :
>
> #define OVERFLOW_ADD_CHECK(a, b)  \
> 	(((a) + (b)) < (a))
Thanks for the suggestion,  I will change it in next version.
> Could you please recheck ?
>
> Thanks,
> Willy

Thanks,
Yueyi

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2018-11-28  6:16 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-27 10:34 [PATCH] lzo: fix ip overrun during compress Yueyi Li
2018-11-28  4:08 ` Willy Tarreau
2018-11-28  6:16   ` Yueyi Li

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).