linux-kernel - Re: [Xen-devel] [PATCH] PVH: vcpu info placement, load selectors, and remove debug printk.

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: <20130605121706.3a8211d6@mantra.us.oracle.com>
Date:	Wed, 5 Jun 2013 12:17:06 -0700
From:	Mukesh Rathor <mukesh.rathor@...cle.com>
To:	"Jan Beulich" <JBeulich@...e.com>
Cc:	"xen-devel" <xen-devel@...ts.xen.org>, <konrad.wilk@...cle.com>,
	<linux-kernel@...r.kernel.org>
Subject: Re: [Xen-devel] [PATCH] PVH: vcpu info placement, load selectors,
 and remove debug printk.

On Wed, 05 Jun 2013 08:03:12 +0100
"Jan Beulich" <JBeulich@...e.com> wrote:

> >>> On 04.06.13 at 23:53, Mukesh Rathor <mukesh.rathor@...cle.com>
> >>> wrote:
> > Following OK? :
> > 
> >         if (xen_feature(XENFEAT_auto_translated_physmap)) {
> >                 switch_to_new_gdt(0);
> > 
> >                 asm volatile (
> >                         "pushq %%rax\n"
> >                         "leaq 1f(%%rip),%%rax\n"
> >                         "pushq %%rax\n"
> >                         "lretq\n"
> >                         "1:\n"
> >                         : : "a" (__KERNEL_CS) : "memory");
> > 
> >                 return;
> >         }
> 
> While generally the choice of using %%rax instead of %0 here is
> a matter of taste to some degree, I still don't see why you can't
> use "r" as the constraint here in the first place.

The compiler mostly picks eax anyways, but good suggestion.

> Furthermore, assuming this sits in a function guaranteed to not be
> inlined, this has a latent bug (and if the assumption isn't right, the
> bug is real) in that the asm() modifies %rax without telling the
> compiler.

According to one of the unofficial asm tutorials i've here, the compiler
knows since it's input and doesn't need to be told. In fact
it'll barf if added to clobber list.

> This is how I would have done it:
> 
> 		unsigned long dummy;
> 
> 		asm volatile ("pushq %0\n"
> 			      "leaq 1f(%%rip),%0\n"
> 			      "pushq %0\n"
> 			      "lretq\n"
> 			      "1:\n"
> 			      : "=&r" (dummy) : "0" (__KERNEL_CS));
> 

Looks good. Thanks,
Mukesh

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