On Tue, Jan 24, 2023 at 02:42:11PM +0200, Alexander Shishkin wrote:
Leon Romanovsky leon@kernel.org writes:
On Tue, Jan 24, 2023 at 01:52:37PM +0200, Alexander Shishkin wrote:
Leon Romanovsky leon@kernel.org writes:
I'm not security expert here, but not sure that this protects from anything.
- 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.
- 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