2023-04-21 06:29:40

by Daniil Tatianin

[permalink] [raw]
Subject: [PATCH v2] md/md-multipath: guard against a possible NULL dereference

mempool_alloc might fail to allocate a slot, in which case we will end
up dereferencing a NULL mp_bh pointer.

Found by Linux Verification Center (linuxtesting.org) with the SVACE
static analysis tool.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Daniil Tatianin <[email protected]>
---
Changes since v1: fixed a typo
---
drivers/md/md-multipath.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/md/md-multipath.c b/drivers/md/md-multipath.c
index 66edf5e72bd6..8dfa353440e5 100644
--- a/drivers/md/md-multipath.c
+++ b/drivers/md/md-multipath.c
@@ -108,6 +108,8 @@ static bool multipath_make_request(struct mddev *mddev, struct bio * bio)
return true;

mp_bh = mempool_alloc(&conf->pool, GFP_NOIO);
+ if (!mp_bh)
+ return false;

mp_bh->master_bio = bio;
mp_bh->mddev = mddev;
--
2.25.1


2023-04-23 03:24:57

by Yu Kuai

[permalink] [raw]
Subject: Re: [PATCH v2] md/md-multipath: guard against a possible NULL dereference

Hi,

?? 2023/04/21 14:21, Daniil Tatianin ะด??:
> mempool_alloc might fail to allocate a slot, in which case we will end
> up dereferencing a NULL mp_bh pointer.
>
> Found by Linux Verification Center (linuxtesting.org) with the SVACE
> static analysis tool.
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Daniil Tatianin <[email protected]>
> ---
> Changes since v1: fixed a typo
> ---
> drivers/md/md-multipath.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/md/md-multipath.c b/drivers/md/md-multipath.c
> index 66edf5e72bd6..8dfa353440e5 100644
> --- a/drivers/md/md-multipath.c
> +++ b/drivers/md/md-multipath.c
> @@ -108,6 +108,8 @@ static bool multipath_make_request(struct mddev *mddev, struct bio * bio)
> return true;
>
> mp_bh = mempool_alloc(&conf->pool, GFP_NOIO);

The comment in mempool_alloc says:

Note that due to preallocation, this function *never* fails when called
from process contexts.

And there are many places to use this way, so I think this patch is not
needed.

Thanks,
Kuai
> + if (!mp_bh)
> + return false;
>
> mp_bh->master_bio = bio;
> mp_bh->mddev = mddev;
>