linux-kernel - Re: [PATCH] BUG in pv_clock when overflow condition is detected

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:	Fri, 17 Feb 2012 16:25:04 +0100
From:	Igor Mammedov <imammedo@...hat.com>
To:	Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>
CC:	Avi Kivity <avi@...hat.com>, linux-kernel@...r.kernel.org,
	kvm@...r.kernel.org, tglx@...utronix.de, mingo@...hat.com,
	hpa@...or.com, riel@...hat.com, amit shah <amit.shah@...hat.com>,
	mtosatti@...hat.com, xen-devel@...ts.xensource.com,
	Ian.Campbell@...rix.com
Subject: Re: [PATCH] BUG in pv_clock when overflow condition is detected

On 02/16/2012 03:03 PM, Avi Kivity wrote:
> On 02/15/2012 07:18 PM, Igor Mammedov wrote:
>>> On 02/15/2012 01:23 PM, Igor Mammedov wrote:
>>>>>>    static u64 pvclock_get_nsec_offset(struct pvclock_shadow_time
>>>>>> *shadow)
>>>>>>    {
>>>>>> -    u64 delta = native_read_tsc() - shadow->tsc_timestamp;
>>>>>> +    u64 delta;
>>>>>> +    u64 tsc = native_read_tsc();
>>>>>> +    BUG_ON(tsc<   shadow->tsc_timestamp);
>>>>>> +    delta = tsc - shadow->tsc_timestamp;
>>>>>>        return pvclock_scale_delta(delta, shadow->tsc_to_nsec_mul,
>>>>>>                       shadow->tsc_shift);
>>>>>
>>>>> Maybe a WARN_ON_ONCE()?  Otherwise a relatively minor hypervisor
>>>>> bug can
>>>>> kill the guest.
>>>>
>>>>
>>>> An attempt to print from this place is not perfect since it often
>>>> leads
>>>> to recursive calling to this very function and it hang there
>>>> anyway.
>>>> But if you insist I'll re-post it with WARN_ON_ONCE,
>>>> It won't make much difference because guest will hang/stall due
>>>> overflow
>>>> anyway.
>>>
>>> Won't a BUG_ON() also result in a printk?
>> Yes, it will. But stack will still keep failure point and poking
>> with crash/gdb at core will always show where it's BUGged.
>>
>> In case it manages to print dump somehow (saw it couple times from ~
>> 30 test cycles), logs from console or from kernel message buffer
>> (again poking with gdb) will show where it was called from.
>>
>> If WARN* is used, it will still totaly screwup clock and
>> "last value" and system will become unusable, requiring looking with
>> gdb/crash at the core any way.
>>
>> So I've just used more stable failure point that will leave trace
>> everywhere it manages (maybe in console log, but for sure in stack)
>> in case of WARN it might leave trace on console or not and probably
>> won't reflect failure point in stack either leaving only kernel
>> message buffer for clue.
>>
>
> Makes sense.  But do get an ack from the Xen people to ensure this
> doesn't break for them.
>
Konrad, Ian

Could you please review patch form point of view of xen?
Whole thread could be found here https://lkml.org/lkml/2012/2/13/286

-- 
Thanks,
  Igor
--
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/