[RAID0] Possible bug in raid0_make_request() function

[RAID0] Possible bug in raid0_make_request() function

[Anuj Goel]

Hi Guys,
I was going through the RAID0 code and found that there could be a
possible bug in the raid0_make_request() function.

Consider the below snippet of this function.

/* This is a one page bio that upper layers
* refuse to split for us, so we need to split it.
*/
if (likely(is_power_of_2(chunk_sects)))
bp = bio_split(bio, chunk_sects - (sector &
  (chunk_sects-1)));
else
bp = bio_split(bio, chunk_sects -
      sector_div(sector, chunk_sects));
raid0_make_request(mddev, &bp->bio1);
raid0_make_request(mddev, &bp->bio2);
bio_pair_release(bp);
return;

In function bio_split(..), if mempool_alloc() fails, we return NULL into bp.
Now the call raid0_make_request(mddev, &bp->bio1) does not check for
bp to be not NULL before de-referencing it.

Could this be a problem ?

--
Best Regards,
Anuj Goel
--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RAID0] Possible bug in raid0_make_request() function

[NeilBrown]

[-- Attachment #1: Type: text/plain, Size: 1538 bytes --]

On Thu, 17 May 2012 02:50:28 -0400 Anuj Goel <agoel@cs.stonybrook.edu> wrote:

> Hi Guys,
> I was going through the RAID0 code and found that there could be a
> possible bug in the raid0_make_request() function.

Thanks.  I love it when people read the md/raid code and report what they
find!!

> 
> Consider the below snippet of this function.
> 
> /* This is a one page bio that upper layers
> * refuse to split for us, so we need to split it.
> */
> if (likely(is_power_of_2(chunk_sects)))
> bp = bio_split(bio, chunk_sects - (sector &
>   (chunk_sects-1)));
> else
> bp = bio_split(bio, chunk_sects -
>       sector_div(sector, chunk_sects));
> raid0_make_request(mddev, &bp->bio1);
> raid0_make_request(mddev, &bp->bio2);
> bio_pair_release(bp);
> return;
> 
> In function bio_split(..), if mempool_alloc() fails, we return NULL into bp.
> Now the call raid0_make_request(mddev, &bp->bio1) does not check for
> bp to be not NULL before de-referencing it.
> 
> Could this be a problem ?

No.  mempool_alloc can only fail if __GFP_WAIT it not passed in,
and bio_split calls it with GFP_NOIO which contains __GFP_WAIT.

So the 
   if (!bp)
      return bp;

at the top of bio_split is completely pointless - bp will never be NULL.

NeilBrown


> 
> --
> Best Regards,
> Anuj Goel
> --
> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]