2016-04-01 14:34:26

by Sudip Mukherjee

[permalink] [raw]
Subject: [PATCH v2] block: fix possible NULL dereference

We were checking for iter to be NULL after dereferencing it. There is
actually no need to check for iter to be NULL as all the callers of
blk_rq_map_user_iov() does call it with a valid pointer to
struct iov_iter.
But as iter->count can be NULL so the assignment to copy is being done
after checking for it.

Signed-off-by: Sudip Mukherjee <[email protected]>
---

v2: removed the check for iter
v1: moved the assignment to copy after check for iter and iter->count


block/blk-map.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/block/blk-map.c b/block/blk-map.c
index a54f054..e15b4aa 100644
--- a/block/blk-map.c
+++ b/block/blk-map.c
@@ -126,14 +126,15 @@ int blk_rq_map_user_iov(struct request_queue *q, struct request *rq,
const struct iov_iter *iter, gfp_t gfp_mask)
{
struct iovec iov, prv = {.iov_base = NULL, .iov_len = 0};
- bool copy = (q->dma_pad_mask & iter->count) || map_data;
+ bool copy;
struct bio *bio = NULL;
struct iov_iter i;
int ret;

- if (!iter || !iter->count)
+ if (!iter->count)
return -EINVAL;

+ copy = (q->dma_pad_mask & iter->count) || map_data;
iov_for_each(iov, i, *iter) {
unsigned long uaddr = (unsigned long) iov.iov_base;

--
2.1.4


2016-04-01 14:38:28

by Jens Axboe

[permalink] [raw]
Subject: Re: [PATCH v2] block: fix possible NULL dereference

On 04/01/2016 08:34 AM, Sudip Mukherjee wrote:
> We were checking for iter to be NULL after dereferencing it. There is
> actually no need to check for iter to be NULL as all the callers of
> blk_rq_map_user_iov() does call it with a valid pointer to
> struct iov_iter.
> But as iter->count can be NULL so the assignment to copy is being done
> after checking for it.
>
> Signed-off-by: Sudip Mukherjee <[email protected]>
> ---
>
> v2: removed the check for iter
> v1: moved the assignment to copy after check for iter and iter->count

Your subject is wrong (there's no NULL deref). Ditto for the commit
message - it can be zero, not NULL. The latter would imply a memory
address, but it's just an integer.

--
Jens Axboe

2016-04-01 15:18:40

by Sudip Mukherjee

[permalink] [raw]
Subject: Re: [PATCH v2] block: fix possible NULL dereference

On Fri, Apr 01, 2016 at 08:38:23AM -0600, Jens Axboe wrote:
> On 04/01/2016 08:34 AM, Sudip Mukherjee wrote:
> >We were checking for iter to be NULL after dereferencing it. There is
> >actually no need to check for iter to be NULL as all the callers of
> >blk_rq_map_user_iov() does call it with a valid pointer to
> >struct iov_iter.
> >But as iter->count can be NULL so the assignment to copy is being done
> >after checking for it.
> >
> >Signed-off-by: Sudip Mukherjee <[email protected]>
> >---
> >
> >v2: removed the check for iter
> >v1: moved the assignment to copy after check for iter and iter->count
>
> Your subject is wrong (there's no NULL deref). Ditto for the commit message
> - it can be zero, not NULL. The latter would imply a memory address, but
> it's just an integer.

oops. I should have checked. I wanted to keep the commit message similar to
v1. I will send a v3 for this.

regards
sudip