[PATCH] Fix type and security bugs

9 views
Skip to first unread message

Andreas J. Reichel

unread,
Sep 28, 2017, 9:17:37 AM9/28/17
to efibootg...@googlegroups.com, Andreas Reichel
From: Andreas Reichel <andreas.r...@siemens.com>

* Fix missing format strings to avoid exploits or stack corruption
* Fix warnings:
* missing type casts
* missing return values
* ignored return values
* possibly unitialized variables
* unused variables
* unintended integer overflows
* dereference of freed pointers
* Fix file offset size to 64 bits in Makefile

Signed-off-by: Andreas Reichel <andreas.r...@siemens.com>
---
Makefile.am | 3 ++-
env/env_api.c | 3 +--
env/env_api_fat.c | 16 +++++++--------
env/uservars.c | 6 +++---
tools/bg_setenv.c | 59 ++++++++++++++++++++++++++++++++-----------------------
tools/ebgpart.c | 55 ++++++++++++++++++++++++++++-----------------------
6 files changed, 78 insertions(+), 64 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index c7dcb10..6618083 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -27,7 +27,8 @@ AM_CFLAGS = \
-Wmissing-prototypes \
-fshort-wchar \
-DHAVE_ENDIAN_H \
- -D_GNU_SOURCE
+ -D_GNU_SOURCE \
+ -D_FILE_OFFSET_BITS=64

AM_LDFLAGS = -L$(top_builddir)/

diff --git a/env/env_api.c b/env/env_api.c
index b3377fc..f6d3327 100644
--- a/env/env_api.c
+++ b/env/env_api.c
@@ -137,7 +137,6 @@ uint16_t ebg_env_getglobalstate(ebgenv_t *e)
* with ustate == USTATE_FAILED */
if (env->data->revision == REVISION_FAILED &&
env->data->ustate == USTATE_FAILED) {
- (void)bgenv_close(env);
res = 3;
}
(void)bgenv_close(env);
@@ -166,7 +165,7 @@ int ebg_env_setglobalstate(ebgenv_t *e, uint16_t ustate)
if (ustate > USTATE_FAILED) {
return EINVAL;
}
- snprintf(buffer, sizeof(buffer), "%d", ustate);
+ (void)snprintf(buffer, sizeof(buffer), "%d", ustate);
res =
bgenv_set((BGENV *)e->bgenv, "ustate", "String", buffer,
strlen(buffer));
diff --git a/env/env_api_fat.c b/env/env_api_fat.c
index a16db61..354d7f8 100644
--- a/env/env_api_fat.c
+++ b/env/env_api_fat.c
@@ -78,7 +78,7 @@ bool mount_partition(CONFIG_PART *cfgpart)
{
char tmpdir_template[256];
char *mountpoint;
- snprintf(tmpdir_template, 256, "%s", tmp_mnt_dir);
+ (void)snprintf(tmpdir_template, 256, "%s", tmp_mnt_dir);
if (!cfgpart) {
return false;
}
@@ -204,7 +204,7 @@ bool probe_config_partitions(CONFIG_PART *cfgpart)

ped_device_probe_all();

- while (dev = ped_device_get_next(dev)) {
+ while ((dev = ped_device_get_next(dev))) {
printf_debug("Device: %s\n", dev->model);
PedDisk *pd = ped_disk_new(dev);
if (!pd) {
@@ -220,11 +220,11 @@ bool probe_config_partitions(CONFIG_PART *cfgpart)
continue;
}
if (strncmp("/dev/mmcblk", dev->path, 11) == 0) {
- snprintf(devpath, 4096, "%sp%u", dev->path,
- part->num);
+ (void)snprintf(devpath, 4096, "%sp%u",
+ dev->path, part->num);
} else {
- snprintf(devpath, 4096, "%s%u", dev->path,
- part->num);
+ (void)snprintf(devpath, 4096, "%s%u",
+ dev->path, part->num);
}
if (!cfgpart[count].devpath) {
cfgpart[count].devpath =
@@ -493,7 +493,7 @@ int bgenv_get(BGENV *env, char *key, char *type, void *data, uint32_t maxlen)
}
break;
case EBGENV_WATCHDOG_TIMEOUT_SEC:
- sprintf(buffer, "%lu", env->data->watchdog_timeout_sec);
+ sprintf(buffer, "%u", env->data->watchdog_timeout_sec);
if (!data) {
return strlen(buffer);
}
@@ -503,7 +503,7 @@ int bgenv_get(BGENV *env, char *key, char *type, void *data, uint32_t maxlen)
}
break;
case EBGENV_REVISION:
- sprintf(buffer, "%lu", env->data->revision);
+ sprintf(buffer, "%u", env->data->revision);
if (!data) {
return strlen(buffer);
}
diff --git a/env/uservars.c b/env/uservars.c
index ffdd604..3359ec6 100644
--- a/env/uservars.c
+++ b/env/uservars.c
@@ -69,7 +69,7 @@ void bgenv_serialize_uservar(uint8_t *p, char *key, char *type, void *data,
uint32_t payload_size, data_size;

/* store key */
- strncpy(p, key, strlen(key) + 1);
+ strncpy((char *)p, key, strlen(key) + 1);
p += strlen(key) + 1;

/* store payload_size after key */
@@ -180,8 +180,8 @@ uint8_t *bgenv_uservar_alloc(uint8_t *udata, uint32_t datalen)
return NULL;
}
spaceleft = bgenv_user_free(udata);
- VERBOSE(stdout, "uservar_alloc: free: %u requested: %u \n", spaceleft,
- datalen);
+ VERBOSE(stdout, "uservar_alloc: free: %lu requested: %lu \n",
+ (unsigned long)spaceleft, (unsigned long)datalen);

/* To find the end of user variables, a 2nd 0 must be there after the
* last variable content, thus, we need one extra byte if appending a
diff --git a/tools/bg_setenv.c b/tools/bg_setenv.c
index 6f7f8f9..6b554c4 100644
--- a/tools/bg_setenv.c
+++ b/tools/bg_setenv.c
@@ -94,6 +94,7 @@ static void journal_process_action(BGENV *env, struct env_action *action)
uint8_t *var;
ebgenv_t e;
uint16_t ustate;
+ unsigned long t;
char *arg, *tmp;
int ret;

@@ -105,22 +106,24 @@ static void journal_process_action(BGENV *env, struct env_action *action)
e.bgenv = env;
arg = (char *)action->data;
errno = 0;
- ustate = strtol(arg, &tmp, 10);
- if ((errno == ERANGE && (ustate == LONG_MAX ||
- ustate == LONG_MIN)) ||
- (errno != 0 && ustate == 0) || (tmp == arg)) {
+ t = strtol(arg, &tmp, 10);
+ if ((errno == ERANGE && (t == LONG_MAX ||
+ t == LONG_MIN)) ||
+ (errno != 0 && t == 0) || (tmp == arg)) {
fprintf(stderr, "Invalid value for ustate: %s",
(char *)action->data);
return;
}
- if (ret = ebg_env_setglobalstate(&e, ustate) != 0) {
- fprintf(stderr, "Error setting global state.",
+ ustate = (uint16_t)t;;
+ if ((ret = ebg_env_setglobalstate(&e, ustate)) != 0) {
+ fprintf(stderr,
+ "Error setting global state: %s.",
strerror(ret));
}
return;
}
bgenv_set(env, action->key, action->type, action->data,
- strlen(action->data) + 1);
+ strlen((char *)action->data) + 1);
break;
case ENV_TASK_DEL:
VERBOSE(stdout, "Task = DEL, key = %s\n", action->key);
@@ -162,9 +165,10 @@ static uint8_t str2ustate(char *str)

static char *ustate2str(uint8_t ustate)
{
- if (ustate >= USTATE_MIN && ustate <= USTATE_MAX) {
- return ustatemap[ustate];
+ if (ustate > USTATE_MAX) {
+ ustate = USTATE_MAX;
}
+ return ustatemap[USTATE_MAX];
}

static int set_uservars(char *arg)
@@ -182,8 +186,8 @@ static int set_uservars(char *arg)
return 0;
}

- journal_add_action(ENV_TASK_SET, key, USERVAR_TYPE_DEFAULT, value,
- strlen(value) + 1);
+ journal_add_action(ENV_TASK_SET, key, USERVAR_TYPE_DEFAULT,
+ (uint8_t *)value, strlen(value) + 1);

return 0;
}
@@ -192,7 +196,6 @@ static error_t parse_opt(int key, char *arg, struct argp_state *state)
{
struct arguments *arguments = state->input;
int i;
- wchar_t buffer[ENV_STRING_LENGTH];
char *tmp;

switch (key) {
@@ -204,8 +207,8 @@ static error_t parse_opt(int key, char *arg, struct argp_state *state)
ENV_STRING_LENGTH);
return 1;
}
- journal_add_action(ENV_TASK_SET, "kernelfile", "String", arg,
- strlen(arg) + 1);
+ journal_add_action(ENV_TASK_SET, "kernelfile", "String",
+ (uint8_t *)arg, strlen(arg) + 1);
break;
case 'a':
if (strlen(arg) > ENV_STRING_LENGTH) {
@@ -215,8 +218,8 @@ static error_t parse_opt(int key, char *arg, struct argp_state *state)
ENV_STRING_LENGTH);
return 1;
}
- journal_add_action(ENV_TASK_SET, "kernelparams", "String", arg,
- strlen(arg) + 1);
+ journal_add_action(ENV_TASK_SET, "kernelparams", "String",
+ (uint8_t *)arg, strlen(arg) + 1);
break;
case 'p':
i = strtol(arg, &tmp, 10);
@@ -258,7 +261,7 @@ static error_t parse_opt(int key, char *arg, struct argp_state *state)
} else {
asprintf(&tmp, "%u", i);
journal_add_action(ENV_TASK_SET, "ustate", "String",
- tmp, strlen(tmp) + 1);
+ (uint8_t *)tmp, strlen(tmp) + 1);
VERBOSE(stdout, "Ustate set to %u (%s).\n", i,
ustate2str(i));
}
@@ -266,8 +269,8 @@ static error_t parse_opt(int key, char *arg, struct argp_state *state)
case 'r':
i = atoi(arg);
VERBOSE(stdout, "Revision is set to %d.\n", i);
- journal_add_action(ENV_TASK_SET, "revision", "String", arg,
- strlen(arg) + 1);
+ journal_add_action(ENV_TASK_SET, "revision", "String",
+ (uint8_t *)arg, strlen(arg) + 1);
break;
case 'w':
i = atoi(arg);
@@ -275,7 +278,8 @@ static error_t parse_opt(int key, char *arg, struct argp_state *state)
VERBOSE(stdout,
"Setting watchdog timeout to %d seconds.\n", i);
journal_add_action(ENV_TASK_SET, "watchdog_timeout_sec",
- "String", arg, strlen(arg) + 1);
+ "String", (uint8_t *)arg,
+ strlen(arg) + 1);
} else {
fprintf(stderr, "Watchdog timeout must be non-zero.\n");
return 1;
@@ -289,7 +293,8 @@ static error_t parse_opt(int key, char *arg, struct argp_state *state)
VERBOSE(stdout,
"Confirming environment to work. Removing boot-once "
"and testing flag.\n");
- journal_add_action(ENV_TASK_SET, "ustate", "String", "0", 2);
+ journal_add_action(ENV_TASK_SET, "ustate", "String",
+ (uint8_t *)"0", 2);
break;
case 'u':
if (part_specified) {
@@ -367,11 +372,16 @@ static void update_environment(BGENV *env)
journal_process_action(env, action);
journal_free_action(action);
STAILQ_REMOVE(&head, action, env_action, journal);
- free(action);
}

env->data->crc32 = crc32(0, (const Bytef *)env->data,
sizeof(BG_ENVDATA) - sizeof(env->data->crc32));
+
+ while (!STAILQ_EMPTY(&head)) {
+ action = STAILQ_FIRST(&head);
+ STAILQ_REMOVE_HEAD(&head, journal);
+ free(action);
+ }
}

static void dump_envs(void)
@@ -418,7 +428,7 @@ int main(int argc, char **argv)
STAILQ_INIT(&head);

error_t e;
- if (e = argp_parse(argp, argc, argv, 0, 0, &arguments)) {
+ if ((e = argp_parse(argp, argc, argv, 0, 0, &arguments))) {
return e;
}

@@ -450,7 +460,7 @@ int main(int argc, char **argv)
}
if (fclose(of)) {
fprintf(stderr, "Error closing output file.\n");
- result = 1;
+ result = errno;
};
printf("Output written to %s.\n", envfilepath);
} else {
@@ -477,7 +487,6 @@ int main(int argc, char **argv)

BGENV *env_new;
BGENV *env_current;
- char *tmp;

if (auto_update) {
/* clone latest environment */
diff --git a/tools/ebgpart.c b/tools/ebgpart.c
index d0997bd..992099a 100644
--- a/tools/ebgpart.c
+++ b/tools/ebgpart.c
@@ -43,8 +43,9 @@ static void add_block_dev(PedDevice *dev)

static char *GUID_to_str(uint8_t *g)
{
- snprintf(buffer, 37, "%02X%02X%02X%02X-%02X%02X-%02X%02X-%02X%02X-%02X%"
- "02X%02X%02X%02X%02X",
+ (void)snprintf(buffer, 37,
+ "%02X%02X%02X%02X-%02X%02X-%02X%02X-%02X%02X-"
+ "%02X%02X%02X%02X%02X%02X",
g[3], g[2], g[1], g[0], g[5], g[4], g[7], g[6], g[8], g[9],
g[10], g[11], g[12], g[13], g[14], g[15]);
return buffer;
@@ -75,7 +76,7 @@ static bool check_GPT_FAT_entry(int fd, struct EFIpartitionentry *e,
if (strcmp(GPT_PARTITION_GUID_FAT_NTFS, GUID_to_str(e->type_GUID)) !=
0 &&
strcmp(GPT_PARTITION_GUID_ESP, GUID_to_str(e->type_GUID)) != 0) {
- asprintf(&pfst->name, "not supported");
+ (void)asprintf(&pfst->name, "%s", "not supported");
return true;
}
VERBOSE(stdout, "GPT Partition #%u is FAT/NTFS.\n", i);
@@ -87,7 +88,7 @@ static bool check_GPT_FAT_entry(int fd, struct EFIpartitionentry *e,
return false;
}
/* Look if it is a FAT12 or FAT16 */
- off64_t dest = e->start_LBA * LB_SIZE + 0x36;
+ off64_t dest = (off64_t)e->start_LBA * LB_SIZE + 0x36;
if (lseek64(fd, dest, SEEK_SET) == -1) {
VERBOSE(stderr, "Error seeking FAT12/16 Id String: %s\n",
strerror(errno));
@@ -103,7 +104,7 @@ static bool check_GPT_FAT_entry(int fd, struct EFIpartitionentry *e,
if (strcmp(FAT_id, "FAT12 ") != 0 &&
strcmp(FAT_id, "FAT16 ") != 0) {
/* No FAT12/16 so read ID field for FAT32 */
- dest = e->start_LBA * LB_SIZE + 0x52;
+ dest = (off64_t)e->start_LBA * LB_SIZE + 0x52;
if (lseek64(fd, dest, SEEK_SET) == -1) {
VERBOSE(stderr, "Error seeking FAT32 Id String: %s\n",
strerror(errno));
@@ -116,11 +117,11 @@ static bool check_GPT_FAT_entry(int fd, struct EFIpartitionentry *e,
}
}
if (strcmp(FAT_id, "FAT12 ") == 0) {
- asprintf(&pfst->name, "fat12");
+ (void)asprintf(&pfst->name, "%s", "fat12");
} else if (strcmp(FAT_id, "FAT16 ") == 0) {
- asprintf(&pfst->name, "fat16");
+ (void)asprintf(&pfst->name, "%s", "fat16");
} else {
- asprintf(&pfst->name, "fat32");
+ (void)asprintf(&pfst->name, "%s", "fat32");
}
VERBOSE(stdout, "GPT Partition #%u is %s.\n", i, pfst->name);
if (lseek64(fd, curr, SEEK_SET) == -1) {
@@ -191,19 +192,19 @@ static void scanLogicalVolumes(int fd, off64_t extended_start_LBA,
PedPartition *partition, int lognum)
{
struct Masterbootrecord next_ebr;
- PedFileSystemType *pfst;
+ PedFileSystemType *pfst = NULL;

off64_t offset = extended_start_LBA + ebr->parttable[i].start_LBA;
if (extended_start_LBA == 0) {
extended_start_LBA = offset;
}
- VERBOSE(stdout, "Seeking to LBA %ld\n", offset);
+ VERBOSE(stdout, "Seeking to LBA %llu\n", (unsigned long long)offset);
off64_t res = lseek64(fd, offset * LB_SIZE, SEEK_SET);
if (res == -1) {
VERBOSE(stderr, "(%s)\n", strerror(errno));
return;
}
- VERBOSE(stdout, "Seek returned %ld\n", res);
+ VERBOSE(stdout, "Seek returned %lld\n", (signed long long)res);
if (read(fd, &next_ebr, sizeof(next_ebr)) != sizeof(next_ebr)) {
VERBOSE(stderr, "Error reading next EBR (%s)\n",
strerror(errno));
@@ -233,7 +234,7 @@ static void scanLogicalVolumes(int fd, off64_t extended_start_LBA,
if (!pfst) {
goto scl_out_of_mem;
}
- if (asprintf(&pfst->name, type_to_name(t)) == -1) {
+ if (asprintf(&pfst->name, "%s", type_to_name(t)) == -1) {
goto scl_out_of_mem;
};
partition = partition->next;
@@ -270,6 +271,7 @@ static bool check_partition_table(PedDevice *dev)
}
int numpartitions = 0;
PedPartition **list_end = &dev->part_list;
+ PedPartition *tmp = NULL;
for (int i = 0; i < 4; i++) {
if (mbr.parttable[i].partition_type == 0) {
continue;
@@ -281,10 +283,12 @@ static bool check_partition_table(PedDevice *dev)
if (t == MBR_TYPE_GPT) {
VERBOSE(stdout, "GPT header at %X\n",
mbr.parttable[i].start_LBA);
- off64_t offset = LB_SIZE * mbr.parttable[i].start_LBA;
+ off64_t offset = LB_SIZE *
+ (off64_t)mbr.parttable[i].start_LBA;
if (lseek64(fd, offset, SEEK_SET) != offset) {
VERBOSE(stderr, "Error seeking EFI Header\n.");
VERBOSE(stderr, "(%s)", strerror(errno));
+ close(fd);
return false;
}
struct EFIHeader efihdr;
@@ -301,8 +305,8 @@ static bool check_partition_table(PedDevice *dev)
efihdr.signature[6], efihdr.signature[7]);
VERBOSE(stdout, "Number of partition entries: %u\n",
efihdr.partitions);
- VERBOSE(stdout, "Partition Table @ LBA %lu\n",
- efihdr.partitiontable_LBA);
+ VERBOSE(stdout, "Partition Table @ LBA %llu\n",
+ (unsigned long long)efihdr.partitiontable_LBA);
read_GPT_entries(fd, efihdr.partitiontable_LBA,
efihdr.partitions, dev);
break;
@@ -312,7 +316,7 @@ static bool check_partition_table(PedDevice *dev)
goto cpt_out_of_mem;
}

- PedPartition *tmp = calloc(sizeof(PedPartition), 1);
+ tmp = calloc(sizeof(PedPartition), 1);
if (!tmp) {
goto cpt_out_of_mem;
}
@@ -324,7 +328,7 @@ static bool check_partition_table(PedDevice *dev)
list_end = &((*list_end)->next);

if (t == MBR_TYPE_EXTENDED || t == MBR_TYPE_EXTENDED_LBA) {
- asprintf(&pfst->name, "extended");
+ (void)asprintf(&pfst->name, "%s", "extended");
scanLogicalVolumes(fd, 0, &mbr, i, tmp, 5);
/* Could be we still have MBR entries after
* logical volumes */
@@ -332,7 +336,7 @@ static bool check_partition_table(PedDevice *dev)
list_end = &((*list_end)->next);
}
} else {
- asprintf(&pfst->name, type_to_name(t));
+ (void)asprintf(&pfst->name, "%s", type_to_name(t));
}
continue;
cpt_out_of_mem:
@@ -363,7 +367,8 @@ static int scan_devdir(unsigned int fmajor, unsigned int fminor, char *fullname,
if (!devfile) {
break;
}
- snprintf(fullname, maxlen, "%s/%s", DEVDIR, devfile->d_name);
+ (void)snprintf(fullname, maxlen, "%s/%s", DEVDIR,
+ devfile->d_name);
struct stat fstat;
if (stat(fullname, &fstat) == -1) {
VERBOSE(stderr, "stat failed on %s\n", fullname);
@@ -389,7 +394,7 @@ static int get_major_minor(char *filename, unsigned int *major, unsigned int *mi
return -1;
}
int res = fscanf(fh, "%u:%u", major, minor);
- fclose(fh);
+ (void)fclose(fh);
if (res < 2) {
VERBOSE(stderr,
"Error reading major/minor of device entry. (%s)\n",
@@ -402,7 +407,7 @@ static int get_major_minor(char *filename, unsigned int *major, unsigned int *mi
void ped_device_probe_all(void)
{
struct dirent *sysblockfile;
- char fullname[DEV_FILENAME_LEN];
+ char fullname[DEV_FILENAME_LEN+16];

DIR *sysblockdir = opendir(SYSBLOCKDIR);
if (!sysblockdir) {
@@ -420,7 +425,7 @@ void ped_device_probe_all(void)
strcmp(sysblockfile->d_name, "..") == 0) {
continue;
}
- snprintf(fullname, sizeof(fullname), "/sys/block/%s/dev",
+ (void)snprintf(fullname, sizeof(fullname), "/sys/block/%s/dev",
sysblockfile->d_name);
/* Get major and minor revision from /sys/block/sdX/dev */
unsigned int fmajor, fminor;
@@ -431,7 +436,7 @@ void ped_device_probe_all(void)
"Trying device with: Major = %d, Minor = %d, (%s)\n",
fmajor, fminor, fullname);
/* Check if this file is really in the dev directory */
- snprintf(fullname, sizeof(fullname), "%s/%s", DEVDIR,
+ (void)snprintf(fullname, sizeof(fullname), "%s/%s", DEVDIR,
sysblockfile->d_name);
struct stat fstat;
if (stat(fullname, &fstat) == -1) {
@@ -444,8 +449,8 @@ void ped_device_probe_all(void)
}
/* This is a block device, so add it to the list*/
PedDevice *dev = calloc(sizeof(PedDevice), 1);
- asprintf(&dev->model, "N/A");
- asprintf(&dev->path, "%s", fullname);
+ (void)asprintf(&dev->model, "%s", "N/A");
+ (void)asprintf(&dev->path, "%s", fullname);
if (check_partition_table(dev)) {
add_block_dev(dev);
} else {
--
2.14.1

Andreas J. Reichel

unread,
Sep 28, 2017, 9:36:53 AM9/28/17
to efibootg...@googlegroups.com, Andreas Reichel
From: Andreas Reichel <andreas.r...@siemens.com>

* Fix missing format strings to avoid exploits or stack corruption
* Fix warnings:
* missing type casts
* missing return values
* ignored return values
* possibly unitialized variables
* unused variables
* unintended integer overflows
* dereference of freed pointers
* Fix file offset size to 64 bits in Makefile

Signed-off-by: Andreas Reichel <andreas.r...@siemens.com>
---
Makefile.am | 3 ++-
env/env_api.c | 3 +--
env/env_api_fat.c | 16 +++++++--------
env/uservars.c | 6 +++---
tools/bg_setenv.c | 59 ++++++++++++++++++++++++++++++++-----------------------
tools/ebgpart.c | 57 +++++++++++++++++++++++++++++------------------------
6 files changed, 79 insertions(+), 65 deletions(-)
index d0997bd..b1eea1c 100644
@@ -254,7 +255,7 @@ static bool check_partition_table(PedDevice *dev)

VERBOSE(stdout, "Checking %s\n", dev->path);
fd = open(dev->path, O_RDONLY);
- if (fd == 0) {
+ if (fd < 0) {
VERBOSE(stderr, "Error opening block device.\n");
return false;

Jan Kiszka

unread,
Sep 29, 2017, 2:40:35 AM9/29/17
to [ext] Andreas J. Reichel, efibootg...@googlegroups.com
Duplicate ";".

Not seeing the full context here ATM, but shouldn't we do a more
sophisticated range check on the input instead of simply down-casting to
16 bits? I mean, we do have USTATE_MIN/MAX.
And here is nothing remarked about not checking the return value?
Let's just kill these ugly and error-prone assignments inside if clause:
two statements, please.
Do we check elsewhere if the allocation succeeded, or do we just crash
then by chance?

What should not happen that due to "NULL+offset" we end up pointing into
mapped memory and loose the NULL-page protection. We may consider
wrapping these allocating functions, check centrally if they succeeded
and terminate the program otherwise. The latter is fine if we avoid
doing such allocation in the middle of some possibly critical file update.
Jan

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

Andreas Reichel

unread,
Oct 4, 2017, 12:36:23 PM10/4/17
to Jan Kiszka, efibootg...@googlegroups.com
The more sophisticatedcheck is already done by ebg_env_setglobalstate
internally. The bound checks are here because of strtol itself.

However, do not merge this, as a "green travis-cppcheck" patch is on the
way.
Thanks for finding.
If you wish it... for me they are neither ugly nor error-prone :)
I have to think about this...
--
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

Reply all
Reply to author
Forward
0 new messages