* [PATCH] Fix NULL difference in super_by_fd
@ 2022-12-12 9:23 lixiaokeng
2022-12-13 15:30 ` Jes Sorensen
0 siblings, 1 reply; 4+ messages in thread
From: lixiaokeng @ 2022-12-12 9:23 UTC (permalink / raw)
To: jes, linux-raid; +Cc: linfeilong, liuzhiqiang (I), Wu Guanghao
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';
} else
free(subarray);
--
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] Fix NULL difference in super_by_fd
2022-12-12 9:23 [PATCH] Fix NULL difference in super_by_fd lixiaokeng
@ 2022-12-13 15:30 ` Jes Sorensen
2022-12-14 1:19 ` lixiaokeng
2022-12-14 3:08 ` lixiaokeng
0 siblings, 2 replies; 4+ messages in thread
From: Jes Sorensen @ 2022-12-13 15:30 UTC (permalink / raw)
To: lixiaokeng, linux-raid; +Cc: linfeilong, liuzhiqiang (I), Wu Guanghao
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Fix NULL difference in super_by_fd
2022-12-13 15:30 ` Jes Sorensen
@ 2022-12-14 1:19 ` lixiaokeng
2022-12-14 3:08 ` lixiaokeng
1 sibling, 0 replies; 4+ messages in thread
From: lixiaokeng @ 2022-12-14 1:19 UTC (permalink / raw)
To: Jes Sorensen, linux-raid; +Cc: linfeilong, liuzhiqiang (I), Wu Guanghao
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.
>
> .
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Fix NULL difference in super_by_fd
2022-12-13 15:30 ` Jes Sorensen
2022-12-14 1:19 ` lixiaokeng
@ 2022-12-14 3:08 ` lixiaokeng
1 sibling, 0 replies; 4+ messages in thread
From: lixiaokeng @ 2022-12-14 3:08 UTC (permalink / raw)
To: Jes Sorensen, linux-raid; +Cc: linfeilong, liuzhiqiang (I), Wu Guanghao
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.
Here is another question: Should devid2devnm() be modifed?
Becasue the major of MD partition may not be 254(mdp).
>
> Cheers,
> Jes
>
>
> .
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-12-14 3:08 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-12-12 9:23 [PATCH] Fix NULL difference in super_by_fd lixiaokeng
2022-12-13 15:30 ` Jes Sorensen
2022-12-14 1:19 ` lixiaokeng
2022-12-14 3:08 ` lixiaokeng
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox