[Skiboot] [PATCH 1/2] nvram: nvram_query do not run if no nvram was found

[Oliver]

On Mon, Mar 26, 2018 at 11:15 AM, Nicholas Piggin <npiggin at gmail.com> wrote:
> On Mon, 26 Mar 2018 10:50:31 +1100
> Oliver <oohall at gmail.com> wrote:
>
>> On Sun, Mar 25, 2018 at 11:48 AM, Nicholas Piggin <npiggin at gmail.com> wrote:
>> > If nvram_check fails, leaving skiboot_part_hdr == NULL, do not
>> > have nvram_query proceed using that pointer, just return NULL.
>>
>> Shouldn't the checking in nvram_validate() catch that case?
>
> It doesn't.

Bleh, we need to sort out the mess of global nvram flag variables at some point.

>> If not we
>> should probably fix it there.
>
> Yeah, good thinking. Looks like this turned out to be a deeper bug.
> What do you reckon about this?

Looks good.

Reviewed-by: Oliver O'Halloran <oohall at gmail.com>

>
> ---
>  core/nvram-format.c | 3 +++
>  core/nvram.c        | 8 +++++---
>  2 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/core/nvram-format.c b/core/nvram-format.c
> index 3d030a30..42c5cbbb 100644
> --- a/core/nvram-format.c
> +++ b/core/nvram-format.c
> @@ -180,6 +180,7 @@ int nvram_check(void *nvram_image, const uint32_t nvram_size)
>         }
>
>         prlog(PR_INFO, "NVRAM: Layout appears sane\n");
> +       assert(skiboot_part_hdr);
>         return 0;
>   failed:
>         return -1;
> @@ -234,6 +235,8 @@ const char *nvram_query(const char *key)
>         if (!nvram_validate())
>                 return NULL;
>
> +       assert(skiboot_part_hdr);
> +
>         part_end = (const char *) skiboot_part_hdr
>                 + be16_to_cpu(skiboot_part_hdr->len) * 16 - 1;
>
> diff --git a/core/nvram.c b/core/nvram.c
> index de6cbddf..1c3cdfaf 100644
> --- a/core/nvram.c
> +++ b/core/nvram.c
> @@ -69,8 +69,10 @@ opal_call(OPAL_WRITE_NVRAM, opal_write_nvram, 3);
>
>  bool nvram_validate(void)
>  {
> -       if (!nvram_valid)
> -               nvram_valid = !nvram_check(nvram_image, nvram_size);
> +       if (!nvram_valid) {
> +               if (!nvram_check(nvram_image, nvram_size))
> +                       nvram_valid = true;
> +       }
>
>         return nvram_valid;
>  }
> @@ -87,7 +89,7 @@ static void nvram_reformat(void)
>         if (platform.nvram_write)
>                 platform.nvram_write(0, nvram_image, nvram_size);
>
> -       nvram_valid = true;
> +       nvram_validate();
>  }
>
>  void nvram_reinit(void)
> --
> 2.16.1
>