[Skiboot] [PATCH 2/5] nvram: ibm,skiboot NUL terminator check

[Joel Stanley]

On Mon, Aug 8, 2016 at 12:28 PM, Oliver O'Halloran <oohall at gmail.com> wrote:
> NVRAM configuration strings are required to be NUL terminated and unused
> data bytes in the partition should be set to NUL. Badly behaved system
> software may not do this so same sanity checking is required. Ensuring
> that the final data byte in a partition is a NUL should be sufficent.

Spelling (sufficient).


>
> Signed-off-by: Oliver O'Halloran <oohall at gmail.com>
> ---
>  core/nvram-format.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/core/nvram-format.c b/core/nvram-format.c
> index a81663ceb35f..be8d77f8394b 100644
> --- a/core/nvram-format.c
> +++ b/core/nvram-format.c
> @@ -155,6 +155,20 @@ int nvram_check(void *nvram_image, const uint32_t nvram_size)
>                 prerror("NVRAM: Skiboot private partition "
>                         "not found !\n");
>                 goto failed;
> +       } else {
> +               /*
> +                * The OF NVRAM format requires config strings to be NUL
> +                * terminated and unused memory to be set ot zero. Well behaved

Typo

> +                * software should ensure this is done for us, but we should
> +                * always check.
> +                */
> +               const char *c = (const char *) skiboot_part_hdr +
> +                       skiboot_part_hdr->len * 16 - 1;

c is going to point to the last byte of the partition? We could call
it 'last_byte'.

Is ->len user controlled? Do we need to make sure it's not pointing
past the end of something?

> +
> +               if (*c != 0) {
> +                       prerror("NVRAM: Skiboot private partition is not NUL terminated");
> +                       goto failed;
> +               }
>         }
>
>         prlog(PR_INFO, "NVRAM: Layout appears sane\n");
> --
> 2.5.5
>
> _______________________________________________
> Skiboot mailing list
> Skiboot at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/skiboot