| Message ID | 20220620140659.20822-1-kinga.tanska@intel.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Coly Li |
| Headers | show |
| Series | [v3] Monitor: use devname as char array instead of pointer | expand |
> 2022年6月20日 22:06，Kinga Tanska <kinga.tanska@intel.com> 写道： > > Device name wasn't filled properly due to incorrect use of strcpy. > Strcpy was used twice. Firstly to fill devname with "/dev/md/" > and then to add chosen name. First strcpy result was overwritten by > second one (as a result <device_name> instead of "/dev/md/<device_name>" > was assigned). This commit changes this implementation to use snprintf > and devname with fixed size. Also safer string functions are propagated. > > Signed-off-by: Kinga Tanska <kinga.tanska@intel.com> > --- > Monitor.c | 30 +++++++++++------------------- > 1 file changed, 11 insertions(+), 19 deletions(-) > > diff --git a/Monitor.c b/Monitor.c > index 6ca1ebe5..04112791 100644 > --- a/Monitor.c > +++ b/Monitor.c > @@ -33,8 +33,8 @@ > #endif > > struct state { > - char *devname; > - char devnm[32]; /* to sync with mdstat info */ > + char devname[MD_NAME_MAX + 8]; > + char devnm[MD_NAME_MAX]; /* to sync with mdstat info */ > unsigned int utime; > int err; > char *spare_group; > @@ -45,7 +45,7 @@ struct state { > int devstate[MAX_DISKS]; > dev_t devid[MAX_DISKS]; > int percent; > - char parent_devnm[32]; /* For subarray, devnm of parent. > + char parent_devnm[MD_NAME_MAX]; /* For subarray, devnm of parent. > * For others, "" > */ > struct supertype *metadata; > @@ -187,13 +187,7 @@ int Monitor(struct mddev_dev *devlist, > continue; > > st = xcalloc(1, sizeof *st); > - if (mdlist->devname[0] == '/') > - st->devname = xstrdup(mdlist->devname); > - else { > - st->devname = xmalloc(8+strlen(mdlist->devname)+1); > - strcpy(strcpy(st->devname, "/dev/md/"), > - mdlist->devname); > - } > + snprintf(st->devname, MD_NAME_MAX + 8, "/dev/md/%s", basename(mdlist->devname)); Hi Kinga, The above location is what the whole patch wants to fix. It is not worth to change other locations. IMHO the change set is a bit large than what it should be, just a suggestion maybe a following style change is enough (not test it), @@ -190,9 +190,13 @@ int Monitor(struct mddev_dev *devlist, if (mdlist->devname[0] == '/') st->devname = xstrdup(mdlist->devname); else { - st->devname = xmalloc(8+strlen(mdlist->devname)+1); - strcpy(strcpy(st->devname, "/dev/md/"), - mdlist->devname); + int _len; + + _len = 8 + strlen(mdlist->devname); + st->devname = xmalloc(_len + 1); + memset(st->devname, 0, _len + 1); + snprintf(st->devname, _len, "/dev/md/%s", + mdlist->devname); } if (!is_mddev(mdlist->devname)) return 1; Thanks. Coly Li > if (!is_mddev(mdlist->devname)) > return 1; > st->next = statelist; > @@ -216,7 +210,7 @@ int Monitor(struct mddev_dev *devlist, > > st = xcalloc(1, sizeof *st); > mdlist = conf_get_ident(dv->devname); > - st->devname = xstrdup(dv->devname); > + snprintf(st->devname, MD_NAME_MAX + 8, "%s", dv->devname); > st->next = statelist; > st->devnm[0] = 0; > st->percent = RESYNC_UNKNOWN; > @@ -299,7 +293,6 @@ int Monitor(struct mddev_dev *devlist, > for (stp = &statelist; (st = *stp) != NULL; ) { > if (st->from_auto && st->err > 5) { > *stp = st->next; > - free(st->devname); > free(st->spare_group); > free(st); > } else > @@ -552,7 +545,7 @@ static int check_array(struct state *st, struct mdstat_ent *mdstat, > goto disappeared; > > if (st->devnm[0] == 0) > - strcpy(st->devnm, fd2devnm(fd)); > + snprintf(st->devnm, MD_NAME_MAX, "%s", fd2devnm(fd)); > > for (mse2 = mdstat; mse2; mse2 = mse2->next) > if (strcmp(mse2->devnm, st->devnm) == 0) { > @@ -682,7 +675,7 @@ static int check_array(struct state *st, struct mdstat_ent *mdstat, > strncmp(mse->metadata_version, "external:", 9) == 0 && > is_subarray(mse->metadata_version+9)) { > char *sl; > - strcpy(st->parent_devnm, mse->metadata_version + 10); > + snprintf(st->parent_devnm, MD_NAME_MAX, "%s", mse->metadata_version + 10); > sl = strchr(st->parent_devnm, '/'); > if (sl) > *sl = 0; > @@ -770,14 +763,13 @@ static int add_new_arrays(struct mdstat_ent *mdstat, struct state **statelist, > continue; > } > > - st->devname = xstrdup(name); > + snprintf(st->devname, MD_NAME_MAX + 8, "%s", name); > if ((fd = open(st->devname, O_RDONLY)) < 0 || > md_get_array_info(fd, &array) < 0) { > /* no such array */ > if (fd >= 0) > close(fd); > put_md_name(st->devname); > - free(st->devname); > if (st->metadata) { > st->metadata->ss->free_super(st->metadata); > free(st->metadata); > @@ -789,7 +781,7 @@ static int add_new_arrays(struct mdstat_ent *mdstat, struct state **statelist, > st->next = *statelist; > st->err = 1; > st->from_auto = 1; > - strcpy(st->devnm, mse->devnm); > + snprintf(st->devnm, MD_NAME_MAX, "%s", mse->devnm); > st->percent = RESYNC_UNKNOWN; > st->expected_spares = -1; > if (mse->metadata_version && > @@ -797,8 +789,8 @@ static int add_new_arrays(struct mdstat_ent *mdstat, struct state **statelist, > "external:", 9) == 0 && > is_subarray(mse->metadata_version+9)) { > char *sl; > - strcpy(st->parent_devnm, > - mse->metadata_version+10); > + snprintf(st->parent_devnm, MD_NAME_MAX, > + "%s", mse->metadata_version + 10); > sl = strchr(st->parent_devnm, '/'); > *sl = 0; > } else > -- > 2.26.2 >
On Sat, 25 Jun 2022 15:03:32 +0800 Coly Li <colyli@suse.de> wrote: > > 2022年6月20日 22:06，Kinga Tanska <kinga.tanska@intel.com> 写道： > > > > Device name wasn't filled properly due to incorrect use of strcpy. > > Strcpy was used twice. Firstly to fill devname with "/dev/md/" > > and then to add chosen name. First strcpy result was overwritten by > > second one (as a result <device_name> instead of > > "/dev/md/<device_name>" was assigned). This commit changes this > > implementation to use snprintf and devname with fixed size. Also > > safer string functions are propagated. > > > > Signed-off-by: Kinga Tanska <kinga.tanska@intel.com> > > --- > > Monitor.c | 30 +++++++++++------------------- > > 1 file changed, 11 insertions(+), 19 deletions(-) > > > > diff --git a/Monitor.c b/Monitor.c > > index 6ca1ebe5..04112791 100644 > > --- a/Monitor.c > > +++ b/Monitor.c > > @@ -33,8 +33,8 @@ > > #endif > > > > struct state { > > - char *devname; > > - char devnm[32]; /* to sync with mdstat info */ > > + char devname[MD_NAME_MAX + 8]; > > + char devnm[MD_NAME_MAX]; /* to sync with mdstat > > info */ unsigned int utime; > > int err; > > char *spare_group; > > @@ -45,7 +45,7 @@ struct state { > > int devstate[MAX_DISKS]; > > dev_t devid[MAX_DISKS]; > > int percent; > > - char parent_devnm[32]; /* For subarray, devnm of parent. > > + char parent_devnm[MD_NAME_MAX]; /* For subarray, devnm of > > parent. > > * For others, "" > > */ > > struct supertype *metadata; > > @@ -187,13 +187,7 @@ int Monitor(struct mddev_dev *devlist, > > continue; > > > > st = xcalloc(1, sizeof *st); > > - if (mdlist->devname[0] == '/') > > - st->devname = > > xstrdup(mdlist->devname); > > - else { > > - st->devname = > > xmalloc(8+strlen(mdlist->devname)+1); > > - strcpy(strcpy(st->devname, > > "/dev/md/"), > > - mdlist->devname); > > - } > > + snprintf(st->devname, MD_NAME_MAX + 8, > > "/dev/md/%s", basename(mdlist->devname)); > > Hi Kinga, > > The above location is what the whole patch wants to fix. It is not > worth to change other locations. IMHO the change set is a bit large > than what it should be, just a suggestion maybe a following style > change is enough (not test it), > > @@ -190,9 +190,13 @@ int Monitor(struct mddev_dev *devlist, > if (mdlist->devname[0] == '/') > st->devname = > xstrdup(mdlist->devname); else { > - st->devname = > xmalloc(8+strlen(mdlist->devname)+1); > - strcpy(strcpy(st->devname, > "/dev/md/"), > - mdlist->devname); > + int _len; > + > + _len = 8 + strlen(mdlist->devname); > + st->devname = xmalloc(_len + 1); > + memset(st->devname, 0, _len + 1); > + snprintf(st->devname, _len, > "/dev/md/%s", > + mdlist->devname); > } > if (!is_mddev(mdlist->devname)) > return 1; > > Thanks. > > Coly Li > > > > > if (!is_mddev(mdlist->devname)) > > return 1; > > st->next = statelist; > > @@ -216,7 +210,7 @@ int Monitor(struct mddev_dev *devlist, > > > > st = xcalloc(1, sizeof *st); > > mdlist = conf_get_ident(dv->devname); > > - st->devname = xstrdup(dv->devname); > > + snprintf(st->devname, MD_NAME_MAX + 8, > > "%s", dv->devname); st->next = statelist; > > st->devnm[0] = 0; > > st->percent = RESYNC_UNKNOWN; > > @@ -299,7 +293,6 @@ int Monitor(struct mddev_dev *devlist, > > for (stp = &statelist; (st = *stp) != NULL; ) { > > if (st->from_auto && st->err > 5) { > > *stp = st->next; > > - free(st->devname); > > free(st->spare_group); > > free(st); > > } else > > @@ -552,7 +545,7 @@ static int check_array(struct state *st, struct > > mdstat_ent *mdstat, goto disappeared; > > > > if (st->devnm[0] == 0) > > - strcpy(st->devnm, fd2devnm(fd)); > > + snprintf(st->devnm, MD_NAME_MAX, "%s", > > fd2devnm(fd)); > > > > for (mse2 = mdstat; mse2; mse2 = mse2->next) > > if (strcmp(mse2->devnm, st->devnm) == 0) { > > @@ -682,7 +675,7 @@ static int check_array(struct state *st, struct > > mdstat_ent *mdstat, strncmp(mse->metadata_version, "external:", 9) > > == 0 && is_subarray(mse->metadata_version+9)) { > > char *sl; > > - strcpy(st->parent_devnm, mse->metadata_version + > > 10); > > + snprintf(st->parent_devnm, MD_NAME_MAX, "%s", > > mse->metadata_version + 10); sl = strchr(st->parent_devnm, '/'); > > if (sl) > > *sl = 0; > > @@ -770,14 +763,13 @@ static int add_new_arrays(struct mdstat_ent > > *mdstat, struct state **statelist, continue; > > } > > > > - st->devname = xstrdup(name); > > + snprintf(st->devname, MD_NAME_MAX + 8, > > "%s", name); if ((fd = open(st->devname, O_RDONLY)) < 0 || > > md_get_array_info(fd, &array) < 0) { > > /* no such array */ > > if (fd >= 0) > > close(fd); > > put_md_name(st->devname); > > - free(st->devname); > > if (st->metadata) { > > st->metadata->ss->free_super(st->metadata); > > free(st->metadata); > > @@ -789,7 +781,7 @@ static int add_new_arrays(struct mdstat_ent > > *mdstat, struct state **statelist, st->next = *statelist; > > st->err = 1; > > st->from_auto = 1; > > - strcpy(st->devnm, mse->devnm); > > + snprintf(st->devnm, MD_NAME_MAX, "%s", > > mse->devnm); st->percent = RESYNC_UNKNOWN; > > st->expected_spares = -1; > > if (mse->metadata_version && > > @@ -797,8 +789,8 @@ static int add_new_arrays(struct mdstat_ent > > *mdstat, struct state **statelist, "external:", 9) == 0 && > > is_subarray(mse->metadata_version+9)) { > > char *sl; > > - strcpy(st->parent_devnm, > > - mse->metadata_version+10); > > + snprintf(st->parent_devnm, > > MD_NAME_MAX, > > + "%s", > > mse->metadata_version + 10); sl = strchr(st->parent_devnm, '/'); > > *sl = 0; > > } else > > -- > > 2.26.2 > > > Hi Coly, Yes, fragment of code, which you selected, is enough to fix it. I have changed also 32 to define, and fixed unsafe functions in this file to improve code quality and avoid magic number. What's your opinion, is this change worth to add? Would you mind getting two patches, one with this specific fix, and other one with another improvements in this file? Regards, Kinga
> 2022年7月5日 20:05，Kinga Tanska <kinga.tanska@linux.intel.com> 写道： > > On Sat, 25 Jun 2022 15:03:32 +0800 > Coly Li <colyli@suse.de> wrote: > >>> 2022年6月20日 22:06，Kinga Tanska <kinga.tanska@intel.com> 写道： >>> >>> Device name wasn't filled properly due to incorrect use of strcpy. >>> Strcpy was used twice. Firstly to fill devname with "/dev/md/" >>> and then to add chosen name. First strcpy result was overwritten by >>> second one (as a result <device_name> instead of >>> "/dev/md/<device_name>" was assigned). This commit changes this >>> implementation to use snprintf and devname with fixed size. Also >>> safer string functions are propagated. >>> >>> Signed-off-by: Kinga Tanska <kinga.tanska@intel.com> >>> --- >>> Monitor.c | 30 +++++++++++------------------- >>> 1 file changed, 11 insertions(+), 19 deletions(-) >>> >>> diff --git a/Monitor.c b/Monitor.c >>> index 6ca1ebe5..04112791 100644 >>> --- a/Monitor.c >>> +++ b/Monitor.c >>> @@ -33,8 +33,8 @@ >>> #endif >>> >>> struct state { >>> - char *devname; >>> - char devnm[32]; /* to sync with mdstat info */ >>> + char devname[MD_NAME_MAX + 8]; >>> + char devnm[MD_NAME_MAX]; /* to sync with mdstat >>> info */ unsigned int utime; >>> int err; >>> char *spare_group; >>> @@ -45,7 +45,7 @@ struct state { >>> int devstate[MAX_DISKS]; >>> dev_t devid[MAX_DISKS]; >>> int percent; >>> - char parent_devnm[32]; /* For subarray, devnm of parent. >>> + char parent_devnm[MD_NAME_MAX]; /* For subarray, devnm of >>> parent. >>> * For others, "" >>> */ >>> struct supertype *metadata; >>> @@ -187,13 +187,7 @@ int Monitor(struct mddev_dev *devlist, >>> continue; >>> >>> st = xcalloc(1, sizeof *st); >>> - if (mdlist->devname[0] == '/') >>> - st->devname = >>> xstrdup(mdlist->devname); >>> - else { >>> - st->devname = >>> xmalloc(8+strlen(mdlist->devname)+1); >>> - strcpy(strcpy(st->devname, >>> "/dev/md/"), >>> - mdlist->devname); >>> - } >>> + snprintf(st->devname, MD_NAME_MAX + 8, >>> "/dev/md/%s", basename(mdlist->devname)); >> >> Hi Kinga, >> >> The above location is what the whole patch wants to fix. It is not >> worth to change other locations. IMHO the change set is a bit large >> than what it should be, just a suggestion maybe a following style >> change is enough (not test it), >> >> @@ -190,9 +190,13 @@ int Monitor(struct mddev_dev *devlist, >> if (mdlist->devname[0] == '/') >> st->devname = >> xstrdup(mdlist->devname); else { >> - st->devname = >> xmalloc(8+strlen(mdlist->devname)+1); >> - strcpy(strcpy(st->devname, >> "/dev/md/"), >> - mdlist->devname); >> + int _len; >> + >> + _len = 8 + strlen(mdlist->devname); >> + st->devname = xmalloc(_len + 1); >> + memset(st->devname, 0, _len + 1); >> + snprintf(st->devname, _len, >> "/dev/md/%s", >> + mdlist->devname); >> } >> if (!is_mddev(mdlist->devname)) >> return 1; >> >> Thanks. >> >> Coly Li >> >> >> >>> if (!is_mddev(mdlist->devname)) >>> return 1; >>> st->next = statelist; >>> @@ -216,7 +210,7 @@ int Monitor(struct mddev_dev *devlist, >>> >>> st = xcalloc(1, sizeof *st); >>> mdlist = conf_get_ident(dv->devname); >>> - st->devname = xstrdup(dv->devname); >>> + snprintf(st->devname, MD_NAME_MAX + 8, >>> "%s", dv->devname); st->next = statelist; >>> st->devnm[0] = 0; >>> st->percent = RESYNC_UNKNOWN; >>> @@ -299,7 +293,6 @@ int Monitor(struct mddev_dev *devlist, >>> for (stp = &statelist; (st = *stp) != NULL; ) { >>> if (st->from_auto && st->err > 5) { >>> *stp = st->next; >>> - free(st->devname); >>> free(st->spare_group); >>> free(st); >>> } else >>> @@ -552,7 +545,7 @@ static int check_array(struct state *st, struct >>> mdstat_ent *mdstat, goto disappeared; >>> >>> if (st->devnm[0] == 0) >>> - strcpy(st->devnm, fd2devnm(fd)); >>> + snprintf(st->devnm, MD_NAME_MAX, "%s", >>> fd2devnm(fd)); >>> >>> for (mse2 = mdstat; mse2; mse2 = mse2->next) >>> if (strcmp(mse2->devnm, st->devnm) == 0) { >>> @@ -682,7 +675,7 @@ static int check_array(struct state *st, struct >>> mdstat_ent *mdstat, strncmp(mse->metadata_version, "external:", 9) >>> == 0 && is_subarray(mse->metadata_version+9)) { >>> char *sl; >>> - strcpy(st->parent_devnm, mse->metadata_version + >>> 10); >>> + snprintf(st->parent_devnm, MD_NAME_MAX, "%s", >>> mse->metadata_version + 10); sl = strchr(st->parent_devnm, '/'); >>> if (sl) >>> *sl = 0; >>> @@ -770,14 +763,13 @@ static int add_new_arrays(struct mdstat_ent >>> *mdstat, struct state **statelist, continue; >>> } >>> >>> - st->devname = xstrdup(name); >>> + snprintf(st->devname, MD_NAME_MAX + 8, >>> "%s", name); if ((fd = open(st->devname, O_RDONLY)) < 0 || >>> md_get_array_info(fd, &array) < 0) { >>> /* no such array */ >>> if (fd >= 0) >>> close(fd); >>> put_md_name(st->devname); >>> - free(st->devname); >>> if (st->metadata) { >>> st->metadata->ss->free_super(st->metadata); >>> free(st->metadata); >>> @@ -789,7 +781,7 @@ static int add_new_arrays(struct mdstat_ent >>> *mdstat, struct state **statelist, st->next = *statelist; >>> st->err = 1; >>> st->from_auto = 1; >>> - strcpy(st->devnm, mse->devnm); >>> + snprintf(st->devnm, MD_NAME_MAX, "%s", >>> mse->devnm); st->percent = RESYNC_UNKNOWN; >>> st->expected_spares = -1; >>> if (mse->metadata_version && >>> @@ -797,8 +789,8 @@ static int add_new_arrays(struct mdstat_ent >>> *mdstat, struct state **statelist, "external:", 9) == 0 && >>> is_subarray(mse->metadata_version+9)) { >>> char *sl; >>> - strcpy(st->parent_devnm, >>> - mse->metadata_version+10); >>> + snprintf(st->parent_devnm, >>> MD_NAME_MAX, >>> + "%s", >>> mse->metadata_version + 10); sl = strchr(st->parent_devnm, '/'); >>> *sl = 0; >>> } else >>> -- >>> 2.26.2 >>> >> > > Hi Coly, > > Yes, fragment of code, which you selected, is enough to fix it. I have > changed also 32 to define, and fixed unsafe functions in this file to > improve code quality and avoid magic number. What's your opinion, is > this change worth to add? > Would you mind getting two patches, one with this specific fix, and > other one with another improvements in this file? It will be cool, if the patch can be split into read-fix and code-clean-up parts. So I can provide my feedback fast for the real fix part. Thanks. Coly Li
diff --git a/Monitor.c b/Monitor.c index 6ca1ebe5..04112791 100644 --- a/Monitor.c +++ b/Monitor.c @@ -33,8 +33,8 @@ #endif struct state { - char *devname; - char devnm[32]; /* to sync with mdstat info */ + char devname[MD_NAME_MAX + 8]; + char devnm[MD_NAME_MAX]; /* to sync with mdstat info */ unsigned int utime; int err; char *spare_group; @@ -45,7 +45,7 @@ struct state { int devstate[MAX_DISKS]; dev_t devid[MAX_DISKS]; int percent; - char parent_devnm[32]; /* For subarray, devnm of parent. + char parent_devnm[MD_NAME_MAX]; /* For subarray, devnm of parent. * For others, "" */ struct supertype *metadata; @@ -187,13 +187,7 @@ int Monitor(struct mddev_dev *devlist, continue; st = xcalloc(1, sizeof *st); - if (mdlist->devname[0] == '/') - st->devname = xstrdup(mdlist->devname); - else { - st->devname = xmalloc(8+strlen(mdlist->devname)+1); - strcpy(strcpy(st->devname, "/dev/md/"), - mdlist->devname); - } + snprintf(st->devname, MD_NAME_MAX + 8, "/dev/md/%s", basename(mdlist->devname)); if (!is_mddev(mdlist->devname)) return 1; st->next = statelist; @@ -216,7 +210,7 @@ int Monitor(struct mddev_dev *devlist, st = xcalloc(1, sizeof *st); mdlist = conf_get_ident(dv->devname); - st->devname = xstrdup(dv->devname); + snprintf(st->devname, MD_NAME_MAX + 8, "%s", dv->devname); st->next = statelist; st->devnm[0] = 0; st->percent = RESYNC_UNKNOWN; @@ -299,7 +293,6 @@ int Monitor(struct mddev_dev *devlist, for (stp = &statelist; (st = *stp) != NULL; ) { if (st->from_auto && st->err > 5) { *stp = st->next; - free(st->devname); free(st->spare_group); free(st); } else @@ -552,7 +545,7 @@ static int check_array(struct state *st, struct mdstat_ent *mdstat, goto disappeared; if (st->devnm[0] == 0) - strcpy(st->devnm, fd2devnm(fd)); + snprintf(st->devnm, MD_NAME_MAX, "%s", fd2devnm(fd)); for (mse2 = mdstat; mse2; mse2 = mse2->next) if (strcmp(mse2->devnm, st->devnm) == 0) { @@ -682,7 +675,7 @@ static int check_array(struct state *st, struct mdstat_ent *mdstat, strncmp(mse->metadata_version, "external:", 9) == 0 && is_subarray(mse->metadata_version+9)) { char *sl; - strcpy(st->parent_devnm, mse->metadata_version + 10); + snprintf(st->parent_devnm, MD_NAME_MAX, "%s", mse->metadata_version + 10); sl = strchr(st->parent_devnm, '/'); if (sl) *sl = 0; @@ -770,14 +763,13 @@ static int add_new_arrays(struct mdstat_ent *mdstat, struct state **statelist, continue; } - st->devname = xstrdup(name); + snprintf(st->devname, MD_NAME_MAX + 8, "%s", name); if ((fd = open(st->devname, O_RDONLY)) < 0 || md_get_array_info(fd, &array) < 0) { /* no such array */ if (fd >= 0) close(fd); put_md_name(st->devname); - free(st->devname); if (st->metadata) { st->metadata->ss->free_super(st->metadata); free(st->metadata); @@ -789,7 +781,7 @@ static int add_new_arrays(struct mdstat_ent *mdstat, struct state **statelist, st->next = *statelist; st->err = 1; st->from_auto = 1; - strcpy(st->devnm, mse->devnm); + snprintf(st->devnm, MD_NAME_MAX, "%s", mse->devnm); st->percent = RESYNC_UNKNOWN; st->expected_spares = -1; if (mse->metadata_version && @@ -797,8 +789,8 @@ static int add_new_arrays(struct mdstat_ent *mdstat, struct state **statelist, "external:", 9) == 0 && is_subarray(mse->metadata_version+9)) { char *sl; - strcpy(st->parent_devnm, - mse->metadata_version+10); + snprintf(st->parent_devnm, MD_NAME_MAX, + "%s", mse->metadata_version + 10); sl = strchr(st->parent_devnm, '/'); *sl = 0; } else
Device name wasn't filled properly due to incorrect use of strcpy. Strcpy was used twice. Firstly to fill devname with "/dev/md/" and then to add chosen name. First strcpy result was overwritten by second one (as a result <device_name> instead of "/dev/md/<device_name>" was assigned). This commit changes this implementation to use snprintf and devname with fixed size. Also safer string functions are propagated. Signed-off-by: Kinga Tanska <kinga.tanska@intel.com> --- Monitor.c | 30 +++++++++++------------------- 1 file changed, 11 insertions(+), 19 deletions(-)