[PATCH] Bugfix: bgenv_get: Error codes overlap return values

[Andreas J. Reichel]

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

If bgenv_get is called with a NULL pointer as data destination, it
returns the size of the buffer that must be provided by the user.

However, if the function fails, it also returns an error code greater
than zero, that overlaps possible values for the buffer size, so that
the user cannot distinguish failed and successful states.

To solve this, error codes are mapped to their negative counterparts,
i.e. bgenv_get returns -errno instead of errno.

To be symmetric to bgenv_set, the same is changed there.

Signed-off-by: Andreas Reichel <andreas.r...@siemens.com>
---
env/env_api_fat.c | 24 ++++++++++++------------
env/uservars.c | 4 ++--
tools/tests/test_api.c | 8 ++++----
3 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/env/env_api_fat.c b/env/env_api_fat.c
index 0635024..851b70c 100644
--- a/env/env_api_fat.c
+++ b/env/env_api_fat.c
@@ -454,11 +454,11 @@ int bgenv_get(BGENV *env, char *key, char *type, void *data, uint32_t maxlen)
char buffer[ENV_STRING_LENGTH];

if (!key || maxlen == 0) {
- return EINVAL;
+ return -EINVAL;
}
e = bgenv_str2enum(key);
if (!env) {
- return EPERM;
+ return -EPERM;
}
if (e == EBGENV_UNKNOWN) {
if (!data) {
@@ -526,7 +526,7 @@ int bgenv_get(BGENV *env, char *key, char *type, void *data, uint32_t maxlen)
if (!data) {
return 0;
}
- return EINVAL;
+ return -EINVAL;
}
return 0;
}
@@ -539,12 +539,12 @@ int bgenv_set(BGENV *env, char *key, char *type, void *data, uint32_t datalen)
char *value = (char *)data;

if (!key || !data || datalen == 0) {
- return EINVAL;
+ return -EINVAL;
}

e = bgenv_str2enum(key);
if (!env) {
- return EPERM;
+ return -EPERM;
}
if (e == EBGENV_UNKNOWN) {
return bgenv_set_uservar(env->data->userdata, key, type, data,
@@ -555,10 +555,10 @@ int bgenv_set(BGENV *env, char *key, char *type, void *data, uint32_t datalen)
val = strtol(value, &p, 10);
if ((errno == ERANGE && (val == LONG_MAX || val == LONG_MIN)) ||
(errno != 0 && val == 0)) {
- return errno;
+ return -errno;
}
if (p == value) {
- return EINVAL;
+ return -EINVAL;
}
env->data->revision = val;
break;
@@ -572,10 +572,10 @@ int bgenv_set(BGENV *env, char *key, char *type, void *data, uint32_t datalen)
val = strtol(value, &p, 10);
if ((errno == ERANGE && (val == LONG_MAX || val == LONG_MIN)) ||
(errno != 0 && val == 0)) {
- return errno;
+ return -errno;
}
if (p == value) {
- return EINVAL;
+ return -EINVAL;
}
env->data->watchdog_timeout_sec = val;
break;
@@ -583,15 +583,15 @@ int bgenv_set(BGENV *env, char *key, char *type, void *data, uint32_t datalen)
val = strtol(value, &p, 10);
if ((errno == ERANGE && (val == LONG_MAX || val == LONG_MIN)) ||
(errno != 0 && val == 0)) {
- return errno;
+ return -errno;
}
if (p == value) {
- return EINVAL;
+ return -EINVAL;
}
env->data->ustate = val;
break;
default:
- return EINVAL;
+ return -EINVAL;
}
return 0;
}
diff --git a/env/uservars.c b/env/uservars.c
index abbcf54..cd3f24b 100644
--- a/env/uservars.c
+++ b/env/uservars.c
@@ -96,7 +96,7 @@ int bgenv_get_uservar(uint8_t *udata, char *key, char *type, void *data,
uservar = bgenv_find_uservar(udata, key);

if (!uservar) {
- return EINVAL;
+ return -EINVAL;
}

bgenv_map_uservar(uservar, &lkey, &ltype, &value, NULL, &dsize);
@@ -136,7 +136,7 @@ int bgenv_set_uservar(uint8_t *udata, char *key, char *type, void *data,
p = bgenv_uservar_alloc(udata, total_size);
}
if (!p) {
- return errno;
+ return -errno;
}

bgenv_serialize_uservar(p, key, type, data, total_size);
diff --git a/tools/tests/test_api.c b/tools/tests/test_api.c
index fc5267d..d6b12e2 100644
--- a/tools/tests/test_api.c
+++ b/tools/tests/test_api.c
@@ -104,7 +104,7 @@ static void test_api_accesscurrent(void **state)
assert_int_equal(ret, 0);

ret = ebg_env_set(&e, "kernelfile", "vmlinuz");
- assert_int_equal(ret, EPERM);
+ assert_int_equal(ret, -EPERM);

will_return(bgenv_open_latest, &env);
ret = ebg_env_open_current(&e);
@@ -112,7 +112,7 @@ static void test_api_accesscurrent(void **state)

assert_int_equal(ebg_env_set(&e, "kernelfile", "vmlinuz"), 0);
assert_int_equal(ebg_env_set(&e, "kernelparams", "root=/dev/sda"), 0);
- assert_int_equal(ebg_env_set(&e, "watchdog_timeout_sec", "abc"), EINVAL);
+ assert_int_equal(ebg_env_set(&e, "watchdog_timeout_sec", "abc"), -EINVAL);
assert_int_equal(ebg_env_set(&e, "watchdog_timeout_sec", "0013"), 0);
assert_int_equal(ebg_env_set(&e, "ustate", "1"), 0);

@@ -121,7 +121,7 @@ static void test_api_accesscurrent(void **state)
assert_int_equal(ret, 0);

ret = ebg_env_get(&e, "kernelfile", buffer);
- assert_int_equal(ret, EPERM);
+ assert_int_equal(ret, -EPERM);

will_return(bgenv_open_latest, &env);
ret = ebg_env_open_current(&e);
@@ -282,7 +282,7 @@ static void test_api_uservars(void **state)
ret = ebg_env_set_ex(&e, "A", "B", dummymem, space_left);
free(dummymem);

- assert_int_equal(ret, ENOMEM);
+ assert_int_equal(ret, -ENOMEM);

// test user data type
ret = ebg_env_set_ex(&e, "A", "B", "C", 2);
--
2.14.2

[Andreas J. Reichel]

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

If bgenv_get is called with a NULL pointer as data destination, it
returns the size of the buffer that must be provided by the user.

However, if the function fails, it also returns an error code greater
than zero, that overlaps possible values for the buffer size, so that
the user cannot distinguish failed and successful states.

To solve this, error codes are mapped to their negative counterparts,
i.e. bgenv_get returns -errno instead of errno.

To be symmetric to bgenv_set, the same is changed there.

Signed-off-by: Andreas Reichel <andreas.r...@siemens.com>
---
env/env_api_fat.c | 24 ++++++++++++------------
env/uservars.c | 4 ++--

include/ebgenv.h | 5 +++--
tools/tests/test_api.c | 8 ++++----
4 files changed, 21 insertions(+), 20 deletions(-)

diff --git a/include/ebgenv.h b/include/ebgenv.h
index 4343cef..bb5b3c5 100644
--- a/include/ebgenv.h
+++ b/include/ebgenv.h
@@ -58,7 +58,8 @@ int ebg_env_get(ebgenv_t *e, char *key, char* buffer);
* @param e A pointer to an ebgenv_t context.
* @param key name of the environment variable to set
* @param value a string to be stored into the variable
- * @return 0 on success, errno on failure
+ * @return 0 on success, -errno on failure. If buffer is NULL,
+ * the required buffer size is returned.
*/
int ebg_env_set(ebgenv_t *e, char *key, char *value);

@@ -68,7 +69,7 @@ int ebg_env_set(ebgenv_t *e, char *key, char *value);
* @param datatype user specific string to identify the datatype of the value
* @param value arbitrary data to be stored into the variable
* @param datalen length of the data to be stored into the variable
- * @return 0 on success, errno on failure
+ * @return 0 on success, -errno on failure
*/
int ebg_env_set_ex(ebgenv_t *e, char *key, char *datatype, uint8_t *value,
uint32_t datalen);

[Jan Kiszka]

Is that a regression of a previous patch that is still in next, thus
should be fold that, or an additional topic? Same for the other patch.

Jan

[Andreas Reichel]

To be on the same side: "A regression of a previous patch" means to me,
that a previous patch introduced an error in an existing code, which
worked before.

Then: No, this is not a regression. The same for the other patch.

Andreas

> Jan

--
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

[Andreas Reichel]

On Fri, Oct 20, 2017 at 10:55:01AM +0200, [ext] Andreas Reichel wrote:
> On Thu, Oct 19, 2017 at 06:20:17PM +0200, Jan Kiszka wrote:
> > On 2017-10-19 16:49, [ext] Andreas J. Reichel wrote:

Can we please apply this patch now? [ This is 2/3 ]

> --
> 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 efibootguard-d...@googlegroups.com.
> To post to this group, send email to efibootg...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/efibootguard-dev/20171020085501.GA17185%40iiotirae.
> For more options, visit https://groups.google.com/d/optout.

[Jan Kiszka]

On 2017-10-19 16:49, [ext] Andreas J. Reichel wrote:

Thanks, applied.

Jan

--
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Corporate Competence Center Embedded Linux