[Skiboot] [PATCH v3 3/3] secvar: Fix signedness issues when validating ESL

[Nick Child]

On Mon, Jul 12, 2021 at 9:07 PM Nayna <nayna at linux.vnet.ibm.com> wrote:

> On 7/1/21 3:12 PM, Nick Child wrote:
> > -static EFI_SIGNATURE_LIST* get_esl_signature_list(const char *buf, size_t buflen)
> > +/*
> > + * Returns success or error number if buffer is not a valid ESL
> > + * If success, `list` will point to ESL struct which is a `buf` internal pointer
> > + * If success, `list_size` will be assigned the size of the ESL header + its data
> > + * `list` can be NULL if only size of ESL is requested
> > + */
> > +static int get_esl_signature_list(const char *buf, size_t buflen, EFI_SIGNATURE_LIST** list, uint32_t *list_size)
> >   {
>
> get_auth_descriptor2() / get_esl_cert() returns either size if valid or
> negative error. Can we follow similar format ?

Hi Nayna. I agree we should follow similar format everywhere in the
secvar functions, but maybe we should revisit our approach. I am not
saying that my approach is better but we should have some discussions
and, if we decide we should change the function code style, send in a
new patchset that addresses all instances of the issue. So, I will revoke
this patch.

> I think Nick mentioned that problem is in verify_signature() it once
> calls only to get the size.
>
> We can probably simplify this by passing list to get_esl_cert() which is
> for now anyway calling get_esl_signature_list() internally.
>

Agreed.
Thanks for a helpful review Nayna.
Since this patch is causing some congestion, I will remove it and we can return
to it during a more holistic approach later on.

Regards,
Nick Child