Skip to contentSkip to conversation

virtio-blk: fix NULL checking in virtblk_alloc_req()

PATCH In review Dan Carpenter 4 participants 5 messages Sep 5, 2012 – Sep 6, 2012

Subsystems (4)
Block layer Kernel janitors virtio core virtio block and SCSI drivers
lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

PATCH 1/1 virtio-blk: fix NULL checking in virtblk_alloc_req()

Smatch complains about the inconsistent NULL checking here. Fix it to return NULL on failure.

Signed-off-by Dan Carpenter <[email protected]>

---
This is only needed in linux-next.

drivers/block/virtio_blk.c +4 -3
index 2edfb5c..457db0c 100644
@@ -90,10 +90,11 @@ static inline struct virtblk_req *virtblk_alloc_req(struct virtio_blk *vblk,
 	struct virtblk_req *vbr;
 
 	vbr = mempool_alloc(vblk->pool, gfp_mask);
-	if (vbr && use_bio)
-		sg_init_table(vbr->sg, vblk->sg_elems);
+	if (!vbr)
+		return NULL;
 
-	vbr->vblk = vblk;
+	if (use_bio)
+		sg_init_table(vbr->sg, vblk->sg_elems);
 
 	return vbr;
 }
maintainer
#1 lore raw
Quoted 4 lines · Dan Carpenter

Smatch complains about the inconsistent NULL checking here. Fix it to return NULL on failure.

Signed-off-by Dan Carpenter <[email protected]>

ACK

Quoted 2 lines

---
This is only needed in linux-next.

Yes upstream is OK.

linux-next picks up stuff from rusty's patch queue so presumably this can be folded into patch that triggered this.

Quoted 6 lines
drivers/block/virtio_blk.c +0 -0
index 2edfb5c..457db0c 100644

drivers/block/virtio_blk.c static inline struct virtblk_req *virtblk_alloc_req(struct virtio_blk *vblk, · line 96

Quoted 9 lines
drivers/block/virtio_blk.c +2 -3
@@ -90,10 +90,11 @@ static inline struct virtblk_req *virtblk_alloc_req(struct virtio_blk *vblk,
 	struct virtblk_req *vbr;
 
 	vbr = mempool_alloc(vblk->pool, gfp_mask);
-	if (vbr && use_bio)
-		sg_init_table(vbr->sg, vblk->sg_elems);
+	if (!vbr)
+		return NULL;
 
-	vbr->vblk = vblk;

Smatch is right to complain: on memory allocation
failure this will dereference NULL.

Quoted 5 lines
drivers/block/virtio_blk.c +2 -0
+	if (use_bio)
+		sg_init_table(vbr->sg, vblk->sg_elems);
 
 	return vbr;
 }
#2 lore raw
In reply to Dan Carpenter
Quoted 6 lines · Dan Carpenter

Smatch complains about the inconsistent NULL checking here. Fix it to return NULL on failure.

Signed-off-by Dan Carpenter <[email protected]>

---
This is only needed in linux-next.

Nice!

Quoted 9 lines
drivers/block/virtio_blk.c +0 -1
index 2edfb5c..457db0c 100644
@@ -90,10 +90,11 @@ static inline struct virtblk_req *virtblk_alloc_req(struct virtio_blk *vblk,
 	struct virtblk_req *vbr;
 
 	vbr = mempool_alloc(vblk->pool, gfp_mask);
-	if (vbr && use_bio)

drivers/block/virtio_blk.c static inline struct virtblk_req *virtblk_alloc_req(struct virtio_blk *vblk, · line 100

Quoted 10 lines
drivers/block/virtio_blk.c +4 -2
-		sg_init_table(vbr->sg, vblk->sg_elems);
+	if (!vbr)
+		return NULL;
 
-	vbr->vblk = vblk;
+	if (use_bio)
+		sg_init_table(vbr->sg, vblk->sg_elems);
 
 	return vbr;
 }

But it turns out that "vbr->vblk = vblk;" assignment is important :)

Fixed and applied,
Rusty.

#3 lore raw
Quoted 24 lines · Rusty Russell
Quoted 6 lines · Dan Carpenter

Smatch complains about the inconsistent NULL checking here. Fix it to return NULL on failure.

Signed-off-by Dan Carpenter <[email protected]>

---
This is only needed in linux-next.

Nice!

Quoted 13 lines
drivers/block/virtio_blk.c +2 -2
index 2edfb5c..457db0c 100644
@@ -90,10 +90,11 @@ static inline struct virtblk_req *virtblk_alloc_req(struct virtio_blk *vblk,
 	struct virtblk_req *vbr;
 
 	vbr = mempool_alloc(vblk->pool, gfp_mask);
-	if (vbr && use_bio)
-		sg_init_table(vbr->sg, vblk->sg_elems);
+	if (!vbr)
+		return NULL;
Quoted 8 lines

drivers/block/virtio_blk.c static inline struct virtblk_req *virtblk_alloc_req(struct virtio_blk *vblk, · line 100

Quoted 6 lines
drivers/block/virtio_blk.c +2 -1
-	vbr->vblk = vblk;
+	if (use_bio)
+		sg_init_table(vbr->sg, vblk->sg_elems);
 
 	return vbr;
 }

But it turns out that "vbr->vblk = vblk;" assignment is important :)

I was just replying ;-)

Quoted 1 line

Fixed and applied,

Thanks Rusty!

Signature · 1 line
Asias
#4 lore raw
In reply to Dan Carpenter

Hello Dan,

Quoted 4 lines · Dan Carpenter

Smatch complains about the inconsistent NULL checking here. Fix it to return NULL on failure.

Signed-off-by Dan Carpenter <[email protected]>

Thanks for catching this.

Quoted 8 lines

---
This is only needed in linux-next.

drivers/block/virtio_blk.c +0 -0
index 2edfb5c..457db0c 100644

drivers/block/virtio_blk.c static inline struct virtblk_req *virtblk_alloc_req(struct virtio_blk *vblk, · line 96

Quoted 9 lines
drivers/block/virtio_blk.c +2 -3
@@ -90,10 +90,11 @@ static inline struct virtblk_req *virtblk_alloc_req(struct virtio_blk *vblk,
 	struct virtblk_req *vbr;
 
 	vbr = mempool_alloc(vblk->pool, gfp_mask);
-	if (vbr && use_bio)
-		sg_init_table(vbr->sg, vblk->sg_elems);
+	if (!vbr)
+		return NULL;
 
-	vbr->vblk = vblk;

The assignment of vbr->vblk is needed.

Quoted 5 lines
drivers/block/virtio_blk.c +2 -0
+	if (use_bio)
+		sg_init_table(vbr->sg, vblk->sg_elems);
 
 	return vbr;
 }
Signature · 1 line
Asias