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 <e.emanuelegiuse...@gmail.com> > --- > tests/libqos/pci-pc.c | 4 ++++ > 1 file changed, 4 insertions(+)
Reviewed-by: Stefan Hajnoczi <stefa...@redhat.com>
signature.asc
Description: PGP signature
