[Qemu-devel] Possible null-ptr dereference

[Qemu-devel] Possible null-ptr dereference

[mateusz.krzywicki]

[-- Attachment #1: Type: text/plain, Size: 327 bytes --]

Hey,
Found a little bug in latest qemu:
In function: static int blk_send_response_one(struct ioreq *ioreq)
File: qemu\hw\block\xen_disk.c
Code:
    default:        dst = NULL;    }    memcpy(dst, &resp, sizeof(resp));

Just add simple check for dst and it will be all cool ;-)
Best regards,Mateusz Krzywicki 		 	   		  

[-- Attachment #2: Type: text/html, Size: 991 bytes --]

Re: [Qemu-devel] Possible null-ptr dereference

[Gonglei (Arei)]

[-- Attachment #1: Type: text/plain, Size: 1308 bytes --]

Hi,

Should be easy to fix though. Does the following help?

(Cc'ing Stefan & Kevin)

-->
xen_disk:  fix possible null-ptr dereference

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
hw/block/xen_disk.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
index aed5b5b..a221d0b 100644
--- a/hw/block/xen_disk.c
+++ b/hw/block/xen_disk.c
@@ -589,6 +589,7 @@ static int blk_send_response_one(struct ioreq *ioreq)
         break;
     default:
         dst = NULL;
+        return 0;
     }
     memcpy(dst, &resp, sizeof(resp));
     blkdev->rings.common.rsp_prod_pvt++;
--

Best regards,
-Gonglei

From: qemu-devel-bounces+arei.gonglei=huawei.com@nongnu.org [mailto:qemu-devel-bounces+arei.gonglei=huawei.com@nongnu.org] On Behalf Of mateusz.krzywicki@windowslive.com
Sent: Saturday, July 26, 2014 6:52 PM
To: qemu-devel@nongnu.org
Subject: [Qemu-devel] Possible null-ptr dereference

Hey,

Found a little bug in latest qemu:

In function:
static int blk_send_response_one(struct ioreq *ioreq)

File:
qemu\hw\block\xen_disk.c

Code:

    default:
        dst = NULL;
    }
    memcpy(dst, &resp, sizeof(resp));


Just add simple check for dst and it will be all cool ;-)

Best regards,
Mateusz Krzywicki

[-- Attachment #2: Type: text/html, Size: 13590 bytes --]

Re: [Qemu-devel] Possible null-ptr dereference

[mateusz.krzywicki]

[-- Attachment #1: Type: text/plain, Size: 1785 bytes --]

Hey,

Yup, thanks, task closed ;-)

Best regards,
Mateusz Krzywicki

From: arei.gonglei@huawei.com
To: mateusz.krzywicki@windowslive.com; qemu-devel@nongnu.org
CC: stefanha@redhat.com; kwolf@redhat.com
Subject: RE: [Qemu-devel] Possible null-ptr dereference
Date: Mon, 28 Jul 2014 06:03:45 +0000









Hi,
 
Should be easy to fix though. Does the following help?
 
(Cc’ing Stefan & Kevin)
 
-->
xen_disk:  fix possible null-ptr dereference

 
Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
hw/block/xen_disk.c | 1 +
1
 file changed, 1 insertion(+)
 
diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
index aed5b5b..a221d0b 100644
--- a/hw/block/xen_disk.c
+++ b/hw/block/xen_disk.c
@@ -589,6 +589,7 @@ static int blk_send_response_one(struct ioreq *ioreq)
         break;
     default:
         dst = NULL;
+        return 0;
     }
     memcpy(dst, &resp, sizeof(resp));
     blkdev->rings.common.rsp_prod_pvt++;
--
 
Best regards,
-Gonglei

 



From: qemu-devel-bounces+arei.gonglei=huawei.com@nongnu.org [mailto:qemu-devel-bounces+arei.gonglei=huawei.com@nongnu.org]
On Behalf Of mateusz.krzywicki@windowslive.com

Sent: Saturday, July 26, 2014 6:52 PM

To: qemu-devel@nongnu.org

Subject: [Qemu-devel] Possible null-ptr dereference


 


Hey,


 


Found a little bug in latest qemu:


 


In function: 


static int blk_send_response_one(struct ioreq *ioreq)


 


File: 


qemu\hw\block\xen_disk.c


 


Code:


 



    default:


        dst = NULL;


    }


    memcpy(dst, &resp, sizeof(resp));



 


 


Just add simple check for dst and it will be all cool ;-)


 


Best regards,


Mateusz Krzywicki



 		 	   		  

[-- Attachment #2: Type: text/html, Size: 12511 bytes --]

Re: [Qemu-devel] Possible null-ptr dereference

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 759 bytes --]

On Mon, Jul 28, 2014 at 06:03:45AM +0000, Gonglei (Arei) wrote:
> Hi,
> 
> Should be easy to fix though. Does the following help?
> 
> (Cc'ing Stefan & Kevin)
> 
> -->
> xen_disk:  fix possible null-ptr dereference
> 
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> ---
> hw/block/xen_disk.c | 1 +
> 1 file changed, 1 insertion(+)

This code path can never be reached since protocol is always set to one
of 3 valid values in xen_disk.c.  Therefore, I'm not merging this for
QEMU 2.1 where we are only taking critical bug fixes now.

Still, it will help silence static checkers and make the intent clear to
readers.

Thanks, applied to my block-next tree for QEMU 2.2:
https://github.com/stefanha/qemu/commits/block-next

Stefan

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

Re: [Qemu-devel] Possible null-ptr dereference

[Gonglei (Arei)]

> -----Original Message-----
> From: Stefan Hajnoczi [mailto:stefanha@redhat.com]
> Sent: Monday, July 28, 2014 5:49 PM
> To: Gonglei (Arei)
> Cc: mateusz.krzywicki@windowslive.com; qemu-devel@nongnu.org;
> kwolf@redhat.com
> Subject: Re: [Qemu-devel] Possible null-ptr dereference
> 
> On Mon, Jul 28, 2014 at 06:03:45AM +0000, Gonglei (Arei) wrote:
> > Hi,
> >
> > Should be easy to fix though. Does the following help?
> >
> > (Cc'ing Stefan & Kevin)
> >
> > -->
> > xen_disk:  fix possible null-ptr dereference
> >
> > Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> > ---
> > hw/block/xen_disk.c | 1 +
> > 1 file changed, 1 insertion(+)
> 
> This code path can never be reached since protocol is always set to one
> of 3 valid values in xen_disk.c.  Therefore, I'm not merging this for
> QEMU 2.1 where we are only taking critical bug fixes now.
> 
OK.

> Still, it will help silence static checkers and make the intent clear to
> readers.
> 
> Thanks, applied to my block-next tree for QEMU 2.2:
> https://github.com/stefanha/qemu/commits/block-next
> 
Thanks.

Best regards,
-Gonglei

Re: [Qemu-devel] Possible null-ptr dereference

[Peter Maydell]

On 28 July 2014 10:49, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> On Mon, Jul 28, 2014 at 06:03:45AM +0000, Gonglei (Arei) wrote:
>> Hi,
>>
>> Should be easy to fix though. Does the following help?
>>
>> (Cc'ing Stefan & Kevin)
>>
>> -->
>> xen_disk:  fix possible null-ptr dereference
>>
>> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
>> ---
>> hw/block/xen_disk.c | 1 +
>> 1 file changed, 1 insertion(+)
>
> This code path can never be reached since protocol is always set to one
> of 3 valid values in xen_disk.c.

Maybe g_assert_not_reached(); ?

thanks
-- PMM