[PATCH 1/3] Bugfix: Don't map non-existing user variables

[Andreas J. Reichel]

From: Andreas Reichel <andreas.r...@siemens.com>

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 <andreas.r...@siemens.com>
---
env/env_api_fat.c | 3 +++
env/uservars.c | 4 ++++
2 files changed, 7 insertions(+)

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);
return size;
}
diff --git a/env/uservars.c b/env/uservars.c
index eff1cf8..e1e2334 100644
--- a/env/uservars.c
+++ b/env/uservars.c
@@ -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;
+ }
+
/* Get the key */
var_key = (char *)udata;
if (key) {
--
2.16.2

[Jan Kiszka]

Did you check that this does not paper over an issue of the caller (like
the one you fixes with the first hunk)?

Jan

[Andreas Reichel]

This is intended.
Both are usable API functions and they should be robust. That one is
called by the other does not change this.

> Jan
>
> > /* Get the key */
> > var_key = (char *)udata;
> > if (key) {
> >
>

--
Andreas Reichel
Dipl.-Phys. (Univ.)
Software Consultant

Andreas...@tngtech.com, +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

[Jan Kiszka]

If udata == NULL is a bug of the caller (rather than a valid mode of
operation), that bug should be reported rather than swallowed. If you
consider that like free(NULL), then it's a different thing, but this
function does not look like that at all. So, please do not drop errors
under the table.

Jan