[PATCH v2 0/3] vfio/pci: Fix buffer overrun when writing the VF token

Cédric Le Goater posted 3 patches 1 year, 1 month ago
Failed in applying to current master (apply log)
include/qemu/uuid.h              | 2 +-
block/parallels-ext.c            | 2 +-
block/vdi.c                      | 2 +-
hw/core/qdev-properties-system.c | 2 +-
hw/hyperv/vmbus.c                | 4 ++--
hw/vfio/pci.c                    | 2 +-
migration/savevm.c               | 4 ++--
tests/unit/test-uuid.c           | 2 +-
util/uuid.c                      | 2 +-
9 files changed, 11 insertions(+), 11 deletions(-)
[PATCH v2 0/3] vfio/pci: Fix buffer overrun when writing the VF token
Posted by Cédric Le Goater 1 year, 1 month ago
Hello,

This series fixes a buffer overrun in VFIO. The buffer used in
vfio_realize() by qemu_uuid_unparse() is too small, UUID_FMT_LEN lacks
one byte for the trailing NUL.

Instead of adding + 1, as done elsewhere, the changes introduce a
UUID_STR_LEN define for the correct size and use it where required.

Thanks,

C. 

Changes in v2:
 - removal of UUID_FMT_LEN

Cédric Le Goater (3):
  util/uuid: Add UUID_STR_LEN definition
  vfio/pci: Fix buffer overrun when writing the VF token
  util/uuid: Remove UUID_FMT_LEN

 include/qemu/uuid.h              | 2 +-
 block/parallels-ext.c            | 2 +-
 block/vdi.c                      | 2 +-
 hw/core/qdev-properties-system.c | 2 +-
 hw/hyperv/vmbus.c                | 4 ++--
 hw/vfio/pci.c                    | 2 +-
 migration/savevm.c               | 4 ++--
 tests/unit/test-uuid.c           | 2 +-
 util/uuid.c                      | 2 +-
 9 files changed, 11 insertions(+), 11 deletions(-)

-- 
2.41.0


Re: [PATCH v2 0/3] vfio/pci: Fix buffer overrun when writing the VF token
Posted by Cédric Le Goater 1 year, 1 month ago
On 10/26/23 09:06, Cédric Le Goater wrote:
> Hello,
> 
> This series fixes a buffer overrun in VFIO. The buffer used in
> vfio_realize() by qemu_uuid_unparse() is too small, UUID_FMT_LEN lacks
> one byte for the trailing NUL.
> 
> Instead of adding + 1, as done elsewhere, the changes introduce a
> UUID_STR_LEN define for the correct size and use it where required.

Cc: qemu-stable@nongnu.org # 8.1+

I propose to take this series in vfio-next if no one objects.

Thanks,

C.


> 
> Thanks,
> 
> C.
> 
> Changes in v2:
>   - removal of UUID_FMT_LEN
> 
> Cédric Le Goater (3):
>    util/uuid: Add UUID_STR_LEN definition
>    vfio/pci: Fix buffer overrun when writing the VF token
>    util/uuid: Remove UUID_FMT_LEN
> 
>   include/qemu/uuid.h              | 2 +-
>   block/parallels-ext.c            | 2 +-
>   block/vdi.c                      | 2 +-
>   hw/core/qdev-properties-system.c | 2 +-
>   hw/hyperv/vmbus.c                | 4 ++--
>   hw/vfio/pci.c                    | 2 +-
>   migration/savevm.c               | 4 ++--
>   tests/unit/test-uuid.c           | 2 +-
>   util/uuid.c                      | 2 +-
>   9 files changed, 11 insertions(+), 11 deletions(-)
> 


Re: [PATCH v2 0/3] vfio/pci: Fix buffer overrun when writing the VF token
Posted by Cédric Le Goater 1 year ago
On 10/26/23 16:00, Cédric Le Goater wrote:
> On 10/26/23 09:06, Cédric Le Goater wrote:
>> Hello,
>>
>> This series fixes a buffer overrun in VFIO. The buffer used in
>> vfio_realize() by qemu_uuid_unparse() is too small, UUID_FMT_LEN lacks
>> one byte for the trailing NUL.
>>
>> Instead of adding + 1, as done elsewhere, the changes introduce a
>> UUID_STR_LEN define for the correct size and use it where required.
> 
> Cc: qemu-stable@nongnu.org # 8.1+
> 
> I propose to take this series in vfio-next if no one objects.


Applied to vfio-next.

Thanks,

C.


Re: [PATCH v2 0/3] vfio/pci: Fix buffer overrun when writing the VF token
Posted by Philippe Mathieu-Daudé 1 year, 1 month ago
On 26/10/23 16:00, Cédric Le Goater wrote:
> On 10/26/23 09:06, Cédric Le Goater wrote:
>> Hello,
>>
>> This series fixes a buffer overrun in VFIO. The buffer used in
>> vfio_realize() by qemu_uuid_unparse() is too small, UUID_FMT_LEN lacks
>> one byte for the trailing NUL.
>>
>> Instead of adding + 1, as done elsewhere, the changes introduce a
>> UUID_STR_LEN define for the correct size and use it where required.
> 
> Cc: qemu-stable@nongnu.org # 8.1+

Hopefully 8.2 shouldn't be affected ;)

> 
> I propose to take this series in vfio-next if no one objects.
> 
> Thanks,
> 
> C.


Re: [PATCH v2 0/3] vfio/pci: Fix buffer overrun when writing the VF token
Posted by Denis V. Lunev 1 year, 1 month ago
On 10/26/23 09:06, Cédric Le Goater wrote:
> Hello,
>
> This series fixes a buffer overrun in VFIO. The buffer used in
> vfio_realize() by qemu_uuid_unparse() is too small, UUID_FMT_LEN lacks
> one byte for the trailing NUL.
>
> Instead of adding + 1, as done elsewhere, the changes introduce a
> UUID_STR_LEN define for the correct size and use it where required.
>
> Thanks,
>
> C.
>
> Changes in v2:
>   - removal of UUID_FMT_LEN
>
> Cédric Le Goater (3):
>    util/uuid: Add UUID_STR_LEN definition
>    vfio/pci: Fix buffer overrun when writing the VF token
>    util/uuid: Remove UUID_FMT_LEN
>
>   include/qemu/uuid.h              | 2 +-
>   block/parallels-ext.c            | 2 +-
>   block/vdi.c                      | 2 +-
>   hw/core/qdev-properties-system.c | 2 +-
>   hw/hyperv/vmbus.c                | 4 ++--
>   hw/vfio/pci.c                    | 2 +-
>   migration/savevm.c               | 4 ++--
>   tests/unit/test-uuid.c           | 2 +-
>   util/uuid.c                      | 2 +-
>   9 files changed, 11 insertions(+), 11 deletions(-)
>
Reviwed-by: Denis V. Lunev <den@openvz.org>

Re: [PATCH v2 0/3] vfio/pci: Fix buffer overrun when writing the VF token
Posted by Cédric Le Goater 1 year, 1 month ago
On 10/26/23 10:41, Denis V. Lunev wrote:
> On 10/26/23 09:06, Cédric Le Goater wrote:
>> Hello,
>>
>> This series fixes a buffer overrun in VFIO. The buffer used in
>> vfio_realize() by qemu_uuid_unparse() is too small, UUID_FMT_LEN lacks
>> one byte for the trailing NUL.
>>
>> Instead of adding + 1, as done elsewhere, the changes introduce a
>> UUID_STR_LEN define for the correct size and use it where required.
>>
>> Thanks,
>>
>> C.
>>
>> Changes in v2:
>>   - removal of UUID_FMT_LEN
>>
>> Cédric Le Goater (3):
>>    util/uuid: Add UUID_STR_LEN definition
>>    vfio/pci: Fix buffer overrun when writing the VF token
>>    util/uuid: Remove UUID_FMT_LEN
>>
>>   include/qemu/uuid.h              | 2 +-
>>   block/parallels-ext.c            | 2 +-
>>   block/vdi.c                      | 2 +-
>>   hw/core/qdev-properties-system.c | 2 +-
>>   hw/hyperv/vmbus.c                | 4 ++--
>>   hw/vfio/pci.c                    | 2 +-
>>   migration/savevm.c               | 4 ++--
>>   tests/unit/test-uuid.c           | 2 +-
>>   util/uuid.c                      | 2 +-
>>   9 files changed, 11 insertions(+), 11 deletions(-)
>>
> Reviwed-by: Denis V. Lunev <den@openvz.org>

I changed that to "Reviewed-by".

Interesting to see that b4 was ok with this new tag.

Thanks,

C.


Re: [PATCH v2 0/3] vfio/pci: Fix buffer overrun when writing the VF token
Posted by Konstantin Ryabitsev 1 year, 1 month ago
October 26, 2023 at 5:58 AM, "Cédric Le Goater" <clg@redhat.com> wrote:
> >  Reviwed-by: Denis V. Lunev <den@openvz.org>
> > 
> 
> I changed that to "Reviewed-by".
> 
> Interesting to see that b4 was ok with this new tag.

When we see an email address in the trailer contents, we don't check it against a known-trailers list, because there are just too many things like "Co-developed-by", "Reviewed-and-acked-by", etc. We could add some kind of logic to break these apart and compare individual parts to a list of known person-trailers (e.g. ["co", "reviewed", "developed", "and", "by", ...]), but we currently don't, which is why typos like this one sneak through.

-K
Re: [PATCH v2 0/3] vfio/pci: Fix buffer overrun when writing the VF token
Posted by Peter Maydell 1 year, 1 month ago
On Thu, 26 Oct 2023 at 16:06, Konstantin Ryabitsev
<konstantin.ryabitsev@linux.dev> wrote:
>
> October 26, 2023 at 5:58 AM, "Cédric Le Goater" <clg@redhat.com> wrote:
> > >  Reviwed-by: Denis V. Lunev <den@openvz.org>
> > >
> >
> > I changed that to "Reviewed-by".
> >
> > Interesting to see that b4 was ok with this new tag.
>
> When we see an email address in the trailer contents, we don't check it against a known-trailers list, because there are just too many things like "Co-developed-by", "Reviewed-and-acked-by", etc. We could add some kind of logic to break these apart and compare individual parts to a list of known person-trailers (e.g. ["co", "reviewed", "developed", "and", "by", ...]), but we currently don't, which is why typos like this one sneak through.

From the QEMU development perspective, I would be mildly in favour
of having checkpatch at least warn about unusual trailers, because
I don't think the profusion of oddball stuff is actually helpful,
and nudging towards standardization and guarding against typos in
the tag string would be better (eg if you want to do both a review
and an ack, provide both tags, not an -and- tag that no tooling
that might be asking "did this get review?" will be looking for).
But I don't care enough to have actually looked at getting our
checkpatch to do this :-)

-- PMM