[Skiboot] [PATCH] nvram: Fix a possible NULL pointer de-ref in nvram_query_eq()

[Oliver]

On Sat, Sep 8, 2018 at 4:51 PM, Vaibhav Jain <vaibhav at linux.ibm.com> wrote:
> A fault will occur if 'value == NULL' is passed to nvram_query_eq() to
> check if a given key doesn't exists in nvram partition. This patch
> fixes this issue by ensuring that a NULL value for argument 'value'
> never reaches the call to strcmp().

This isn't a valid use of the API. nvram_query_eq() is just a helper
for the common case where there's only one valid value for that key
(e.g. fast-reset=0). If you're doing anything more complex you should
be using nvram_query() to retrieve the value (which is NULL if the key
doesn't exist) and checking each case manually.

> Signed-off-by: Vaibhav Jain <vaibhav at linux.ibm.com>
> ---
>  core/nvram-format.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/core/nvram-format.c b/core/nvram-format.c
> index 42c5cbbb..c52b443a 100644
> --- a/core/nvram-format.c
> +++ b/core/nvram-format.c
> @@ -282,8 +282,8 @@ bool nvram_query_eq(const char *key, const char *value)
>  {
>         const char *s = nvram_query(key);
>
> -       if (!s)
> -               return false;
> +       if (s == NULL || value == NULL)
> +               return s == value;

Put an assert(value) before the strcmp and leave it at that. Maybe add
comment about the intended usage of nvram_query_eq()

>
>         return !strcmp(s, value);
>  }
> --
> 2.17.1

Also, just FYI Cyril isn't working at IBM any more.