On Wed, May 02, 2018 at 01:20:24PM +0200, Jan Kiszka wrote:
> On 2018-04-27 14:44, Andreas J. Reichel wrote:
> > From: Andreas Reichel <[email protected]>
> >
> > If a non-existing user variable is requested and a NULL-buffer is
> > provided, the getter maps an invalid memory area to retrieve the needed
> > buffer size.
> > Fix this by checking if the data pointer is valid first before mapping
> > anything and furthermore don't call the mapper if the variable is not
> > found.
> >
> > Signed-off-by: Andreas Reichel <[email protected]>
> > ---
> > env/env_api_fat.c | 3 +++
> > env/uservars.c | 16 +++++++++++++---
> > include/uservars.h | 6 +++---
> > 3 files changed, 19 insertions(+), 6 deletions(-)
> >
> > diff --git a/env/env_api_fat.c b/env/env_api_fat.c
> > index 1705cb9..a86c05d 100644
> > --- a/env/env_api_fat.c
> > +++ b/env/env_api_fat.c
> > @@ -287,6 +287,9 @@ int bgenv_get(BGENV *env, char *key, uint64_t *type,
> > void *data,
> > uint8_t *u;
> > uint32_t size;
> > u = bgenv_find_uservar(env->data->userdata, key);
> > + if (!u) {
> > + return -EINVAL;
> > + }
> > bgenv_map_uservar(u, NULL, NULL, NULL, NULL, &size);
>
> Here, you do not check the new return code. OK, u can't be NULL anymore,
> but then it has to be clear (comment) that we do not expect
> bgenv_map_uservar to fail otherwise.
>
I simplified according to your suggestion that the caller has to check
and bgenv_map_uservar is type void.
> > return size;
> > }
> > diff --git a/env/uservars.c b/env/uservars.c
> > index eff1cf8..3ad95c3 100644
> > --- a/env/uservars.c
> > +++ b/env/uservars.c
> > @@ -14,8 +14,8 @@
> > #include "env_api.h"
> > #include "uservars.h"
> >
> > -void bgenv_map_uservar(uint8_t *udata, char **key, uint64_t *type, uint8_t
> > **val,
> > - uint32_t *record_size, uint32_t *data_size)
> > +int bgenv_map_uservar(uint8_t *udata, char **key, uint64_t *type, uint8_t
> > **val,
> > + uint32_t *record_size, uint32_t *data_size)
> > {
> > /* Each user variable is encoded as follows:
> > * |------------|--------------|---------------|----------------|
> > @@ -38,6 +38,10 @@ void bgenv_map_uservar(uint8_t *udata, char **key,
> > uint64_t *type, uint8_t **val
> > uint64_t *var_type;
> > uint8_t *data;
> >
> > + if (!udata) {
> > + return -EINVAL;
> > + }
> > +
>
> In the case above, this check is not needed. Why can't we make that
> consistent: either do the NULL check on caller side in all cases of let
> bgenv_map_uservar do it and evaluate its error in all cases?
>
caller now checks... see v3
> > /* Get the key */
> > var_key = (char *)udata;
> > if (key) {
> > @@ -68,6 +72,8 @@ void bgenv_map_uservar(uint8_t *udata, char **key,
> > uint64_t *type, uint8_t **val
> > if (val) {
> > *val = data;
> > }
> > +
> > + return 0;
> > }
> >
> > void bgenv_serialize_uservar(uint8_t *p, char *key, uint64_t type, void
> > *data,
> > @@ -100,6 +106,7 @@ int bgenv_get_uservar(uint8_t *udata, char *key,
> > uint64_t *type, void *data,
> > char *lkey;
> > uint32_t dsize;
> > uint64_t ltype;
> > + int res;
> >
> > uservar = bgenv_find_uservar(udata, key);
> >
> > @@ -107,7 +114,10 @@ int bgenv_get_uservar(uint8_t *udata, char *key,
> > uint64_t *type, void *data,
> > return -EINVAL;
> > }
> >
> > - bgenv_map_uservar(uservar, &lkey, <ype, &value, NULL, &dsize);
> > + res = bgenv_map_uservar(uservar, &lkey, <ype, &value, NULL, &dsize);
> > + if (res) {
> > + return res;
> > + }
> >
> > if (dsize > maxlen) {
> > dsize = maxlen;
> > diff --git a/include/uservars.h b/include/uservars.h
> > index b1d1d7c..012240c 100644
> > --- a/include/uservars.h
> > +++ b/include/uservars.h
> > @@ -15,9 +15,9 @@
> >
> > #include <stdint.h>
> >
> > -void bgenv_map_uservar(uint8_t *udata, char **key, uint64_t *type,
> > - uint8_t **val, uint32_t *record_size,
> > - uint32_t *data_size);
> > +int bgenv_map_uservar(uint8_t *udata, char **key, uint64_t *type,
> > + uint8_t **val, uint32_t *record_size,
> > + uint32_t *data_size);
> > void bgenv_serialize_uservar(uint8_t *p, char *key, uint64_t type, void
> > *data,
> > uint32_t record_size);
> >
> >
>
> Jan
>
> --
> Siemens AG, Corporate Technology, CT RDA IOT SES-DE
> Corporate Competence Center Embedded Linux
--
Andreas Reichel
Dipl.-Phys. (Univ.)
Software Consultant
[email protected], +49-174-3180074
TNG Technology Consulting GmbH, Betastr. 13a, 85774 Unterfoehring
Geschaeftsfuehrer: Henrik Klagges, Dr. Robert Dahlke, Gerhard Mueller
Sitz: Unterfoehring * Amtsgericht Muenchen * HRB 135082
--
You received this message because you are subscribed to the Google Groups "EFI
Boot Guard" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/efibootguard-dev/20180502114610.GC14318%40iiotirae.
For more options, visit https://groups.google.com/d/optout.
