linux-kernel - Re: [git pull] timer fix

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 4 Feb 2009 23:25:43 +0100
From:	Ingo Molnar <mingo@...e.hu>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	linux-kernel@...r.kernel.org,
	Andrew Morton <akpm@...ux-foundation.org>,
	Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [git pull] timer fix


* Linus Torvalds <torvalds@...ux-foundation.org> wrote:

> On Wed, 4 Feb 2009, Ingo Molnar wrote:
> >
> > Pavel Emelyanov (1):
> >       x86: fix hpet timer reinit for x86_64
> > 
> > 
> >  arch/x86/kernel/hpet.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> > 
> > diff --git a/arch/x86/kernel/hpet.c b/arch/x86/kernel/hpet.c
> > index 64d5ad0..ec319d1 100644
> > --- a/arch/x86/kernel/hpet.c
> > +++ b/arch/x86/kernel/hpet.c
> > @@ -1075,7 +1075,7 @@ static void hpet_rtc_timer_reinit(void)
> >  		hpet_t1_cmp += delta;
> >  		hpet_writel(hpet_t1_cmp, HPET_T1_CMP);
> >  		lost_ints++;
> > -	} while ((long)(hpet_readl(HPET_COUNTER) - hpet_t1_cmp) > 0);
> > +	} while ((long)(hpet_readl(HPET_COUNTER) - (u32)hpet_t1_cmp) > 0);
> 
> This is bordering on not being correct.

yeah, i had to look twice. The only reason i left it that way was because i 
couldnt reproduce the problem and hpet is hellishly fragile and this patch 
was tested so i chickened out.

OTOH that fragility is partly because such constructs have piled up so you 
very much have a valid point ...

We'll clean this up. I've already added the clean 32-bit casts - which also 
has another advantage: it does not actually trust the hw to always return 
32-bit values - it explicitly cuts to 32 bits and does signed arithmetics on 
that. Will also do the helper function cleanup to abstract the counter 
arithmetics away.

> In particular, think about when HPET_COUNTER or hpet_t1_cmp overflows in 
> 32 bits, and what you want to happen. If you do the subtract add test in 
> 64 bits, it will simply do the wrong thing. Think what happens if 
> hpet_t1_cmp is actually _larger_ than HPET_COUNTER, but overflowed in 32 
> bits, and you're now looking at:
> 
> 	(long) (0xffffffff - 0x00000001)
> 
> which is actually > 0, so the thing will continue to loop INCORRECTLY. It 
> should have stopped (and _would_ have stopped on 32-bit x86).

yeah, allowing that to happen is just wrong.

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/