| Message ID | 1562872923-2463-1-git-send-email-wang6495@umn.edu (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] block/bio-integrity: fix a memory leak bug | expand |
On Fri, Jul 12, 2019 at 3:29 AM Wenwen Wang <wang6495@umn.edu> wrote: > > From: Wenwen Wang <wenwen@cs.uga.edu> > > In bio_integrity_prep(), a kernel buffer is allocated through kmalloc() to > hold integrity metadata. Later on, the buffer will be attached to the bio > structure through bio_integrity_add_page(), which returns the number of > bytes of integrity metadata attached. Due to unexpected situations, > bio_integrity_add_page() may return 0. As a result, bio_integrity_prep() > needs to be terminated with 'false' returned to indicate this error. > However, the allocated kernel buffer is not freed on this execution path, > leading to a memory leak. > > To fix this issue, free the allocated buffer before returning from > bio_integrity_prep(). > > Signed-off-by: Wenwen Wang <wenwen@cs.uga.edu> > --- > block/bio-integrity.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/block/bio-integrity.c b/block/bio-integrity.c > index 4db6208..fb95dbb 100644 > --- a/block/bio-integrity.c > +++ b/block/bio-integrity.c > @@ -276,8 +276,12 @@ bool bio_integrity_prep(struct bio *bio) > ret = bio_integrity_add_page(bio, virt_to_page(buf), > bytes, offset); > > - if (ret == 0) > - return false; > + if (ret == 0) { > + printk(KERN_ERR "could not attach integrity payload\n"); > + kfree(buf); > + status = BLK_STS_RESOURCE; > + goto err_end_io; > + } > > if (ret < bytes) > break; Looks fine: Reviewed-by: Ming Lei <ming.lei@redhat.com> Thanks, Ming Lei
Wenwen, > To fix this issue, free the allocated buffer before returning from > bio_integrity_prep(). Acked-by: Martin K. Petersen <martin.petersen@oracle.com>
On 7/11/19 1:22 PM, Wenwen Wang wrote: > From: Wenwen Wang <wenwen@cs.uga.edu> > > In bio_integrity_prep(), a kernel buffer is allocated through kmalloc() to > hold integrity metadata. Later on, the buffer will be attached to the bio > structure through bio_integrity_add_page(), which returns the number of > bytes of integrity metadata attached. Due to unexpected situations, > bio_integrity_add_page() may return 0. As a result, bio_integrity_prep() > needs to be terminated with 'false' returned to indicate this error. > However, the allocated kernel buffer is not freed on this execution path, > leading to a memory leak. > > To fix this issue, free the allocated buffer before returning from > bio_integrity_prep(). Applied, thanks.
diff --git a/block/bio-integrity.c b/block/bio-integrity.c index 4db6208..fb95dbb 100644 --- a/block/bio-integrity.c +++ b/block/bio-integrity.c @@ -276,8 +276,12 @@ bool bio_integrity_prep(struct bio *bio) ret = bio_integrity_add_page(bio, virt_to_page(buf), bytes, offset); - if (ret == 0) - return false; + if (ret == 0) { + printk(KERN_ERR "could not attach integrity payload\n"); + kfree(buf); + status = BLK_STS_RESOURCE; + goto err_end_io; + } if (ret < bytes) break;