On Thu, Mar 18, 2021 at 08:26:21PM +0800, Jason Yan wrote:
> When the user submitted a request with unaligned buffer, we will
> allocate a new page and try to copy data to or from the new page.
> If it is a reading request, we always copy back the data to user's
> buffer, whether the result is good or error. So if the driver or
> hardware returns an error, garbage data is copied to the user space.
> This is a potential security issue which makes kernel info leaks.
>
> So do not copy the uninitalized data to user's buffer if the
> bio->bi_status is not BLK_STS_OK in bio_copy_kern_endio_read().
If we're using copy_kern routines, doesn't that mean it's a kernel
request rather than user space?
I think the patch is probably good, though. The only minor concern I
have is if anyone wanted to observe a partial data transfer on error.
The caller currently gets that data, but this patch will prevent it. I
don't know of anyone actually depending on that behavior, so it may not
be a real concern.
> Signed-off-by: Jason Yan <yanaijie@xxxxxxxxxx>
> ---
> block/blk-map.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/block/blk-map.c b/block/blk-map.c
> index 1ffef782fcf2..c2e2162d54d9 100644
> --- a/block/blk-map.c
> +++ b/block/blk-map.c
> @@ -439,9 +439,11 @@ static void bio_copy_kern_endio_read(struct bio *bio)
> struct bio_vec *bvec;
> struct bvec_iter_all iter_all;
>
> - bio_for_each_segment_all(bvec, bio, iter_all) {
> - memcpy(p, page_address(bvec->bv_page), bvec->bv_len);
> - p += bvec->bv_len;
> + if (!bio->bi_status) {
> + bio_for_each_segment_all(bvec, bio, iter_all) {
> + memcpy(p, page_address(bvec->bv_page), bvec->bv_len);
> + p += bvec->bv_len;
> + }
> }
>
> bio_copy_kern_endio(bio);
