Re: [PATCH] PM / Hiberante : optimize swsusp_free()
>From 6cb5fffc41911a29212be52d4ce7e481f5077ccf Mon Sep 17 00:00:00 2001 From: "Tom(JeHyeon) Yeon" Date: Thu, 19 Mar 2015 17:10:45 +0900 Subject: [PATCH] PM / Hiberante : optimize swsusp_free() Our team developed the snapshot booting. Fisrt of all, make a snapshot image, compress it and finally save it in the storage(like mmc). When the system is booting next time, bootloader read it from mmc, decompress it and jump to the kernel. In this circumstance, mili seconds is very important. So, I prepared this patch, but not applied because I missed the time to apply it. And, I came across to find commit fdd64ed. It's very similar to the patch I prepared. I think do { ... } while (fb_pfn != fr_pfn) operation is very similar to my patch. but, it takes a little more time to iterate. So suggest to iterate one of two maps and check whether the other map has the same pfn, finally free the page. Signed-off-by: Tom(JeHyeon) Yeon --- kernel/power/snapshot.c | 43 ++- 1 file changed, 10 insertions(+), 33 deletions(-) diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c index c24d5a2..a1ad801 100644 --- a/kernel/power/snapshot.c +++ b/kernel/power/snapshot.c @@ -726,14 +726,6 @@ static void memory_bm_clear_bit(struct memory_bitmap *bm, unsigned long pfn) clear_bit(bit, addr); } -static void memory_bm_clear_current(struct memory_bitmap *bm) -{ - int bit; - - bit = max(bm->cur.node_bit - 1, 0); - clear_bit(bit, bm->cur.node->data); -} - static int memory_bm_test_bit(struct memory_bitmap *bm, unsigned long pfn) { void *addr; @@ -1342,36 +1334,21 @@ static struct memory_bitmap copy_bm; void swsusp_free(void) { - unsigned long fb_pfn, fr_pfn; + unsigned long pfn; if (!forbidden_pages_map || !free_pages_map) goto out; memory_bm_position_reset(forbidden_pages_map); - memory_bm_position_reset(free_pages_map); - -loop: - fr_pfn = memory_bm_next_pfn(free_pages_map); - fb_pfn = memory_bm_next_pfn(forbidden_pages_map); - - /* -* Find the next bit set in both bitmaps. This is guaranteed to -* terminate when fb_pfn == fr_pfn == BM_END_OF_MAP. -*/ - do { - if (fb_pfn < fr_pfn) - fb_pfn = memory_bm_next_pfn(forbidden_pages_map); - if (fr_pfn < fb_pfn) - fr_pfn = memory_bm_next_pfn(free_pages_map); - } while (fb_pfn != fr_pfn); - - if (fr_pfn != BM_END_OF_MAP && pfn_valid(fr_pfn)) { - struct page *page = pfn_to_page(fr_pfn); - - memory_bm_clear_current(forbidden_pages_map); - memory_bm_clear_current(free_pages_map); - __free_page(page); - goto loop; + for ( ; ; ) { + pfn = memory_bm_next_pfn(forbidden_pages_map); + if (BM_END_OF_MAP == pfn) + break; + if (memory_bm_test_bit(free_pages_map, pfn)) { + memory_bm_clear_bit(forbidden_pages_map, pfn); + memory_bm_clear_bit(free_pages_map, pfn); + __free_page(pfn_to_page(pfn)); + } } out: -- 1.7.9.5 describe it in details. As pavel said, 5ms is not important in the normal booting system. but mili seconds is important in the hibernation or snapshot system. Just suggestion. Thank you. -- On Thursday, March 19, 2015 05:28:58 PM Tom Yeon wrote: > From: "Tom(JeHyeon) Yeon" > > I tested the performance of swsusp_free operation. > The free time took about 58768us before commit fdd64ed, and > the free time took about 40535us after the commit fdd64ed. > > But, I optimized the function before I saw commit fdd64ed. > So, I applied the patch in my system.(ARM Coretex A9, Dual Core 1GHz) > The free time took about 35164us. > I think that the finding routine for the same pfn is redundant This changelog in total pants, sorry. Please write what the patch is doing and why instead of describing your testing experience. The numbers are useful too, but only to show what the gain is, and you need to explain what is changing and why. > Signed-off-by: Tom(JeHyeon) Yeon > --- > kernel/power/snapshot.c | 43 ++- > 1 file changed, 10 insertions(+), 33 deletions(-) > > diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c > index c24d5a2..a1ad801 100644 > --- a/kernel/power/snapshot.c > +++ b/kernel/power/snapshot.c > @@ -726,14 +726,6 @@ static void memory_bm_clear_bit(struct memory_bitmap > *bm, unsigned long pfn) > clear_bit(bit, addr); > } > > -static void memory_bm_clear_current(struct memory_bitmap *bm) > -{ > - int bit; > - > - bit = max(bm->cur.node_bit - 1, 0); > - clear_bit(bit, bm->cur.node->data); > -} > - > static int memory_bm_test_bit(struct memory_bitmap *bm, unsigned long pfn) > { > void
Re: [PATCH] PM / Hiberante : optimize swsusp_free()
From 6cb5fffc41911a29212be52d4ce7e481f5077ccf Mon Sep 17 00:00:00 2001 From: Tom(JeHyeon) Yeon tom.y...@windriver.com Date: Thu, 19 Mar 2015 17:10:45 +0900 Subject: [PATCH] PM / Hiberante : optimize swsusp_free() Our team developed the snapshot booting. Fisrt of all, make a snapshot image, compress it and finally save it in the storage(like mmc). When the system is booting next time, bootloader read it from mmc, decompress it and jump to the kernel. In this circumstance, mili seconds is very important. So, I prepared this patch, but not applied because I missed the time to apply it. And, I came across to find commit fdd64ed. It's very similar to the patch I prepared. I think do { ... } while (fb_pfn != fr_pfn) operation is very similar to my patch. but, it takes a little more time to iterate. So suggest to iterate one of two maps and check whether the other map has the same pfn, finally free the page. Signed-off-by: Tom(JeHyeon) Yeon tom.y...@windriver.com --- kernel/power/snapshot.c | 43 ++- 1 file changed, 10 insertions(+), 33 deletions(-) diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c index c24d5a2..a1ad801 100644 --- a/kernel/power/snapshot.c +++ b/kernel/power/snapshot.c @@ -726,14 +726,6 @@ static void memory_bm_clear_bit(struct memory_bitmap *bm, unsigned long pfn) clear_bit(bit, addr); } -static void memory_bm_clear_current(struct memory_bitmap *bm) -{ - int bit; - - bit = max(bm-cur.node_bit - 1, 0); - clear_bit(bit, bm-cur.node-data); -} - static int memory_bm_test_bit(struct memory_bitmap *bm, unsigned long pfn) { void *addr; @@ -1342,36 +1334,21 @@ static struct memory_bitmap copy_bm; void swsusp_free(void) { - unsigned long fb_pfn, fr_pfn; + unsigned long pfn; if (!forbidden_pages_map || !free_pages_map) goto out; memory_bm_position_reset(forbidden_pages_map); - memory_bm_position_reset(free_pages_map); - -loop: - fr_pfn = memory_bm_next_pfn(free_pages_map); - fb_pfn = memory_bm_next_pfn(forbidden_pages_map); - - /* -* Find the next bit set in both bitmaps. This is guaranteed to -* terminate when fb_pfn == fr_pfn == BM_END_OF_MAP. -*/ - do { - if (fb_pfn fr_pfn) - fb_pfn = memory_bm_next_pfn(forbidden_pages_map); - if (fr_pfn fb_pfn) - fr_pfn = memory_bm_next_pfn(free_pages_map); - } while (fb_pfn != fr_pfn); - - if (fr_pfn != BM_END_OF_MAP pfn_valid(fr_pfn)) { - struct page *page = pfn_to_page(fr_pfn); - - memory_bm_clear_current(forbidden_pages_map); - memory_bm_clear_current(free_pages_map); - __free_page(page); - goto loop; + for ( ; ; ) { + pfn = memory_bm_next_pfn(forbidden_pages_map); + if (BM_END_OF_MAP == pfn) + break; + if (memory_bm_test_bit(free_pages_map, pfn)) { + memory_bm_clear_bit(forbidden_pages_map, pfn); + memory_bm_clear_bit(free_pages_map, pfn); + __free_page(pfn_to_page(pfn)); + } } out: -- 1.7.9.5 describe it in details. As pavel said, 5ms is not important in the normal booting system. but mili seconds is important in the hibernation or snapshot system. Just suggestion. Thank you. -- On Thursday, March 19, 2015 05:28:58 PM Tom Yeon wrote: From: Tom(JeHyeon) Yeon tom.y...@windriver.com I tested the performance of swsusp_free operation. The free time took about 58768us before commit fdd64ed, and the free time took about 40535us after the commit fdd64ed. But, I optimized the function before I saw commit fdd64ed. So, I applied the patch in my system.(ARM Coretex A9, Dual Core 1GHz) The free time took about 35164us. I think that the finding routine for the same pfn is redundant This changelog in total pants, sorry. Please write what the patch is doing and why instead of describing your testing experience. The numbers are useful too, but only to show what the gain is, and you need to explain what is changing and why. Signed-off-by: Tom(JeHyeon) Yeon tom.y...@windriver.com --- kernel/power/snapshot.c | 43 ++- 1 file changed, 10 insertions(+), 33 deletions(-) diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c index c24d5a2..a1ad801 100644 --- a/kernel/power/snapshot.c +++ b/kernel/power/snapshot.c @@ -726,14 +726,6 @@ static void memory_bm_clear_bit(struct memory_bitmap *bm, unsigned long pfn) clear_bit(bit, addr); } -static void memory_bm_clear_current(struct memory_bitmap *bm) -{ - int bit; - - bit = max(bm-cur.node_bit - 1, 0); - clear_bit(bit, bm-cur.node-data); -} - static int memory_bm_test_bit(struct memory_bitmap *bm,
RE: [PATCH] PM / Hiberante : optimize swsusp_free()
I'm sorry not to answer it. I'm too busy nowadays on my project. So, I'll add some details later. Thank you. -Original Message- From: Rafael J. Wysocki [mailto:r...@rjwysocki.net] Sent: Thursday, March 19, 2015 8:37 PM To: Tom(JeHyeon) Yeon Cc: pa...@ucw.cz; BROWN, A LEONARD; jroe...@suse.de; linux...@vger.kernel.org; linux-kernel@vger.kernel.org; Yeon, JeHyeon (Tom) Subject: Re: [PATCH] PM / Hiberante : optimize swsusp_free() On Thursday, March 19, 2015 05:28:58 PM Tom Yeon wrote: > From: "Tom(JeHyeon) Yeon" > > I tested the performance of swsusp_free operation. > The free time took about 58768us before commit fdd64ed, and > the free time took about 40535us after the commit fdd64ed. > > But, I optimized the function before I saw commit fdd64ed. > So, I applied the patch in my system.(ARM Coretex A9, Dual Core 1GHz) > The free time took about 35164us. > I think that the finding routine for the same pfn is redundant This changelog in total pants, sorry. Please write what the patch is doing and why instead of describing your testing experience. The numbers are useful too, but only to show what the gain is, and you need to explain what is changing and why. > Signed-off-by: Tom(JeHyeon) Yeon > --- > kernel/power/snapshot.c | 43 ++- > 1 file changed, 10 insertions(+), 33 deletions(-) > > diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c > index c24d5a2..a1ad801 100644 > --- a/kernel/power/snapshot.c > +++ b/kernel/power/snapshot.c > @@ -726,14 +726,6 @@ static void memory_bm_clear_bit(struct memory_bitmap > *bm, unsigned long pfn) > clear_bit(bit, addr); > } > > -static void memory_bm_clear_current(struct memory_bitmap *bm) > -{ > - int bit; > - > - bit = max(bm->cur.node_bit - 1, 0); > - clear_bit(bit, bm->cur.node->data); > -} > - > static int memory_bm_test_bit(struct memory_bitmap *bm, unsigned long pfn) > { > void *addr; > @@ -1342,36 +1334,21 @@ static struct memory_bitmap copy_bm; > > void swsusp_free(void) > { > - unsigned long fb_pfn, fr_pfn; > + unsigned long pfn; > > if (!forbidden_pages_map || !free_pages_map) > goto out; > > memory_bm_position_reset(forbidden_pages_map); > - memory_bm_position_reset(free_pages_map); > - > -loop: > - fr_pfn = memory_bm_next_pfn(free_pages_map); > - fb_pfn = memory_bm_next_pfn(forbidden_pages_map); > - > - /* > - * Find the next bit set in both bitmaps. This is guaranteed to > - * terminate when fb_pfn == fr_pfn == BM_END_OF_MAP. > - */ > - do { > - if (fb_pfn < fr_pfn) > - fb_pfn = memory_bm_next_pfn(forbidden_pages_map); > - if (fr_pfn < fb_pfn) > - fr_pfn = memory_bm_next_pfn(free_pages_map); > - } while (fb_pfn != fr_pfn); > - > - if (fr_pfn != BM_END_OF_MAP && pfn_valid(fr_pfn)) { > - struct page *page = pfn_to_page(fr_pfn); > - > - memory_bm_clear_current(forbidden_pages_map); > - memory_bm_clear_current(free_pages_map); > - __free_page(page); > - goto loop; > + for ( ; ; ) { > + pfn = memory_bm_next_pfn(forbidden_pages_map); > + if (BM_END_OF_MAP == pfn) > + break; > + if (memory_bm_test_bit(free_pages_map, pfn)) { So why exactly isn't it necessary to look at memory_bm_next_pfn(forbidden_pages_map)? > + memory_bm_clear_bit(forbidden_pages_map, pfn); > + memory_bm_clear_bit(free_pages_map, pfn); > + __free_page(pfn_to_page(pfn)); > + } > } > > out: > -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center.
RE: [PATCH] PM / Hiberante : optimize swsusp_free()
I'm sorry not to answer it. I'm too busy nowadays on my project. So, I'll add some details later. Thank you. -Original Message- From: Rafael J. Wysocki [mailto:r...@rjwysocki.net] Sent: Thursday, March 19, 2015 8:37 PM To: Tom(JeHyeon) Yeon Cc: pa...@ucw.cz; BROWN, A LEONARD; jroe...@suse.de; linux...@vger.kernel.org; linux-kernel@vger.kernel.org; Yeon, JeHyeon (Tom) Subject: Re: [PATCH] PM / Hiberante : optimize swsusp_free() On Thursday, March 19, 2015 05:28:58 PM Tom Yeon wrote: From: Tom(JeHyeon) Yeon tom.y...@windriver.com I tested the performance of swsusp_free operation. The free time took about 58768us before commit fdd64ed, and the free time took about 40535us after the commit fdd64ed. But, I optimized the function before I saw commit fdd64ed. So, I applied the patch in my system.(ARM Coretex A9, Dual Core 1GHz) The free time took about 35164us. I think that the finding routine for the same pfn is redundant This changelog in total pants, sorry. Please write what the patch is doing and why instead of describing your testing experience. The numbers are useful too, but only to show what the gain is, and you need to explain what is changing and why. Signed-off-by: Tom(JeHyeon) Yeon tom.y...@windriver.com --- kernel/power/snapshot.c | 43 ++- 1 file changed, 10 insertions(+), 33 deletions(-) diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c index c24d5a2..a1ad801 100644 --- a/kernel/power/snapshot.c +++ b/kernel/power/snapshot.c @@ -726,14 +726,6 @@ static void memory_bm_clear_bit(struct memory_bitmap *bm, unsigned long pfn) clear_bit(bit, addr); } -static void memory_bm_clear_current(struct memory_bitmap *bm) -{ - int bit; - - bit = max(bm-cur.node_bit - 1, 0); - clear_bit(bit, bm-cur.node-data); -} - static int memory_bm_test_bit(struct memory_bitmap *bm, unsigned long pfn) { void *addr; @@ -1342,36 +1334,21 @@ static struct memory_bitmap copy_bm; void swsusp_free(void) { - unsigned long fb_pfn, fr_pfn; + unsigned long pfn; if (!forbidden_pages_map || !free_pages_map) goto out; memory_bm_position_reset(forbidden_pages_map); - memory_bm_position_reset(free_pages_map); - -loop: - fr_pfn = memory_bm_next_pfn(free_pages_map); - fb_pfn = memory_bm_next_pfn(forbidden_pages_map); - - /* - * Find the next bit set in both bitmaps. This is guaranteed to - * terminate when fb_pfn == fr_pfn == BM_END_OF_MAP. - */ - do { - if (fb_pfn fr_pfn) - fb_pfn = memory_bm_next_pfn(forbidden_pages_map); - if (fr_pfn fb_pfn) - fr_pfn = memory_bm_next_pfn(free_pages_map); - } while (fb_pfn != fr_pfn); - - if (fr_pfn != BM_END_OF_MAP pfn_valid(fr_pfn)) { - struct page *page = pfn_to_page(fr_pfn); - - memory_bm_clear_current(forbidden_pages_map); - memory_bm_clear_current(free_pages_map); - __free_page(page); - goto loop; + for ( ; ; ) { + pfn = memory_bm_next_pfn(forbidden_pages_map); + if (BM_END_OF_MAP == pfn) + break; + if (memory_bm_test_bit(free_pages_map, pfn)) { So why exactly isn't it necessary to look at memory_bm_next_pfn(forbidden_pages_map)? + memory_bm_clear_bit(forbidden_pages_map, pfn); + memory_bm_clear_bit(free_pages_map, pfn); + __free_page(pfn_to_page(pfn)); + } } out: -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center.
회신: LZ4 : fix the data abort issue.
If the part of the compression data are corrupted, or the compression data is totally fake, the memory access over the limit is possible. This is the log from my system usning lz4 decompression. [6502]data abort, halting [6503]r0 0x r1 0x r2 0xdcea0ffc r3 0xdcea0ffc [6509]r4 0xb9ab0bfd r5 0xdcea0ffc r6 0xdcea0ff8 r7 0xdce8 [6515]r8 0x r9 0x r10 0x r11 0xb9a98000 [6522]r12 0xdcea1000 usp 0x ulr 0x pc 0x820149bc [6528]spsr 0x41f3 and the memory addresses of some variables at the moment are ref:0xdcea0ffc, op:0xdcea0ffc, oend:0xdcea1000 As you can see, COPYLENGH is 8bytes, so @ref and @op can access the momory over @oend. Signed-off-by: JeHyeon Yeon --- lib/lz4/lz4_decompress.c |3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/lz4/lz4_decompress.c b/lib/lz4/lz4_decompress.c index 7a85967..f0f5c5c 100644 --- a/lib/lz4/lz4_decompress.c +++ b/lib/lz4/lz4_decompress.c @@ -139,6 +139,9 @@ static int lz4_uncompress(const char *source, char *dest, int osize) /* Error: request to write beyond destination buffer */ if (cpy > oend) goto _output_error; + if ((ref + COPYLENGTH) > oend || + (op + COPYLENGTH) > oend) + goto _output_error; LZ4_SECURECOPY(ref, op, (oend - COPYLENGTH)); while (op < cpy) *op++ = *ref++; -- 1.7.9.5 Dear greg k-h I usually use my English name as tom. But my real name is not tom as you told me but JeHyeon Yeon. So, I changed my signed-off from tom.yeon to JeHyeon Yeon. Thank you. 보낸 사람: gre...@linuxfoundation.org [gre...@linuxfoundation.org] 보낸 날짜: 2015년 3월 13일 금요일 오후 10:23 받는 사람: Yeon, JeHyeon (Tom) 참조: linux-kernel@vger.kernel.org 제목: Re: LZ4 : fix the data abort issue. On Thu, Mar 12, 2015 at 08:28:55AM +, Yeon, JeHyeon (Tom) wrote: > If the part of the compression data are corrupted, or the compression > data is totally fake, the memory access over the limit is possible. > > This is the log from my system usning lz4 decompression. >[6502]data abort, halting >[6503]r0 0x r1 0x r2 0xdcea0ffc r3 0xdcea0ffc >[6509]r4 0xb9ab0bfd r5 0xdcea0ffc r6 0xdcea0ff8 r7 0xdce8 >[6515]r8 0x r9 0x r10 0x r11 0xb9a98000 >[6522]r12 0xdcea1000 usp 0x ulr 0x pc 0x820149bc >[6528]spsr 0x41f3 > and the memory addresses of some variables at the moment are > ref:0xdcea0ffc, op:0xdcea0ffc, oend:0xdcea1000 > > As you can see, COPYLENGH is 8bytes, so @ref and @op can access the momory > over @oend. > > Signed-off-by: tom.yeon I need a "real" name here, I somehow doubt that your government documents has your name as "tom.yeon", right? Please fix this up and resend so that I can apply it. thanks, greg k-h N떑꿩�r툤y鉉싕b쾊Ф푤v�^�)頻{.n�+돴쪐{콗喩zX㎍썳變}찠꼿쟺�:+v돣�쳭喩zZ+�+zf"톒쉱�~넮녬i鎬z�췿ⅱ�?솳鈺�&�)刪f뷌^j푹y쬶끷@A첺뛴 0띠h��뭝
회신: LZ4 : fix the data abort issue.
If the part of the compression data are corrupted, or the compression data is totally fake, the memory access over the limit is possible. This is the log from my system usning lz4 decompression. [6502]data abort, halting [6503]r0 0x r1 0x r2 0xdcea0ffc r3 0xdcea0ffc [6509]r4 0xb9ab0bfd r5 0xdcea0ffc r6 0xdcea0ff8 r7 0xdce8 [6515]r8 0x r9 0x r10 0x r11 0xb9a98000 [6522]r12 0xdcea1000 usp 0x ulr 0x pc 0x820149bc [6528]spsr 0x41f3 and the memory addresses of some variables at the moment are ref:0xdcea0ffc, op:0xdcea0ffc, oend:0xdcea1000 As you can see, COPYLENGH is 8bytes, so @ref and @op can access the momory over @oend. Signed-off-by: JeHyeon Yeon tom.y...@windriver.com --- lib/lz4/lz4_decompress.c |3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/lz4/lz4_decompress.c b/lib/lz4/lz4_decompress.c index 7a85967..f0f5c5c 100644 --- a/lib/lz4/lz4_decompress.c +++ b/lib/lz4/lz4_decompress.c @@ -139,6 +139,9 @@ static int lz4_uncompress(const char *source, char *dest, int osize) /* Error: request to write beyond destination buffer */ if (cpy oend) goto _output_error; + if ((ref + COPYLENGTH) oend || + (op + COPYLENGTH) oend) + goto _output_error; LZ4_SECURECOPY(ref, op, (oend - COPYLENGTH)); while (op cpy) *op++ = *ref++; -- 1.7.9.5 Dear greg k-h I usually use my English name as tom. But my real name is not tom as you told me but JeHyeon Yeon. So, I changed my signed-off from tom.yeon to JeHyeon Yeon. Thank you. 보낸 사람: gre...@linuxfoundation.org [gre...@linuxfoundation.org] 보낸 날짜: 2015년 3월 13일 금요일 오후 10:23 받는 사람: Yeon, JeHyeon (Tom) 참조: linux-kernel@vger.kernel.org 제목: Re: LZ4 : fix the data abort issue. On Thu, Mar 12, 2015 at 08:28:55AM +, Yeon, JeHyeon (Tom) wrote: If the part of the compression data are corrupted, or the compression data is totally fake, the memory access over the limit is possible. This is the log from my system usning lz4 decompression. [6502]data abort, halting [6503]r0 0x r1 0x r2 0xdcea0ffc r3 0xdcea0ffc [6509]r4 0xb9ab0bfd r5 0xdcea0ffc r6 0xdcea0ff8 r7 0xdce8 [6515]r8 0x r9 0x r10 0x r11 0xb9a98000 [6522]r12 0xdcea1000 usp 0x ulr 0x pc 0x820149bc [6528]spsr 0x41f3 and the memory addresses of some variables at the moment are ref:0xdcea0ffc, op:0xdcea0ffc, oend:0xdcea1000 As you can see, COPYLENGH is 8bytes, so @ref and @op can access the momory over @oend. Signed-off-by: tom.yeon tom.y...@windriver.com I need a real name here, I somehow doubt that your government documents has your name as tom.yeon, right? Please fix this up and resend so that I can apply it. thanks, greg k-h N떑꿩�r툤y鉉싕b쾊Ф푤v�^�)頻{.n�+돴쪐{콗喩zX㎍썳變}찠꼿쟺�j:+v돣�쳭喩zZ+�+zf"톒쉱�~넮녬i鎬z�췿ⅱ�?솳鈺��)刪f뷌^j푹y쬶끷@A첺뛴 0띠h��뭝
Re: LZ4 : fix the data abort issue.
If the part of the compression data are corrupted, or the compression data is totally fake, the memory access over the limit is possible. This is the log from my system usning lz4 decompression. [6502]data abort, halting [6503]r0 0x r1 0x r2 0xdcea0ffc r3 0xdcea0ffc [6509]r4 0xb9ab0bfd r5 0xdcea0ffc r6 0xdcea0ff8 r7 0xdce8 [6515]r8 0x r9 0x r10 0x r11 0xb9a98000 [6522]r12 0xdcea1000 usp 0x ulr 0x pc 0x820149bc [6528]spsr 0x41f3 and the memory addresses of some variables at the moment are ref:0xdcea0ffc, op:0xdcea0ffc, oend:0xdcea1000 As you can see, COPYLENGH is 8bytes, so @ref and @op can access the momory over @oend. Signed-off-by: tom.yeon --- lib/lz4/lz4_decompress.c |3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/lz4/lz4_decompress.c b/lib/lz4/lz4_decompress.c index 7a85967..f0f5c5c 100644 --- a/lib/lz4/lz4_decompress.c +++ b/lib/lz4/lz4_decompress.c @@ -139,6 +139,9 @@ static int lz4_uncompress(const char *source, char *dest, int osize) /* Error: request to write beyond destination buffer */ if (cpy > oend) goto _output_error; + if ((ref + COPYLENGTH) > oend || + (op + COPYLENGTH) > oend) + goto _output_error; LZ4_SECURECOPY(ref, op, (oend - COPYLENGTH)); while (op < cpy) *op++ = *ref++; -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: LZ4 : fix the data abort issue.
If the part of the compression data are corrupted, or the compression data is totally fake, the memory access over the limit is possible. This is the log from my system usning lz4 decompression. [6502]data abort, halting [6503]r0 0x r1 0x r2 0xdcea0ffc r3 0xdcea0ffc [6509]r4 0xb9ab0bfd r5 0xdcea0ffc r6 0xdcea0ff8 r7 0xdce8 [6515]r8 0x r9 0x r10 0x r11 0xb9a98000 [6522]r12 0xdcea1000 usp 0x ulr 0x pc 0x820149bc [6528]spsr 0x41f3 and the memory addresses of some variables at the moment are ref:0xdcea0ffc, op:0xdcea0ffc, oend:0xdcea1000 As you can see, COPYLENGH is 8bytes, so @ref and @op can access the momory over @oend. Signed-off-by: tom.yeon tom.y...@windriver.com --- lib/lz4/lz4_decompress.c |3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/lz4/lz4_decompress.c b/lib/lz4/lz4_decompress.c index 7a85967..f0f5c5c 100644 --- a/lib/lz4/lz4_decompress.c +++ b/lib/lz4/lz4_decompress.c @@ -139,6 +139,9 @@ static int lz4_uncompress(const char *source, char *dest, int osize) /* Error: request to write beyond destination buffer */ if (cpy oend) goto _output_error; + if ((ref + COPYLENGTH) oend || + (op + COPYLENGTH) oend) + goto _output_error; LZ4_SECURECOPY(ref, op, (oend - COPYLENGTH)); while (op cpy) *op++ = *ref++; -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/