Re: [PATCH] Fix NULL difference in super_by_fd

[lixiaokeng <lixiaokeng@huawei.com>]

On 2022/12/13 23:30, Jes Sorensen wrote:
> On 12/12/22 04:23, lixiaokeng wrote:
>> When we create 100 partitions(major is 259 not 254) in a raid device,
>> mdadm may coredump:
>>
>> Core was generated by `/usr/sbin/mdadm --detail --export /dev/md1p3'.
>> Program terminated with signal SIGSEGV, Segmentation fault.
>> #0  __strlen_sse2 ()
>>     at ../sysdeps/x86_64/multiarch/strlen-vec.S:126
>> 126		movdqu	(%rax), %xmm4
>> (gdb) bt
>> #0  __strlen_sse2 ()
>>     at ../sysdeps/x86_64/multiarch/strlen-vec.S:126
>> #1  0x00007f1944659139 in __strcpy_chk (
>>     dest=dest@entry=0x55ea8d7c23ac "", src=0x0,
>>     destlen=destlen@entry=32) at strcpy_chk.c:28
>> #2  0x000055ea8d10b66d in strcpy (__src=<optimized out>,
>>     __dest=0x55ea8d7c23ac "")
>>     at /usr/include/bits/string_fortified.h:79
>> #3  super_by_fd (fd=fd@entry=3,
>>     subarrayp=subarrayp@entry=0x7ffe6a1dff08) at util.c:1289
>> #4  0x000055ea8d11b3a6 in Detail (
>>     dev=0x7ffe6a1e2f22 "/dev/md1p3", c=0x7ffe6a1e1700)
>>     at Detail.c:101
>> #5  0x000055ea8d101e61 in misc_list (c=<optimized out>,
>>     ss=<optimized out>, dump_directory=<optimized out>,
>>     ident=<optimized out>, devlist=<optimized out>)
>>     at mdadm.c:1959
>> #6  main (argc=<optimized out>, argv=<optimized out>)
>>     at mdadm.c:1629
>>
>> The direct cause is fd2devnm return NULL. Here add a check.
>>
>> Signed-off-by:Lixiaokeng<lixiaokeng@huawei.com>
>> Signed-off-by:Wuguanghao<wuguanghao3@huawei.com>
>> ---
>>  util.c | 6 +++++-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/util.c b/util.c
>> index 26ffdcea..843bfc6d 100644
>> --- a/util.c
>> +++ b/util.c
>> @@ -1177,6 +1177,7 @@ struct supertype *super_by_fd(int fd, char **subarrayp)
>>  	int i;
>>  	char *subarray = NULL;
>>  	char container[32] = "";
>> +	char *devnm = NULL;
>>
>>  	sra = sysfs_read(fd, NULL, GET_VERSION);
>>
>> @@ -1222,7 +1223,10 @@ struct supertype *super_by_fd(int fd, char **subarrayp)
>>  		if (subarrayp)
>>  			*subarrayp = subarray;
>>  		strcpy(st->container_devnm, container);
>> -		strcpy(st->devnm, fd2devnm(fd));
>> +		if (devnm = fd2devnm(fd))
>> +			strcpy(st->devnm, devnm);
>> +		else
>> +			st->devnm[0] = '\0';
> 
> I don't think this is the correct fix. You end up returning an
> incomplete 'st' entry, which could cause unexpected behavior. I think
> the right way to handle this is to fail properly and return NULL from
> super_by_fd(), after cleaning up properly.
> 
> Cheers,
> Jes
> 
Thanks for reply, I will change my patch and send it again.

> 
> .
>