Messages in this thread | | | From | Linus Torvalds <> | Date | Mon, 21 Mar 2022 12:17:07 -0700 | Subject | Re: [GIT pull] x86/irq for v5.18-rc1 |
| |
On Mon, Mar 21, 2022 at 4:02 AM Thomas Gleixner <tglx@linutronix.de> wrote: > > - Handle the IRT routing table format in AMI BIOSes correctly
*Very* minor nit here in the hope of future cleanups: the other x86 irq routing table structions (Christ, that's a sentence that shouldn't exist in a sane world) use "__attribute__((packed))" and this one uses "__packed".
They are all right next to each other, maybe they could be made to have the same syntax?
HOWEVER.
That's not what the problem with this pull is.
I pulled this and then I unpulled it.
Because that stupid IRT routing table code already been reported to cause bugs:
https://lore.kernel.org/all/a2791312-2957-27e6-43af-c805bbb90266@collabora.com/
which seems to be because the $IRT signature check is complete garbage:
> + for (addr = (u8 *)__va(0xf0000); addr < (u8 *)__va(0x100000); addr++) { > + rt = pirq_convert_irt_table(addr); > + if (rt) > + return rt;
The above doesn't seem like it could really ever have been tested properly, since it will walk off the end of that __va(0x100000) address: it will walk every byte up to the 1MB physical address, and it will try to find that $IRT signature there, but if it never finds it, IT WILL CHECK THE SIGNATURE PAST THE 1MB mark!
So I refuse to pull this, and it should never have been sent to me, considering that it had a known bug, and it took me only moments to see how completely wrong that code was.
The fix seems obvious (you don't walk every byte to 1M, you walk to 1M - the size of the struct, and then you also check that the number of entries actually fits - Dmitry can presumably test), but no way do I want to get this kind of clearly broken thing this merge window.
And yes, I'm unhappy. This bug was reported a week ago. This should not have been sent to me today.
I also assume and suspect that the $IRT format isn't even used in modern PC's, so this must be some really odd special legacy case that very few people can care about. No?
Linus
| |