Re: softraid_raid5: possible NULL dereference
> Date: Thu, 26 Mar 2020 01:03:26 +0100 > From: Tobias Heider > > sr_block_get() returns dma_alloc(length, PR_NOWAIT | PR_ZERO) which may be > NULL if the memory pool is depleted. > The result is used as 'dst' argument to memcpy() in the following call to > sr_raid5_regenerate(), resulting in a possible NULL dereference. > > ok? > > Index: softraid_raid5.c > === > RCS file: /mount/openbsd/cvs/src/sys/dev/softraid_raid5.c,v > retrieving revision 1.29 > diff -u -p -r1.29 softraid_raid5.c > --- softraid_raid5.c 8 Aug 2019 02:19:55 - 1.29 > +++ softraid_raid5.c 25 Mar 2020 23:54:25 - > @@ -818,7 +818,8 @@ sr_raid5_rebuild(struct sr_discipline *s > wu_w = sr_scsi_wu_get(sd, 0); > wu_r = sr_scsi_wu_get(sd, 0); > > - xorbuf = sr_block_get(sd, strip_size); > + if ((xorbuf = sr_block_get(sd, strip_size)) == NULL) > + goto bad; > if (sr_raid5_regenerate(wu_r, rebuild_chunk, chunk_lba, > strip_size, xorbuf)) > goto bad; As a matter of style, it is better to avoid assignments in if-statements.
Re: softraid_raid5: possible NULL dereference
On Thu, Mar 26, 2020 at 01:03:26AM +0100, Tobias Heider wrote: > sr_block_get() returns dma_alloc(length, PR_NOWAIT | PR_ZERO) which may be > NULL if the memory pool is depleted. > The result is used as 'dst' argument to memcpy() in the following call to > sr_raid5_regenerate(), resulting in a possible NULL dereference. > > ok? > > Index: softraid_raid5.c > === > RCS file: /mount/openbsd/cvs/src/sys/dev/softraid_raid5.c,v > retrieving revision 1.29 > diff -u -p -r1.29 softraid_raid5.c > --- softraid_raid5.c 8 Aug 2019 02:19:55 - 1.29 > +++ softraid_raid5.c 25 Mar 2020 23:54:25 - > @@ -818,7 +818,8 @@ sr_raid5_rebuild(struct sr_discipline *s > wu_w = sr_scsi_wu_get(sd, 0); > wu_r = sr_scsi_wu_get(sd, 0); > > - xorbuf = sr_block_get(sd, strip_size); > + if ((xorbuf = sr_block_get(sd, strip_size)) == NULL) > + goto bad; > if (sr_raid5_regenerate(wu_r, rebuild_chunk, chunk_lba, > strip_size, xorbuf)) > goto bad; > Obvious fix. ok stsp@ Same problem is present in sr_raid5_scrub() which is currently in #if 0.
softraid_raid5: possible NULL dereference
sr_block_get() returns dma_alloc(length, PR_NOWAIT | PR_ZERO) which may be NULL if the memory pool is depleted. The result is used as 'dst' argument to memcpy() in the following call to sr_raid5_regenerate(), resulting in a possible NULL dereference. ok? Index: softraid_raid5.c === RCS file: /mount/openbsd/cvs/src/sys/dev/softraid_raid5.c,v retrieving revision 1.29 diff -u -p -r1.29 softraid_raid5.c --- softraid_raid5.c8 Aug 2019 02:19:55 - 1.29 +++ softraid_raid5.c25 Mar 2020 23:54:25 - @@ -818,7 +818,8 @@ sr_raid5_rebuild(struct sr_discipline *s wu_w = sr_scsi_wu_get(sd, 0); wu_r = sr_scsi_wu_get(sd, 0); - xorbuf = sr_block_get(sd, strip_size); + if ((xorbuf = sr_block_get(sd, strip_size)) == NULL) + goto bad; if (sr_raid5_regenerate(wu_r, rebuild_chunk, chunk_lba, strip_size, xorbuf)) goto bad;
