[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH RESEND 1/3] vfio/pci: fix a null pointer reference in vfio_ro

From: Laszlo Ersek
Subject: Re: [PATCH RESEND 1/3] vfio/pci: fix a null pointer reference in vfio_rom_read
Date: Wed, 11 Mar 2020 11:28:26 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1

On 03/11/20 08:08, Markus Armbruster wrote:
> Alex Williamson <address@hidden> writes:
>> On Wed, 11 Mar 2020 00:14:31 +0100
>> Laszlo Ersek <address@hidden> wrote:
> [...]
>>> So from a memcpy() and range perspective, the patch looks OK. But
>>> there's still a wart I dislike: we should never perform pointer
>>> arithmetic on a (void*). I suggest casting (vdev->rom) to (uint8_t*) or
>>> (unsigned char*) first.
>>> Here's an excerpt from the ISO C99 standard:
>>> -v-
>>> 6.5.6 Additive operators
>>> Constraints
>>> 2 For addition, either both operands shall have arithmetic type, or one
>>>   operand shall be a pointer to an object type and the other shall have
>>>   integer type. [...]
>>> -^-
>>> A "pointer-to-void" is not a "pointer to an object type", because "void"
>>> is not an object type -- it is an incomplete type that cannot be completed:
>>> -v-
>>> 6.2.5 Types
>>> 1 [...] Types are partitioned into object types (types that fully
>>>   describe objects), function types (types that describe functions), and
>>>   incomplete types (types that describe objects but lack information
>>>   needed to determine their sizes).
>>> [...]
>>> 19 The void type comprises an empty set of values; it is an incomplete
>>>    type that cannot be completed.
>>> -^-
>>> For a different illustration, (vdev->rom + addr) is equivalent to
>>> &(vdev->rom[addr]) -- and we clearly can't have an "array of void".
>>> This anti-pattern (of doing pointer arithmetic on (void*)) likely comes
>>> from a guarantee that the standard does make, in the same "6.2.5 Types"
>>> section:
>>> -v-
>>> 27 A pointer to void shall have the same representation and alignment
>>>    requirements as a pointer to a character type. 39) [...]
>>> Footnote 39: The same representation and alignment requirements are
>>>              meant to imply interchangeability as arguments to
>>>              functions, return values from functions, and members of
>>>              unions.
>>> -^-
>>> It does not extend to the "+" operator.
>> GNU C specifically allows arithmetic on pointers and defines the size
>> of a void as 1.  I'll comply, but this makes me want to stab myself in
>> the face :-\  Thanks,
> We rely on GNU C extensions all over theplace.  Making the code uglier
> to avoid relying on this one here makes no sense to me.

I agree, in fact. If GNU-isms are liberally used & tolerated in the QEMU
source, then there's no reason to diverge from that here. I steer clear
of GNU-isms as much as I can, regardless of codebase, but I *did* forget
that QEMU permits GNU-isms -- so there's no need for my pedantry here.

Reviewed-by: Laszlo Ersek <address@hidden>


reply via email to

[Prev in Thread] Current Thread [Next in Thread]