Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1369383 > unrolled thread

[PATCH v2] block: fix possible NULL dereference

Back to article view | Back to linux.kernel

Contents

  [PATCH v2] block: fix possible NULL dereference Sudip Mukherjee <sudipm.mukherjee@gmail.com> - 2016-04-01 16:40 +0200
    Re: [PATCH v2] block: fix possible NULL dereference Jens Axboe <axboe@kernel.dk> - 2016-04-01 16:40 +0200
      Re: [PATCH v2] block: fix possible NULL dereference Sudip Mukherjee <sudipm.mukherjee@gmail.com> - 2016-04-01 17:30 +0200

#1369383 — [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 <sudip.mukherjee@codethink.co.uk>
---

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

[toc] | [next] | [standalone]

#1369392

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 <sudip.mukherjee@codethink.co.uk>
> ---
>
> 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

[toc] | [prev] | [next] | [standalone]

#1369421

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 <sudip.mukherjee@codethink.co.uk>
> >---
> >
> >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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web