linux-raid.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Monitor: use devname as char array instead of pointer
@ 2022-02-09  8:56 Kinga Tanska
  2022-03-19 15:42 ` Coly Li
  0 siblings, 1 reply; 4+ messages in thread
From: Kinga Tanska @ 2022-02-09  8:56 UTC (permalink / raw)
  To: linux-raid; +Cc: jes

Device name wasn't filled properly due to incorrect use of strcpy.
Instead pointer, devname with fixed size is used and 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 30c031a2..d02344ec 100644
--- a/Monitor.c
+++ b/Monitor.c
@@ -34,8 +34,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;
@@ -46,7 +46,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;
@@ -184,13 +184,7 @@ int Monitor(struct mddev_dev *devlist,
 			if (strcasecmp(mdlist->devname, "<ignore>") == 0)
 				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));
 			st->next = statelist;
 			st->devnm[0] = 0;
 			st->percent = RESYNC_UNKNOWN;
@@ -206,7 +200,7 @@ int Monitor(struct mddev_dev *devlist,
 		for (dv = devlist; dv; dv = dv->next) {
 			struct state *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;
@@ -289,7 +283,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
@@ -540,7 +533,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) {
@@ -670,7 +663,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;
@@ -758,14 +751,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);
@@ -777,7 +769,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 &&
@@ -785,8 +777,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


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] Monitor: use devname as char array instead of pointer
  2022-02-09  8:56 [PATCH] Monitor: use devname as char array instead of pointer Kinga Tanska
@ 2022-03-19 15:42 ` Coly Li
  2022-04-04 13:54   ` Jes Sorensen
  0 siblings, 1 reply; 4+ messages in thread
From: Coly Li @ 2022-03-19 15:42 UTC (permalink / raw)
  To: Kinga Tanska; +Cc: jes, linux-raid

On 2/9/22 4:56 PM, Kinga Tanska wrote:
> Device name wasn't filled properly due to incorrect use of strcpy.

Could you point out the specific location for improper strcpy?


> Instead pointer, devname with fixed size is used and 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 30c031a2..d02344ec 100644
> --- a/Monitor.c
> +++ b/Monitor.c
> @@ -34,8 +34,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;
> @@ -46,7 +46,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;
> @@ -184,13 +184,7 @@ int Monitor(struct mddev_dev *devlist,
>   			if (strcasecmp(mdlist->devname, "<ignore>") == 0)
>   				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));


I feel the above change is incorrect, the tailing '\0' of the string 
might be cut by your change.


>   			st->next = statelist;
>   			st->devnm[0] = 0;
>   			st->percent = RESYNC_UNKNOWN;
> @@ -206,7 +200,7 @@ int Monitor(struct mddev_dev *devlist,
>   		for (dv = devlist; dv; dv = dv->next) {
>   			struct state *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;
> @@ -289,7 +283,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
> @@ -540,7 +533,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) {
> @@ -670,7 +663,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;
> @@ -758,14 +751,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);
> @@ -777,7 +769,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 &&
> @@ -785,8 +777,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


With your change, the tailing '\0' for dev name might be cut. Could you 
please check whether it may introduce potential memleak ?


Thanks.


Coly Li



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Monitor: use devname as char array instead of pointer
  2022-03-19 15:42 ` Coly Li
@ 2022-04-04 13:54   ` Jes Sorensen
  2022-04-05 12:21     ` Tanska, Kinga
  0 siblings, 1 reply; 4+ messages in thread
From: Jes Sorensen @ 2022-04-04 13:54 UTC (permalink / raw)
  To: Coly Li, Kinga Tanska; +Cc: linux-raid

On 3/19/22 11:42, Coly Li wrote:
> On 2/9/22 4:56 PM, Kinga Tanska wrote:
>> Device name wasn't filled properly due to incorrect use of strcpy.
> 
> Could you point out the specific location for improper strcpy?
> 
> 
>> Instead pointer, devname with fixed size is used and 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 30c031a2..d02344ec 100644
>> --- a/Monitor.c
>> +++ b/Monitor.c
>> @@ -34,8 +34,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;
>> @@ -46,7 +46,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;
>> @@ -184,13 +184,7 @@ int Monitor(struct mddev_dev *devlist,
>>               if (strcasecmp(mdlist->devname, "<ignore>") == 0)
>>                   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));
> 
> 
> I feel the above change is incorrect, the tailing '\0' of the string
> might be cut by your change.
> 
> 
>>               st->next = statelist;
>>               st->devnm[0] = 0;
>>               st->percent = RESYNC_UNKNOWN;
>> @@ -206,7 +200,7 @@ int Monitor(struct mddev_dev *devlist,
>>           for (dv = devlist; dv; dv = dv->next) {
>>               struct state *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;
>> @@ -289,7 +283,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
>> @@ -540,7 +533,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) {
>> @@ -670,7 +663,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;
>> @@ -758,14 +751,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);
>> @@ -777,7 +769,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 &&
>> @@ -785,8 +777,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
> 
> 
> With your change, the tailing '\0' for dev name might be cut. Could you
> please check whether it may introduce potential memleak ?

Kinga,

Any update on this?

Thanks,
Jes



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Monitor: use devname as char array instead of pointer
  2022-04-04 13:54   ` Jes Sorensen
@ 2022-04-05 12:21     ` Tanska, Kinga
  0 siblings, 0 replies; 4+ messages in thread
From: Tanska, Kinga @ 2022-04-05 12:21 UTC (permalink / raw)
  To: Jes Sorensen, Coly Li, Kinga Tanska; +Cc: linux-raid

Hi,

I will send correction in next patchset. I didn't include terminating null
byte '\0' to size in snprintf function, which can cause memleaks.
Also specific location of incorrect usage of strcpy will be pointed out 
in commit message.

Regards,
Kinga

On 4/4/2022 3:54 PM, Jes Sorensen wrote:

On 3/19/22 11:42, Coly Li wrote:

> On 2/9/22 4:56 PM, Kinga Tanska wrote:
>> Device name wasn't filled properly due to incorrect use of strcpy.
> Could you point out the specific location for improper strcpy?
>
>
>> Instead pointer, devname with fixed size is used and 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 30c031a2..d02344ec 100644
>> --- a/Monitor.c
>> +++ b/Monitor.c
>> @@ -34,8 +34,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;
>> @@ -46,7 +46,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;
>> @@ -184,13 +184,7 @@ int Monitor(struct mddev_dev *devlist,
>>                if (strcasecmp(mdlist->devname, "<ignore>") == 0)
>>                    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));
> I feel the above change is incorrect, the tailing '\0' of the string
> might be cut by your change.
>
>
>>                st->next = statelist;
>>                st->devnm[0] = 0;
>>                st->percent = RESYNC_UNKNOWN;
>> @@ -206,7 +200,7 @@ int Monitor(struct mddev_dev *devlist,
>>            for (dv = devlist; dv; dv = dv->next) {
>>                struct state *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;
>> @@ -289,7 +283,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
>> @@ -540,7 +533,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) {
>> @@ -670,7 +663,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;
>> @@ -758,14 +751,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);
>> @@ -777,7 +769,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 &&
>> @@ -785,8 +777,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
> With your change, the tailing '\0' for dev name might be cut. Could you
> please check whether it may introduce potential memleak ?

Kinga,

Any update on this?

Thanks,
Jes



^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2022-04-05 14:59 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-02-09  8:56 [PATCH] Monitor: use devname as char array instead of pointer Kinga Tanska
2022-03-19 15:42 ` Coly Li
2022-04-04 13:54   ` Jes Sorensen
2022-04-05 12:21     ` Tanska, Kinga

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).