pstore: fix buffer overflow while write offset equal to buffer size
PATCH 1/2 pstore: fix buffer overflow while write offset equal to buffer size
From: Liu ShuoX <[email protected]>
In case new offset is equal to prz->buffer_size, it won't wrap at this time and will return old(overflow) value next time.
---fs/pstore/ram_core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
fs/pstore/ram_core.c +0 -0
@@ -54,7 +54,7 @@ static size_t buffer_start_add_atomic(struct persistent_ram_zone *prz, size_t a)
do {
old = atomic_read(&prz->buffer->start);
new = old + a;
- while (unlikely(new > prz->buffer_size))
+ while (unlikely(new >= prz->buffer_size))
new -= prz->buffer_size;
} while (atomic_cmpxchg(&prz->buffer->start, old, new) != old);@@ -91,7 +91,7 @@ static size_t buffer_start_add_locked(struct persistent_ram_zone *prz, size_t a)
old = atomic_read(&prz->buffer->start); new = old + a; - while (unlikely(new > prz->buffer_size)) + while (unlikely(new >= prz->buffer_size)) new -= prz->buffer_size; atomic_set(&prz->buffer->start, new);
Signature · 1 line
1.8.3.2
PATCH 2/2 pstore: fix memory leak when decompress using big_oops_buf
From: Liu ShuoX <[email protected]>
After sucessful decompressing, the buffer which pointed by 'buf' will be lost as 'buf' is overwrite by 'big_oops_buf' and will never be freed.
---fs/pstore/platform.c | 1 +
1 file changed, 1 insertion(+)
fs/pstore/platform.c +0 -0
@@ -497,6 +497,7 @@ void pstore_get_records(int quiet)
big_oops_buf_sz);
if (unzipped_len > 0) {
+ kfree(buf);
buf = big_oops_buf;
size = unzipped_len;
compressed = false;Signature · 1 line
1.8.3.2
Re: [PATCH 2/2] pstore: fix memory leak when decompress using big_oops_buf
Quoted 5 lines · Liu Shuo
From: Liu ShuoX <[email protected]>
After sucessful decompressing, the buffer which pointed by 'buf' will be lost as 'buf' is overwrite by '
big_oops_buf' and will never be freed.Signed-off-by Liu ShuoX <[email protected]>
Thanks again!
-Kees
Quoted 19 lines
---
fs/pstore/platform.c| 1 +1 file changed, 1 insertion(+)fs/pstore/platform.c +1
-1@@ -497,6 +497,7 @@ void pstore_get_records(int quiet) big_oops_buf_sz);if (unzipped_len >0 ) { +kfree (buf); buf = big_oops_buf; size = unzipped_len; compressed = false; --1.8.3.2
Signature · 2 lines
Kees Cook Chrome OS Security
Re: [PATCH 2/2] pstore: fix memory leak when decompress using big_oops_buf
Ok - applied the original two that Andrew had allready taken into -mm, plus these four to my "next" branch. So I have these queued:
Liu ShuoX (6):
pstore: clarify clearing of _read_cnt in ramoops_context
pstore: skip zero size persistent ram buffer in traverse
pstore: Fix NULL pointer fault if get NULL prz in ramoops_get_next_prz
pstore: Correct the max_dump_cnt clearing of ramoops
pstore: Fix buffer overflow while write offset equal to buffer size
pstore: Fix memory leak when decompress using big_oops_buf
Thanks
-Tony
Quoted 6 lines · Liu Shuo
From: Liu ShuoX <[email protected]>
In case new offset is equal to
prz->buffer_size, it won't wrap at this time and will return old(overflow) value next time.Signed-off-by Liu ShuoX <[email protected]>
This seems correct; good catch. Have you seen this problem happen, or is this just from reading the code?
-Kees
Quoted 33 lines
---
fs/pstore/ram_core.c| 4 ++--1 file changed, 2 insertions(+), 2 deletions(-)fs/pstore/ram_core.c +2
-3@@ -54,7 +54,7 @@ static size_t buffer_start_add_atomic(struct persistent_ram_zone *prz, size_t a) do { old =atomic_read (&prz->buffer->start); new = old + a; -while (unlikely (new > prz->buffer_size)) +while (unlikely (new >= prz->buffer_size)) new -= prz->buffer_size; }while (atomic_cmpxchg (&prz->buffer->start, old, new) != old);@@ -91,7 +91,7 @@ static size_t buffer_start_add_locked(struct persistent_ram_zone *prz, size_t a) old =atomic_read (&prz->buffer->start); new = old + a; -while (unlikely (new > prz->buffer_size)) +while (unlikely (new >= prz->buffer_size)) new -= prz->buffer_size;atomic_set (&prz->buffer->start, new); --1.8.3.2
Signature · 2 lines
Kees Cook Chrome OS Security
2014-03-13 0:50 GMT+08:00 Kees Cook <[email protected]>:
Quoted 10 lines · Liu Shuo
Quoted 6 lines
From: Liu ShuoX <[email protected]>
In case new offset is equal to
prz->buffer_size, it won't wrap at this time and will return old(overflow) value next time.Signed-off-by Liu ShuoX <[email protected]>This seems correct; good catch. Have you seen this problem happen, or is this just from reading the code?
Thanks.
We indeed hit it when we enhanced the ramoops tracing.
Quoted 44 lines
Acked-by Kees Cook <[email protected]>-Kees
Quoted 29 lines
---
fs/pstore/ram_core.c| 4 ++--1 file changed, 2 insertions(+), 2 deletions(-)fs/pstore/ram_core.c +2
-3@@ -54,7 +54,7 @@ static size_t buffer_start_add_atomic(struct persistent_ram_zone *prz, size_t a) do { old =atomic_read (&prz->buffer->start); new = old + a; -while (unlikely (new > prz->buffer_size)) +while (unlikely (new >= prz->buffer_size)) new -= prz->buffer_size; }while (atomic_cmpxchg (&prz->buffer->start, old, new) != old);@@ -91,7 +91,7 @@ static size_t buffer_start_add_locked(struct persistent_ram_zone *prz, size_t a) old =atomic_read (&prz->buffer->start); new = old + a; -while (unlikely (new > prz->buffer_size)) +while (unlikely (new >= prz->buffer_size)) new -= prz->buffer_size;atomic_set (&prz->buffer->start, new); --Quoted 4 lines
1.8.3.2
--
Kees Cook
Chrome OS Security