Uninitialized next pointer for pci device may crash kernel

[rick-masters]

When a pci device structure is added to the new linked list implementation its next pointer may be uninitialized. It should be set to NULL.

A new struct pci_device is initialized from a passed in variable pci_dev:

Fiwix/drivers/pci/pci.c

Line 99 in d1a53ab

*pdt = *pci_dev;

However, that structure is allocated on the stack in scan_bus so some members may be uninitialized:

Fiwix/drivers/pci/pci.c

Lines 112 to 117 in d1a53ab

	static void scan_bus(void)
	{
	int b, d, f;
	unsigned int vendor_id, device_id, class;
	unsigned char header, irq, prog_if;
	struct pci_device pci_dev;

If pci_dev has a non-NULL next pointer, the kernel may attempt to access that memory which may be invalid and may crash the kernel.

Whether the crash is reproducible depends on the contents of stack memory which depends on many factors, so I have no easy way of providing a test case that triggers the problem. It always crashes for me and setting next to NULL fixes it.

[mikaku]

Good point.

I think that this line:

Fiwix/drivers/pci/pci.c

Line 91 in d1a53ab

memset_b(pdt, 0, sizeof(struct pci_device));

is useless because pdt is clobbered by the contents of pci_dev some lines below.

I think that instead of setting pdt->next = NULL; we could remove the line 91 and put a similar line inside the scan_bus() loop. So, we make sure that pci_dev is properly initialized from the beginning.

What do you think?

[mikaku]

BTW, It's amazing how you are viewing this code at the same time I was. Today I've been precisely touching these PCI functions. 😃

[rick-masters]

Yes, initializing pci_dev sounds like a better solution.

[mikaku]

I think there are still other bugs because pci_get_device() enters in an infinite loop when it doesn't found the vendor and device ids.

[rick-masters]

Hmm, looks to me like it should exit the while loop when it reaches the end of the list.

[mikaku]

Thank you.