linux-kernel - Re: [PATCH] Cleanup: rationalize paravirt wrappers

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]

Message-id: <460239C3.5000404@qumranet.com>
Date:	Thu, 22 Mar 2007 10:09:39 +0200
From:	Avi Kivity <avi@...ranet.com>
To:	Rusty Russell <rusty@...tcorp.com.au>
Cc:	Andrew Morton <akpm@...l.org>,
	Jeremy Fitzhardinge <jeremy@...p.org>,
	lkml - Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Andi Kleen <ak@....de>
Subject: Re: [PATCH] Cleanup: rationalize paravirt wrappers

Rusty Russell wrote:
>>
>>> +#define rdmsr(msr,val1,val2)						\
>>> +	do {								\
>>> +		int __err;						\
>>> +		unsigned long long __val = native_read_msr(msr, &__err);	\
>>> +		val1 = __val;						\
>>> +		val2 = __val >> 32;					\
>>> +	} while(0)
>>> +
>>>   
>>>       
>> You're silently changing the behavior (as well as the prototype) here,  
>> rdmsr() used to fail loudly, now it fails silently, with no way for the 
>> caller to check.
>>     
>
> Hi Avi!
>
> 	Not sure what you're saying about the prototype: the old macro was:
>
> -#define rdmsr(msr,val1,val2) \
> -	__asm__ __volatile__("rdmsr" \
> -			  : "=a" (val1), "=d" (val2) \
> -			  : "c" (msr))
> -
>
> Which doesn't seem to be a great difference to me.
>   

Right, scratch that.  Was confused by rdmsr_safe().

> The behaviour change (don't oops when an invalid rdmsr is used) was
> there with CONFIG_PARAVIRT=y, the cleanup just made !CONFIG_PARAVIRT the
> same.  Is it important?

I think so.  A function should either never fail, or indicate that it 
has failed (panic, error return, debug message).  With the kinder, 
gentler rdmsr() one can code the wrong msr value and not notice that 
something is wrong.


-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.

-
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/