[PATCH 3/5] pstore: handle zero-sized prz in series

[PATCH 3/5] pstore: handle zero-sized prz in series

[Mark Salyzyn]

ramoops_pstore_read fails to return the next in a prz
series after first zero-sized entry, not venturing to
the next non-zero entry.

Signed-off-by: Mark Salyzyn <salyzyn@android.com>
---
 fs/pstore/ram.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
index 34ed8f8..6150e54 100644
--- a/fs/pstore/ram.c
+++ b/fs/pstore/ram.c
@@ -164,6 +164,12 @@ static int ramoops_read_kmsg_hdr(char *buffer, struct timespec *time,
 	return header_length;
 }
 
+static bool prz_ok(struct persistent_ram_zone *prz)
+{
+	return !!prz && !!(persistent_ram_old_size(prz) +
+			   persistent_ram_ecc_string(prz, NULL, 0));
+}
+
 static ssize_t ramoops_pstore_read(u64 *id, enum pstore_type_id *type,
 				   int *count, struct timespec *time,
 				   char **buf, bool *compressed,
@@ -178,13 +184,13 @@ static ssize_t ramoops_pstore_read(u64 *id, enum pstore_type_id *type,
 	prz = ramoops_get_next_prz(cxt->przs, &cxt->dump_read_cnt,
 				   cxt->max_dump_cnt, id, type,
 				   PSTORE_TYPE_DMESG, 1);
-	if (!prz)
+	if (!prz_ok(prz))
 		prz = ramoops_get_next_prz(&cxt->cprz, &cxt->console_read_cnt,
 					   1, id, type, PSTORE_TYPE_CONSOLE, 0);
-	if (!prz)
+	if (!prz_ok(prz))
 		prz = ramoops_get_next_prz(&cxt->fprz, &cxt->ftrace_read_cnt,
 					   1, id, type, PSTORE_TYPE_FTRACE, 0);
-	if (!prz)
+	if (!prz_ok(prz))
 		return 0;
 
 	if (!persistent_ram_old(prz))
-- 
2.2.0.rc0.207.ga3a616c

Re: [PATCH 3/5] pstore: handle zero-sized prz in series

[Kees Cook]

On Tue, Jan 13, 2015 at 2:32 PM, Mark Salyzyn <salyzyn@android.com> wrote:
> ramoops_pstore_read fails to return the next in a prz
> series after first zero-sized entry, not venturing to
> the next non-zero entry.
>
> Signed-off-by: Mark Salyzyn <salyzyn@android.com>

This seems fine. Out of curiosity, when was the bad behavior this encountered?

Acked-by: Kees Cook <keescook@chromium.org>

Thanks!

-Kees

> ---
>  fs/pstore/ram.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
> index 34ed8f8..6150e54 100644
> --- a/fs/pstore/ram.c
> +++ b/fs/pstore/ram.c
> @@ -164,6 +164,12 @@ static int ramoops_read_kmsg_hdr(char *buffer, struct timespec *time,
>         return header_length;
>  }
>
> +static bool prz_ok(struct persistent_ram_zone *prz)
> +{
> +       return !!prz && !!(persistent_ram_old_size(prz) +
> +                          persistent_ram_ecc_string(prz, NULL, 0));
> +}
> +
>  static ssize_t ramoops_pstore_read(u64 *id, enum pstore_type_id *type,
>                                    int *count, struct timespec *time,
>                                    char **buf, bool *compressed,
> @@ -178,13 +184,13 @@ static ssize_t ramoops_pstore_read(u64 *id, enum pstore_type_id *type,
>         prz = ramoops_get_next_prz(cxt->przs, &cxt->dump_read_cnt,
>                                    cxt->max_dump_cnt, id, type,
>                                    PSTORE_TYPE_DMESG, 1);
> -       if (!prz)
> +       if (!prz_ok(prz))
>                 prz = ramoops_get_next_prz(&cxt->cprz, &cxt->console_read_cnt,
>                                            1, id, type, PSTORE_TYPE_CONSOLE, 0);
> -       if (!prz)
> +       if (!prz_ok(prz))
>                 prz = ramoops_get_next_prz(&cxt->fprz, &cxt->ftrace_read_cnt,
>                                            1, id, type, PSTORE_TYPE_FTRACE, 0);
> -       if (!prz)
> +       if (!prz_ok(prz))
>                 return 0;
>
>         if (!persistent_ram_old(prz))
> --
> 2.2.0.rc0.207.ga3a616c
>



-- 
Kees Cook
Chrome OS Security

Re: [PATCH 3/5] pstore: handle zero-sized prz in series

[Mark Salyzyn]

On 01/13/2015 03:08 PM, Kees Cook wrote:
> On Tue, Jan 13, 2015 at 2:32 PM, Mark Salyzyn <salyzyn@android.com> wrote:
>> ramoops_pstore_read fails to return the next in a prz
>> series after first zero-sized entry, not venturing to
>> the next non-zero entry.
>>
>> Signed-off-by: Mark Salyzyn <salyzyn@android.com>
> This seems fine. Out of curiosity, when was the bad behavior this encountered?
During integration testing of the pmsg interface, we found that if any 
(most notably ftrace) of the previous prz's were zero in length. Before 
the addition of pmsg it would have occurred if you zero'd the console 
size, but had ftrace set; an unlikely scenario _until_ pmsg was added.
>
> Acked-by: Kees Cook <keescook@chromium.org>
>
> Thanks!
>
> -Kees
>
Thanks