[syslinux] [PATCH] pci: resize pci_device arrays

[H. Peter Anvin]

Sebastian Herbszt wrote:
> 
> I thought of this patch as one which does "just fix" the current overflow problems.
> Since it's simple and doesn't require any "big" code changes it can be done fast so
> syslinux doesn't ship with those overflow problems any longer. Later we can change
> the pci code to use your pci_device[PCI_DEVICE][FUNC] array structure or linked
> lists.
> 
>> Also, uint8_t for "count" is unsafe since we can have up to 256 devices. 
> 
> uint8_t count can hold values from 0 till 255 and the array can have 256 elements
> (0 till 255) so it should be ok.

No... think about it.  Valid counts are from 0 (no devices) to 256 
(every possible device) *inclusive*.

Furthermore, due to alignment issues there is no reason not to just use 
"int" (or "unsigned int") here.

>>  On the other hand, if we use a geographical (uncompacted) 
>> representation, perhaps the count is unnecessary.
>>
>>>  struct pci_device_list {
>>> - struct pci_device pci_device[MAX_PCI_DEVICES];
>>> + struct pci_device pci_device[MAX_PCI_BUSES * MAX_PCI_DEVICES * MAX_PCI_FUNC];
>>>  uint8_t count;
>>>  };
>> Using uint8_t for "count" here is even more wrong since we can have up 
>> to 65536 devices.
> 
> Here you are correct, I have missed this. uint16_t should be enough here, right?
> This likely also requires changes to scan.c and maybe some users of pci_device_list.

Again, valid values are from 0-65536, inclusive.  Again, due to 
alignment issues we might just as well use "int".

>> Also, each structure like this is over a megabyte in size.  How many of 
>> them will we *ever* have at the same time?
> 
> The current code only has one, but this array should be changed in the future (s.a).
> 
> Should i respin this patch with the missing changes or do you consider this approach not valid?

If it is only one, then it's good enough for now, and I'm keen on 
getting a low-risk fix for 3.71, so please respin.

	-hpa