diff mbox series

Fix null pointer for incremental in mdadm

Message ID 20230323013723.3242033-1-miaoguanqin@huawei.com (mailing list archive)
State Changes Requested
Headers show
Series Fix null pointer for incremental in mdadm | expand

Commit Message

Guanqin Miao March 23, 2023, 1:37 a.m. UTC
when we excute mdadm --assemble, udev-md-raid-assembly.rules is triggered.
Then we stop array, we found an coredump for mdadm --incremental.func
stack are as follows:

#0  enough (level=10, raid_disks=4, layout=258, clean=1, 
    avail=avail@entry=0x0) at util.c:555
#1  0x0000562170c26965 in Incremental (devlist=<optimized out>, 
    c=<optimized out>, st=0x5621729b6dc0) at Incremental.c:514
#2  0x0000562170bfb6ff in main (argc=<optimized out>, 
    argv=<optimized out>) at mdadm.c:1762

func enough() use array avail,avail allocate space in func count_active,
it may not alloc space, causing a coredump.We fix this coredump.

Signed-off-by: miaoguanqin <miaoguanqin@huawei.com>
Signed-off-by: lixiaokeng <lixiaokeng@huawei.com>
---
 Incremental.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

Jes Sorensen March 24, 2023, 1:30 p.m. UTC | #1
On 3/22/23 21:37, miaoguanqin wrote:
> when we excute mdadm --assemble, udev-md-raid-assembly.rules is triggered.
> Then we stop array, we found an coredump for mdadm --incremental.func
> stack are as follows:
> 
> #0  enough (level=10, raid_disks=4, layout=258, clean=1, 
>     avail=avail@entry=0x0) at util.c:555
> #1  0x0000562170c26965 in Incremental (devlist=<optimized out>, 
>     c=<optimized out>, st=0x5621729b6dc0) at Incremental.c:514
> #2  0x0000562170bfb6ff in main (argc=<optimized out>, 
>     argv=<optimized out>) at mdadm.c:1762
> 
> func enough() use array avail,avail allocate space in func count_active,
> it may not alloc space, causing a coredump.We fix this coredump.
> 
> Signed-off-by: miaoguanqin <miaoguanqin@huawei.com>
> Signed-off-by: lixiaokeng <lixiaokeng@huawei.com>
> ---
>  Incremental.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/Incremental.c b/Incremental.c
> index a4ff7d4..acbbee7 100644
> --- a/Incremental.c
> +++ b/Incremental.c
> @@ -506,6 +506,9 @@ int Incremental(struct mddev_dev *devlist, struct context *c,
>  				    GET_OFFSET | GET_SIZE));
>  	active_disks = count_active(st, sra, mdfd, &avail, &info);
>  
> +	if (!avail)
> +		goto out_unlock;
> +
>  	journal_device_missing = (info.journal_device_required) && (info.journal_clean == 0);
>  
>  	if (info.consistency_policy == CONSISTENCY_POLICY_PPL)
> @@ -620,7 +623,8 @@ int Incremental(struct mddev_dev *devlist, struct context *c,
>  		rv = 0;
>  	}
>  out:
> -	free(avail);
> +	if (avail)
> +		free(avail);

free(NULL) is legitimate, no need to do the avail check here.

Jes
Paul Menzel March 24, 2023, 1:40 p.m. UTC | #2
Dear Miao,


Am 23.03.23 um 02:37 schrieb miaoguanqin:

[…]

> Signed-off-by: miaoguanqin <miaoguanqin@huawei.com>
> Signed-off-by: lixiaokeng <lixiaokeng@huawei.com>

Thank you for your patches. It’d be great if you used the full names 
(also in your From: field) instead of usernames. For example, Li Xiao Keng.

     git config --global user.name "…"
     git commit --amend --author="… <miaoguanqin@huawei.com>"


Kind regards,

Paul
diff mbox series

Patch

diff --git a/Incremental.c b/Incremental.c
index a4ff7d4..acbbee7 100644
--- a/Incremental.c
+++ b/Incremental.c
@@ -506,6 +506,9 @@  int Incremental(struct mddev_dev *devlist, struct context *c,
 				    GET_OFFSET | GET_SIZE));
 	active_disks = count_active(st, sra, mdfd, &avail, &info);
 
+	if (!avail)
+		goto out_unlock;
+
 	journal_device_missing = (info.journal_device_required) && (info.journal_clean == 0);
 
 	if (info.consistency_policy == CONSISTENCY_POLICY_PPL)
@@ -620,7 +623,8 @@  int Incremental(struct mddev_dev *devlist, struct context *c,
 		rv = 0;
 	}
 out:
-	free(avail);
+	if (avail)
+		free(avail);
 	if (dfd >= 0)
 		close(dfd);
 	if (mdfd >= 0)