* [patch] virtio-blk: fix NULL checking in virtblk_alloc_req()
@ 2012-09-05 12:32 Dan Carpenter
2012-09-05 13:11 ` Michael S. Tsirkin
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Dan Carpenter @ 2012-09-05 12:32 UTC (permalink / raw)
To: Asias He; +Cc: kernel-janitors, virtualization, Michael S. Tsirkin
Smatch complains about the inconsistent NULL checking here. Fix it to
return NULL on failure.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
This is only needed in linux-next.
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 2edfb5c..457db0c 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -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;
}
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [patch] virtio-blk: fix NULL checking in virtblk_alloc_req()
2012-09-05 12:32 [patch] virtio-blk: fix NULL checking in virtblk_alloc_req() Dan Carpenter
@ 2012-09-05 13:11 ` Michael S. Tsirkin
2012-09-06 2:55 ` Rusty Russell
2012-09-06 3:02 ` Asias He
2 siblings, 0 replies; 5+ messages in thread
From: Michael S. Tsirkin @ 2012-09-05 13:11 UTC (permalink / raw)
To: Dan Carpenter; +Cc: kernel-janitors, virtualization
On Wed, Sep 05, 2012 at 03:32:53PM +0300, Dan Carpenter wrote:
> Smatch complains about the inconsistent NULL checking here. Fix it to
> return NULL on failure.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
ACK
> ---
> 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.
>
> diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
> index 2edfb5c..457db0c 100644
> --- a/drivers/block/virtio_blk.c
> +++ b/drivers/block/virtio_blk.c
> @@ -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.
> + if (use_bio)
> + sg_init_table(vbr->sg, vblk->sg_elems);
>
> return vbr;
> }
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [patch] virtio-blk: fix NULL checking in virtblk_alloc_req()
2012-09-05 12:32 [patch] virtio-blk: fix NULL checking in virtblk_alloc_req() Dan Carpenter
2012-09-05 13:11 ` Michael S. Tsirkin
@ 2012-09-06 2:55 ` Rusty Russell
2012-09-06 3:04 ` Asias He
2012-09-06 3:02 ` Asias He
2 siblings, 1 reply; 5+ messages in thread
From: Rusty Russell @ 2012-09-06 2:55 UTC (permalink / raw)
To: Dan Carpenter, Asias He
Cc: virtualization, kernel-janitors, Michael S. Tsirkin
Dan Carpenter <dan.carpenter@oracle.com> writes:
> Smatch complains about the inconsistent NULL checking here. Fix it to
> return NULL on failure.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> This is only needed in linux-next.
Nice!
> diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
> index 2edfb5c..457db0c 100644
> --- a/drivers/block/virtio_blk.c
> +++ b/drivers/block/virtio_blk.c
> @@ -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;
> }
But it turns out that "vbr->vblk = vblk;" assignment is important :)
Fixed and applied,
Rusty.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [patch] virtio-blk: fix NULL checking in virtblk_alloc_req()
2012-09-06 2:55 ` Rusty Russell
@ 2012-09-06 3:04 ` Asias He
0 siblings, 0 replies; 5+ messages in thread
From: Asias He @ 2012-09-06 3:04 UTC (permalink / raw)
To: Rusty Russell
Cc: virtualization, kernel-janitors, Dan Carpenter,
Michael S. Tsirkin
On 09/06/2012 10:55 AM, Rusty Russell wrote:
> Dan Carpenter <dan.carpenter@oracle.com> writes:
>
>> Smatch complains about the inconsistent NULL checking here. Fix it to
>> return NULL on failure.
>>
>> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>> ---
>> This is only needed in linux-next.
>
> Nice!
>
>> diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
>> index 2edfb5c..457db0c 100644
>> --- a/drivers/block/virtio_blk.c
>> +++ b/drivers/block/virtio_blk.c
>> @@ -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;
>> }
>
> But it turns out that "vbr->vblk = vblk;" assignment is important :)
I was just replying ;-)
> Fixed and applied,
Thanks Rusty!
--
Asias
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [patch] virtio-blk: fix NULL checking in virtblk_alloc_req()
2012-09-05 12:32 [patch] virtio-blk: fix NULL checking in virtblk_alloc_req() Dan Carpenter
2012-09-05 13:11 ` Michael S. Tsirkin
2012-09-06 2:55 ` Rusty Russell
@ 2012-09-06 3:02 ` Asias He
2 siblings, 0 replies; 5+ messages in thread
From: Asias He @ 2012-09-06 3:02 UTC (permalink / raw)
To: Dan Carpenter; +Cc: kernel-janitors, virtualization, Michael S. Tsirkin
Hello Dan,
On 09/05/2012 08:32 PM, Dan Carpenter wrote:
> Smatch complains about the inconsistent NULL checking here. Fix it to
> return NULL on failure.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Thanks for catching this.
> ---
> This is only needed in linux-next.
>
> diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
> index 2edfb5c..457db0c 100644
> --- a/drivers/block/virtio_blk.c
> +++ b/drivers/block/virtio_blk.c
> @@ -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.
> + if (use_bio)
> + sg_init_table(vbr->sg, vblk->sg_elems);
>
> return vbr;
> }
--
Asias
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-09-06 3:04 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-09-05 12:32 [patch] virtio-blk: fix NULL checking in virtblk_alloc_req() Dan Carpenter
2012-09-05 13:11 ` Michael S. Tsirkin
2012-09-06 2:55 ` Rusty Russell
2012-09-06 3:04 ` Asias He
2012-09-06 3:02 ` Asias He
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).