From 831f800da34ddb449aecae925cd1f154f20b1eed Mon Sep 17 00:00:00 2001 From: "kay.sievers@vrfy.org" Date: Thu, 26 Feb 2004 19:40:22 -0800 Subject: [PATCH] [PATCH] udev - safer string handling - part two As promised, here is the next round. We provide in addition to the already used macros: strfieldcpy(to, from) strfieldcat(to, from) the corresponding friends, if the size of the target is not known and must be provided by the caller: strnfieldcpy(to, from, maxsize) strnfieldcat(to, from, maxsize) and switch nearly all possibly unsafe users of strcat(), strncat(), strcpy() and strncpy() to these safer macros. The last known remaining issue seems the use of sprintf() and snprintf(). I will take on it later today or tomorrow. --- namedev.c | 31 ++++++++++++++++++------------- udev.h | 14 +++++++++++++- udev_config.c | 2 +- 3 files changed, 32 insertions(+), 15 deletions(-) diff --git a/namedev.c b/namedev.c index 219cb8a4..f688507d 100644 --- a/namedev.c +++ b/namedev.c @@ -209,7 +209,9 @@ static int get_format_len(char **str) return -1; } -static void apply_format(struct udevice *udev, unsigned char *string, struct sysfs_class_device *class_dev, struct sysfs_device *sysfs_device) +static void apply_format(struct udevice *udev, char *string, size_t maxsize, + struct sysfs_class_device *class_dev, + struct sysfs_device *sysfs_device) { char temp[NAME_SIZE]; char temp1[NAME_SIZE]; @@ -245,19 +247,19 @@ static void apply_format(struct udevice *udev, unsigned char *string, struct sys case 'b': if (strlen(udev->bus_id) == 0) break; - strcat(pos, udev->bus_id); + strnfieldcat(pos, udev->bus_id, maxsize); dbg("substitute bus_id '%s'", udev->bus_id); break; case 'k': if (strlen(udev->kernel_name) == 0) break; - strcat(pos, udev->kernel_name); + strnfieldcat(pos, udev->kernel_name, maxsize); dbg("substitute kernel name '%s'", udev->kernel_name); break; case 'n': if (strlen(udev->kernel_number) == 0) break; - strcat(pos, udev->kernel_number); + strnfieldcat(pos, udev->kernel_number, maxsize); dbg("substitute kernel number '%s'", udev->kernel_number); break; case 'm': @@ -287,11 +289,11 @@ static void apply_format(struct udevice *udev, unsigned char *string, struct sys } } if (pos3) { - strcat(pos, pos3); + strnfieldcat(pos, pos3, maxsize); dbg("substitute part of result string '%s'", pos3); } } else { - strcat(pos, udev->program_result); + strnfieldcat(pos, udev->program_result, maxsize); dbg("substitute result string '%s'", udev->program_result); } break; @@ -302,20 +304,20 @@ static void apply_format(struct udevice *udev, unsigned char *string, struct sys dbg("sysfa attribute '%s' not found", attr); break; } - strcpy(pos, tmpattr->value); + strnfieldcpy(pos, tmpattr->value, maxsize); dbg("substitute sysfs value '%s'", tmpattr->value); } else { dbg("missing attribute"); } break; case '%': - strcat(pos, "%"); + strnfieldcat(pos, "%", maxsize); break; default: dbg("unknown substitution type '%%%c'", c); break; } - strcat(pos, tail); + strnfieldcat(pos, tail, maxsize); } } @@ -462,7 +464,7 @@ static int execute_program(char *path, char *value, int len) strncpy(value, buffer, len); pos = value + strlen(value)-1; if (pos[0] == '\n') - pos[0] = '\0'; + pos[0] = '\0'; dbg("result is '%s'", value); } } @@ -733,7 +735,8 @@ static int match_rule(struct config_device *dev, struct sysfs_class_device *clas /* execute external program */ if (dev->program[0] != '\0') { dbg("check " FIELD_PROGRAM); - apply_format(udev, dev->program, class_dev, sysfs_device); + apply_format(udev, dev->program, sizeof(dev->program), + class_dev, sysfs_device); if (execute_program(dev->program, udev->program_result, NAME_SIZE) != 0) { dbg(FIELD_PROGRAM " returned nozero"); goto try_parent; @@ -825,8 +828,10 @@ int namedev_name_device(struct sysfs_class_device *class_dev, struct udevice *ud found: /* substitute placeholder */ - apply_format(udev, udev->name, class_dev, sysfs_device); - apply_format(udev, udev->symlink, class_dev, sysfs_device); + apply_format(udev, udev->name, sizeof(udev->name), + class_dev, sysfs_device); + apply_format(udev, udev->symlink, sizeof(udev->symlink), + class_dev, sysfs_device); udev->partitions = dev->partitions; done: perm = find_perm(udev->name); diff --git a/udev.h b/udev.h index fc44a984..0ce010f3 100644 --- a/udev.h +++ b/udev.h @@ -64,7 +64,19 @@ do { \ #define strfieldcat(to, from) \ do { \ to[sizeof(to)-1] = '\0'; \ - strncat(to, from, sizeof(to) - strlen(to) -1); \ + strncat(to, from, sizeof(to) - strlen(to)-1); \ +} while (0) + +#define strnfieldcpy(to, from, maxsize) \ +do { \ + to[maxsize-1] = '\0'; \ + strncpy(to, from, maxsize-1); \ +} while (0) + +#define strnfieldcat(to, from, maxsize) \ +do { \ + to[maxsize-1] = '\0'; \ + strncat(to, from, maxsize - strlen(to)-1); \ } while (0) extern int udev_add_device(char *path, char *subsystem, int fake); diff --git a/udev_config.c b/udev_config.c index cade81cf..de83ef9c 100644 --- a/udev_config.c +++ b/udev_config.c @@ -81,7 +81,7 @@ static void init_variables(void) #define set_var(_name, _var) \ if (strcasecmp(variable, _name) == 0) { \ dbg_parse("%s = '%s'", _name, value); \ - strncpy(_var, value, sizeof(_var)); \ + strnfieldcpy(_var, value, sizeof(_var));\ } #define set_bool(_name, _var) \ -- 2.39.5