On Tue, Jan 24, 2023 at 02:42:11PM +0200, Alexander Shishkin wrote: > Leon Romanovsky <leon@xxxxxxxxxx> writes: > > > On Tue, Jan 24, 2023 at 01:52:37PM +0200, Alexander Shishkin wrote: > >> Leon Romanovsky <leon@xxxxxxxxxx> writes: > >> > >> > I'm not security expert here, but not sure that this protects from anything. > >> > 1. Kernel relies on working and not-malicious HW. There are gazillion ways > >> > to cause crashes other than changing MSI-X. > >> > >> This particular bug was preventing our fuzzing from going deeper into > >> the code and reaching some more of the aforementioned gazillion bugs. > > > > Your commit message says nothing about fuzzing, but talks about > > malicious device. > > A malicious device is what the fuzzing is aiming to simulate. The fact > of fuzzing process itself didn't seem relevant to the patch, so I didn't > include it, going instead for the problem statement and proposed > solution. Will the commit message benefit from mentioning fuzzing? No, for most if not all kernel developers, the fuzzing means some sort of random user-space input. PCI devices are trusted in the kernel. > > > Do you see "gazillion bugs" for devices which don't change their MSI-X > > table size under the hood, which is main kernel assumption? > > Not so far. So please share them with us. > > > If yes, you should fix these bugs. > > That's absolutely the intention. So let's fix the bugs and not hide them. > > >> > 2. Device can report large table size, kernel will cache it and > >> > malicious device will reduce it back. It is not handled and will cause > >> > to kernel crash too. > >> > >> How would that happen? If the device decides to have fewer vectors, > >> they'll all still fit in the ioremapped MSIX table. The worst thing that > >> can happen is 0xffffffff reads from the mmio space, which a device can > >> do anyway. But that shouldn't trigger a page fault or otherwise > >> crash. Or am I missing something? > > > > Like I said, I'm no expert. You should tell me if it safe for all > > callers of pci_msix_vec_count(). > > Well, since you stated that the reverse will cause a kernel crash, I had > to ask how. I'll include some version of the above paragraph in the > commit message to indicate that we reverse situation has been considered. Not really. I didn't see any explanation how will it work if number of vectors (which MSI-X table represents) is completely different from seeing by PCI core. Thanks > > Regards, > -- > Alex
