Messages in this thread | | | From | Thomas Gleixner <> | Subject | Re: [PATCH 113/190] Revert "x86/hpet: Prevent potential NULL pointer dereference" | Date | Fri, 23 Apr 2021 01:33:07 +0200 |
| |
On Wed, Apr 21 2021 at 12:49, Kees Cook wrote: > On Wed, Apr 21, 2021 at 02:59:48PM +0200, Greg Kroah-Hartman wrote: >> diff --git a/arch/x86/kernel/hpet.c b/arch/x86/kernel/hpet.c >> index 08651a4e6aa0..0515a97bf6f5 100644 >> --- a/arch/x86/kernel/hpet.c >> +++ b/arch/x86/kernel/hpet.c >> @@ -930,8 +930,6 @@ int __init hpet_enable(void) >> return 0; >> >> hpet_set_mapping(); >> - if (!hpet_virt_address) >> - return 0; >> >> /* Validate that the config register is working */ >> if (!hpet_cfg_working()) > > FWIW, this patch looks harmless. It is checking for a failure in > hpet_set_mapping(), and avoids the following code from performing > 0-offset reads. hpet_set_mapping() is likely to never fail in real-world > situations. *shrug*
'likely never to fail' is clearly a receipe for disaster and you should know that.
> I think it would make more sense for the check to live in > hpet_cfg_working(), though.
No. That does not make any sense at all.
The proper change would have been to make hpet_set_mapping() return an error/success code and act on that.
But that does _NOT_ make the patch invalid.
I'm pretty sure that I looked at it and thought about the proper solution (see above) and then shrugged it off because of overload...
Thanks,
tglx
| |