Re: [Qemu-devel] [PATCH v2] bt: use size_t type for length parameters instead of int

[Philippe Mathieu-Daudé]

Hi Hugo,

On 1/28/19 10:31 AM, Hugo Lefeuvre wrote:
> Hi,
> 
>> The length parameter values are not negative, thus use an unsigned
>> type 'size_t' for them. Many routines pass 'len' values to memcpy(3)
>> calls. If it was negative, it could lead to memory corruption issues.
>> Add check to avoid it.
> 
> I'm working on a Debian LTS security update for qemu and am currently
> thinking about addressing this issue as well.
> 
> I see this patch has not been applied yet and the bluetooth subsystem
> is pending deprecation. Are you still considering to apply it?

I have been assigned to fix this issue, but rather fixing locally this
BT device, fix the pattern on all devices.
I'll post the series during the week and Cc you (and eventually the
Debian LTS list when it gets merged). The series obsoletes this patch,
so the plan is to not apply it.

> 
>> @@ -113,6 +113,7 @@ static void vhci_host_send(void *opaque,
>>      static uint8_t buf[4096];
>>  
>>      buf[0] = type;
>> +    assert(len < sizeof(buf));
>>      memcpy(buf + 1, data, len);
>>  
>>      while (write(s->fd, buf, len + 1) < 0)
> 
> Any reason why assert() calls are used here ?
> 
> These checks should always be executed, but they won't if user compiles
> without asserts. Also, AFAIK any assert failure will stop the qemu host
> process which is not what we want in this case.

There was a discussion about this, and the outcome is QEMU does not
support building without assertions. See this commit:

https://git.qemu.org/?p=qemu.git;a=blobdiff;f=include/qemu/osdep.h;h=9966638;hp=6855b94;hb=262a69f42;hpb=825bfa005

Regards,

Phil.

signature.asc
Description: OpenPGP digital signature