* [PATCH 1/2] pstore: fix buffer overflow while write offset equal to buffer size
@ 2014-03-12 13:24 Liu Shuo
2014-03-12 13:34 ` [PATCH 2/2] pstore: fix memory leak when decompress using big_oops_buf Liu Shuo
2014-03-12 16:50 ` [PATCH 1/2] pstore: fix buffer overflow while write offset equal to buffer size Kees Cook
0 siblings, 2 replies; 6+ messages in thread
From: Liu Shuo @ 2014-03-12 13:24 UTC (permalink / raw)
To: LKML; +Cc: Kees Cook, Anton Vorontsov, Colin Cross, Tony Luck, Zhang Yanmin
From: Liu ShuoX <shuox.liu@intel.com>
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 <shuox.liu@intel.com>
---
fs/pstore/ram_core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/pstore/ram_core.c b/fs/pstore/ram_core.c
index de272d4..ff7e3d4 100644
--- a/fs/pstore/ram_core.c
+++ b/fs/pstore/ram_core.c
@@ -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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] pstore: fix memory leak when decompress using big_oops_buf
2014-03-12 13:24 [PATCH 1/2] pstore: fix buffer overflow while write offset equal to buffer size Liu Shuo
@ 2014-03-12 13:34 ` Liu Shuo
2014-03-13 3:48 ` Kees Cook
2014-03-12 16:50 ` [PATCH 1/2] pstore: fix buffer overflow while write offset equal to buffer size Kees Cook
1 sibling, 1 reply; 6+ messages in thread
From: Liu Shuo @ 2014-03-12 13:34 UTC (permalink / raw)
To: LKML; +Cc: Kees Cook, Anton Vorontsov, Colin Cross, Tony Luck, Zhang Yanmin
From: Liu ShuoX <shuox.liu@intel.com>
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 <shuox.liu@intel.com>
---
fs/pstore/platform.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c
index 78c3c20..46d269e 100644
--- a/fs/pstore/platform.c
+++ b/fs/pstore/platform.c
@@ -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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] pstore: fix buffer overflow while write offset equal to buffer size
2014-03-12 13:24 [PATCH 1/2] pstore: fix buffer overflow while write offset equal to buffer size Liu Shuo
2014-03-12 13:34 ` [PATCH 2/2] pstore: fix memory leak when decompress using big_oops_buf Liu Shuo
@ 2014-03-12 16:50 ` Kees Cook
2014-03-13 2:18 ` Shuo Liu
1 sibling, 1 reply; 6+ messages in thread
From: Kees Cook @ 2014-03-12 16:50 UTC (permalink / raw)
To: Liu Shuo; +Cc: LKML, Anton Vorontsov, Colin Cross, Tony Luck, Zhang Yanmin
On Wed, Mar 12, 2014 at 6:24 AM, Liu Shuo <shuox.liu@gmail.com> wrote:
> From: Liu ShuoX <shuox.liu@intel.com>
>
> 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 <shuox.liu@intel.com>
This seems correct; good catch. Have you seen this problem happen, or
is this just from reading the code?
Acked-by: Kees Cook <keescook@chromium.org>
-Kees
> ---
> fs/pstore/ram_core.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fs/pstore/ram_core.c b/fs/pstore/ram_core.c
> index de272d4..ff7e3d4 100644
> --- a/fs/pstore/ram_core.c
> +++ b/fs/pstore/ram_core.c
> @@ -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
>
>
>
--
Kees Cook
Chrome OS Security
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] pstore: fix buffer overflow while write offset equal to buffer size
2014-03-12 16:50 ` [PATCH 1/2] pstore: fix buffer overflow while write offset equal to buffer size Kees Cook
@ 2014-03-13 2:18 ` Shuo Liu
0 siblings, 0 replies; 6+ messages in thread
From: Shuo Liu @ 2014-03-13 2:18 UTC (permalink / raw)
To: Kees Cook
Cc: LKML, Anton Vorontsov, Colin Cross, Tony Luck, Zhang Yanmin,
Liu, ShuoX
2014-03-13 0:50 GMT+08:00 Kees Cook <keescook@chromium.org>:
> On Wed, Mar 12, 2014 at 6:24 AM, Liu Shuo <shuox.liu@gmail.com> wrote:
>> From: Liu ShuoX <shuox.liu@intel.com>
>>
>> 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 <shuox.liu@intel.com>
>
> 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.
>
> Acked-by: Kees Cook <keescook@chromium.org>
>
> -Kees
>
>> ---
>> fs/pstore/ram_core.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/pstore/ram_core.c b/fs/pstore/ram_core.c
>> index de272d4..ff7e3d4 100644
>> --- a/fs/pstore/ram_core.c
>> +++ b/fs/pstore/ram_core.c
>> @@ -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
>>
>>
>>
>
>
>
> --
> Kees Cook
> Chrome OS Security
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] pstore: fix memory leak when decompress using big_oops_buf
2014-03-12 13:34 ` [PATCH 2/2] pstore: fix memory leak when decompress using big_oops_buf Liu Shuo
@ 2014-03-13 3:48 ` Kees Cook
2014-03-17 21:45 ` Tony Luck
0 siblings, 1 reply; 6+ messages in thread
From: Kees Cook @ 2014-03-13 3:48 UTC (permalink / raw)
To: Liu Shuo; +Cc: LKML, Anton Vorontsov, Colin Cross, Tony Luck, Zhang Yanmin
On Wed, Mar 12, 2014 at 6:34 AM, Liu Shuo <shuox.liu@gmail.com> wrote:
> From: Liu ShuoX <shuox.liu@intel.com>
>
> 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 <shuox.liu@intel.com>
Thanks again!
Acked-by: Kees Cook <keescook@chromium.org>
-Kees
> ---
> fs/pstore/platform.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c
> index 78c3c20..46d269e 100644
> --- a/fs/pstore/platform.c
> +++ b/fs/pstore/platform.c
> @@ -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
>
--
Kees Cook
Chrome OS Security
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] pstore: fix memory leak when decompress using big_oops_buf
2014-03-13 3:48 ` Kees Cook
@ 2014-03-17 21:45 ` Tony Luck
0 siblings, 0 replies; 6+ messages in thread
From: Tony Luck @ 2014-03-17 21:45 UTC (permalink / raw)
To: Kees Cook; +Cc: Liu Shuo, LKML, Anton Vorontsov, Colin Cross, Zhang Yanmin
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-03-17 21:45 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-03-12 13:24 [PATCH 1/2] pstore: fix buffer overflow while write offset equal to buffer size Liu Shuo
2014-03-12 13:34 ` [PATCH 2/2] pstore: fix memory leak when decompress using big_oops_buf Liu Shuo
2014-03-13 3:48 ` Kees Cook
2014-03-17 21:45 ` Tony Luck
2014-03-12 16:50 ` [PATCH 1/2] pstore: fix buffer overflow while write offset equal to buffer size Kees Cook
2014-03-13 2:18 ` Shuo Liu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox