Re: [Qemu-devel] [PATCH] pci-pc: add NULL check for qpci_free_pc

[Stefan Hajnoczi]

On Mon, Jul 23, 2018 at 12:43:42PM +0200, Emanuele Giuseppe Esposito wrote:
> The current layout of struct QPCIBusPC provides only one field,
> QPCIBus bus, so passing a NULL pointer to qpci_free_pc()
> makes container_of(NULL, QPCIBusPC, bus)
> returning 0 (NULL), that is correctly handled by g_free().
> This is bad practice, allowing the caller to think that it's okay
> to always pass NULL to the function, even though this is just a
> particular case.
> In facts, qpci_free_pc() happens to return NULL only because container_of
> computes the subtraction between the given NULL
> pointer and offsetof(QPCIBus, bus), with the latter returning 0 too,
> since bus is the first element of the struct and there is no
> offset betwen itself and QPCIBusPC.
> 
> However, if in future the bus field changes its position, for example becoming
> the second field, offsetof will return a number > 0, since there is some 
> offset
> between the beginning of the struct and the bus field.
> Therefore passing a NULL pointer to the container_of macro will return a
> negative number, that will be translated into an invalid address passed to
> g_free() and causing a seg fault.
> 
> Adding a preventive safety check that returns from the function if the
> given pointer is NULL solves the problem.
> 
> Signed-off-by: Emanuele Giuseppe Esposito <address@hidden>
> ---
>  tests/libqos/pci-pc.c | 4 ++++
>  1 file changed, 4 insertions(+)

Reviewed-by: Stefan Hajnoczi <address@hidden>

signature.asc
Description: PGP signature