[PATCH] xen/kexec: Clear unused registers before jumping into an image

[PATCH] xen/kexec: Clear unused registers before jumping into an image

[Daniel Kiper]

Clear unused registers before jumping into an image. This way
loaded image could not assume that any register has an specific
info about earlier running Xen hypervisor. However, it also
does not mean that the image may expect that a given register
is zeroed. The image MUST assume that every register has a random
value or in other words it is uninitialized or has undefined state.

Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
---
 xen/arch/x86/x86_64/kexec_reloc.S |   37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/xen/arch/x86/x86_64/kexec_reloc.S b/xen/arch/x86/x86_64/kexec_reloc.S
index 7a16c85..e7eef79 100644
--- a/xen/arch/x86/x86_64/kexec_reloc.S
+++ b/xen/arch/x86/x86_64/kexec_reloc.S
@@ -71,6 +71,29 @@ identity_mapped:
         jnz     call_32_bit
 
 call_64_bit:
+        /*
+         * Clear unused registers before jumping into an image. This way
+         * loaded image could not assume that any register has an specific
+         * info about earlier running Xen hypervisor. However, it also
+         * does not mean that the image may expect that a given register
+         * is zeroed. The image MUST assume that every register has a random
+         * value or in other words it is uninitialized or has undefined state.
+         */
+        xorl    %eax, %eax
+        xorl    %ebx, %ebx
+        xorl    %ecx, %ecx
+        xorl    %edx, %edx
+        xorl    %esi, %esi
+        xorl    %edi, %edi
+        xorl    %r8d, %r8d
+        xorl    %r9d, %r9d
+        xorl    %r10d, %r10d
+        xorl    %r11d, %r11d
+        xorl    %r12d, %r12d
+        xorl    %r13d, %r13d
+        xorl    %r14d, %r14d
+        xorl    %r15d, %r15d
+
         /* Call the image entry point.  This should never return. */
         callq   *%rbp
         ud2
@@ -164,6 +187,20 @@ compatibility_mode:
         xorl    %eax, %eax
         movl    %eax, %cr4
 
+        /*
+         * Clear unused registers before jumping into an image. This way
+         * loaded image could not assume that any register has an specific
+         * info about earlier running Xen hypervisor. However, it also
+         * does not mean that the image may expect that a given register
+         * is zeroed. The image MUST assume that every register has a random
+         * value or in other words it is uninitialized or has undefined state.
+         */
+        xorl    %ebx, %ebx
+        xorl    %ecx, %ecx
+        xorl    %edx, %edx
+        xorl    %esi, %esi
+        xorl    %edi, %edi
+
         /* Call the image entry point.  This should never return. */
         call    *%ebp
         ud2
-- 
1.7.10.4


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH] xen/kexec: Clear unused registers before jumping into an image

[David Vrabel]

On 15/11/13 15:56, Daniel Kiper wrote:
> Clear unused registers before jumping into an image. This way
> loaded image could not assume that any register has an specific
> info about earlier running Xen hypervisor. However, it also
> does not mean that the image may expect that a given register
> is zeroed. The image MUST assume that every register has a random
> value or in other words it is uninitialized or has undefined state.

I think this, where the specification (registers undefined) differs from
the implementation (registers zeroed), is the worst option.

I also think it is more likely for an image to inadvertently rely on a
zero value that whatever junk Xen has left behind.

David

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH] xen/kexec: Clear unused registers before jumping into an image

[Daniel Kiper]

On Fri, Nov 15, 2013 at 08:07:02PM +0000, David Vrabel wrote:
> On 15/11/13 15:56, Daniel Kiper wrote:
> > Clear unused registers before jumping into an image. This way
> > loaded image could not assume that any register has an specific
> > info about earlier running Xen hypervisor. However, it also
> > does not mean that the image may expect that a given register
> > is zeroed. The image MUST assume that every register has a random
> > value or in other words it is uninitialized or has undefined state.
>
> I think this, where the specification (registers undefined) differs from
> the implementation (registers zeroed), is the worst option.
>
> I also think it is more likely for an image to inadvertently rely on a
> zero value that whatever junk Xen has left behind.

I do not agree with you but respect your opinion. So could you provide
a patch with a comment why our implementation deviate from our reference
implementation (I think about Linux one) and even we use kexec-tools
designed for Linux implementation which does things mentioned above?
I hope that this solve this last, widely discussed issue.

Daniel

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH] xen/kexec: Clear unused registers before jumping into an image

[Daniel Kiper]

On Fri, Nov 15, 2013 at 08:07:02PM +0000, David Vrabel wrote:
> On 15/11/13 15:56, Daniel Kiper wrote:
> > Clear unused registers before jumping into an image. This way
> > loaded image could not assume that any register has an specific
> > info about earlier running Xen hypervisor. However, it also
> > does not mean that the image may expect that a given register
> > is zeroed. The image MUST assume that every register has a random
> > value or in other words it is uninitialized or has undefined state.
>
> I think this, where the specification (registers undefined) differs from
> the implementation (registers zeroed), is the worst option.
>
> I also think it is more likely for an image to inadvertently rely on a
> zero value that whatever junk Xen has left behind.

I do not agree with you but respect your opinion. So could you provide
a patch with a comment why our implementation deviate from our reference
implementation (I think about Linux one) and even we use kexec-tools
designed for Linux implementation which does things mentioned above?
I hope that this solve this last, widely discussed issue.

Daniel

Re: [PATCH] xen/kexec: Clear unused registers before jumping into an image

[Jan Beulich]

>>> On 15.11.13 at 21:07, David Vrabel <david.vrabel@citrix.com> wrote:
> On 15/11/13 15:56, Daniel Kiper wrote:
>> Clear unused registers before jumping into an image. This way
>> loaded image could not assume that any register has an specific
>> info about earlier running Xen hypervisor. However, it also
>> does not mean that the image may expect that a given register
>> is zeroed. The image MUST assume that every register has a random
>> value or in other words it is uninitialized or has undefined state.
> 
> I think this, where the specification (registers undefined) differs from
> the implementation (registers zeroed), is the worst option.
> 
> I also think it is more likely for an image to inadvertently rely on a
> zero value that whatever junk Xen has left behind.

Preventing users to rely on anything would likely make it
desirable to put some random value into all unused registers.

Jan

Re: [PATCH] xen/kexec: Clear unused registers before jumping into an image

[Jan Beulich]

>>> On 15.11.13 at 21:07, David Vrabel <david.vrabel@citrix.com> wrote:
> On 15/11/13 15:56, Daniel Kiper wrote:
>> Clear unused registers before jumping into an image. This way
>> loaded image could not assume that any register has an specific
>> info about earlier running Xen hypervisor. However, it also
>> does not mean that the image may expect that a given register
>> is zeroed. The image MUST assume that every register has a random
>> value or in other words it is uninitialized or has undefined state.
> 
> I think this, where the specification (registers undefined) differs from
> the implementation (registers zeroed), is the worst option.
> 
> I also think it is more likely for an image to inadvertently rely on a
> zero value that whatever junk Xen has left behind.

Preventing users to rely on anything would likely make it
desirable to put some random value into all unused registers.

Jan


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH] xen/kexec: Clear unused registers before jumping into an image

[Daniel Kiper]

On Mon, Nov 18, 2013 at 09:29:41AM +0000, Jan Beulich wrote:
> >>> On 15.11.13 at 21:07, David Vrabel <david.vrabel@citrix.com> wrote:
> > On 15/11/13 15:56, Daniel Kiper wrote:
> >> Clear unused registers before jumping into an image. This way
> >> loaded image could not assume that any register has an specific
> >> info about earlier running Xen hypervisor. However, it also
> >> does not mean that the image may expect that a given register
> >> is zeroed. The image MUST assume that every register has a random
> >> value or in other words it is uninitialized or has undefined state.
> >
> > I think this, where the specification (registers undefined) differs from
> > the implementation (registers zeroed), is the worst option.
> >
> > I also think it is more likely for an image to inadvertently rely on a
> > zero value that whatever junk Xen has left behind.
>
> Preventing users to rely on anything would likely make it
> desirable to put some random value into all unused registers.

Right, but on the other hand this way we lose completely chance
to differentiate between old and new implementation of kexec
if we would like to do that in the future (yes, this is small
chance but it still exists). Additionally, I think it could be
quite difficult because at this stage there is no simple reliable
RNGs. Although there are some CPUs with RNGs but they are not
very common right now. However, I will do not object if we find
another simple RNG.

Daniel

Re: [PATCH] xen/kexec: Clear unused registers before jumping into an image

[Daniel Kiper]

On Mon, Nov 18, 2013 at 09:29:41AM +0000, Jan Beulich wrote:
> >>> On 15.11.13 at 21:07, David Vrabel <david.vrabel@citrix.com> wrote:
> > On 15/11/13 15:56, Daniel Kiper wrote:
> >> Clear unused registers before jumping into an image. This way
> >> loaded image could not assume that any register has an specific
> >> info about earlier running Xen hypervisor. However, it also
> >> does not mean that the image may expect that a given register
> >> is zeroed. The image MUST assume that every register has a random
> >> value or in other words it is uninitialized or has undefined state.
> >
> > I think this, where the specification (registers undefined) differs from
> > the implementation (registers zeroed), is the worst option.
> >
> > I also think it is more likely for an image to inadvertently rely on a
> > zero value that whatever junk Xen has left behind.
>
> Preventing users to rely on anything would likely make it
> desirable to put some random value into all unused registers.

Right, but on the other hand this way we lose completely chance
to differentiate between old and new implementation of kexec
if we would like to do that in the future (yes, this is small
chance but it still exists). Additionally, I think it could be
quite difficult because at this stage there is no simple reliable
RNGs. Although there are some CPUs with RNGs but they are not
very common right now. However, I will do not object if we find
another simple RNG.

Daniel

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH] xen/kexec: Clear unused registers before jumping into an image

[Jan Beulich]

>>> On 18.11.13 at 12:08, Daniel Kiper <daniel.kiper@oracle.com> wrote:
> On Mon, Nov 18, 2013 at 09:29:41AM +0000, Jan Beulich wrote:
>> >>> On 15.11.13 at 21:07, David Vrabel <david.vrabel@citrix.com> wrote:
>> > On 15/11/13 15:56, Daniel Kiper wrote:
>> >> Clear unused registers before jumping into an image. This way
>> >> loaded image could not assume that any register has an specific
>> >> info about earlier running Xen hypervisor. However, it also
>> >> does not mean that the image may expect that a given register
>> >> is zeroed. The image MUST assume that every register has a random
>> >> value or in other words it is uninitialized or has undefined state.
>> >
>> > I think this, where the specification (registers undefined) differs from
>> > the implementation (registers zeroed), is the worst option.
>> >
>> > I also think it is more likely for an image to inadvertently rely on a
>> > zero value that whatever junk Xen has left behind.
>>
>> Preventing users to rely on anything would likely make it
>> desirable to put some random value into all unused registers.
> 
> Right, but on the other hand this way we lose completely chance
> to differentiate between old and new implementation of kexec
> if we would like to do that in the future (yes, this is small
> chance but it still exists). Additionally, I think it could be
> quite difficult because at this stage there is no simple reliable
> RNGs. Although there are some CPUs with RNGs but they are not
> very common right now. However, I will do not object if we find
> another simple RNG.

We surely wouldn't need a good quality random number here -
the TSC would very likely already be more random than anything
we need.

Jan


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH] xen/kexec: Clear unused registers before jumping into an image

[Daniel Kiper]

On Mon, Nov 18, 2013 at 11:27:34AM +0000, Jan Beulich wrote:
> >>> On 18.11.13 at 12:08, Daniel Kiper <daniel.kiper@oracle.com> wrote:
> > On Mon, Nov 18, 2013 at 09:29:41AM +0000, Jan Beulich wrote:
> >> >>> On 15.11.13 at 21:07, David Vrabel <david.vrabel@citrix.com> wrote:
> >> > On 15/11/13 15:56, Daniel Kiper wrote:
> >> >> Clear unused registers before jumping into an image. This way
> >> >> loaded image could not assume that any register has an specific
> >> >> info about earlier running Xen hypervisor. However, it also
> >> >> does not mean that the image may expect that a given register
> >> >> is zeroed. The image MUST assume that every register has a random
> >> >> value or in other words it is uninitialized or has undefined state.
> >> >
> >> > I think this, where the specification (registers undefined) differs from
> >> > the implementation (registers zeroed), is the worst option.
> >> >
> >> > I also think it is more likely for an image to inadvertently rely on a
> >> > zero value that whatever junk Xen has left behind.
> >>
> >> Preventing users to rely on anything would likely make it
> >> desirable to put some random value into all unused registers.
> >
> > Right, but on the other hand this way we lose completely chance
> > to differentiate between old and new implementation of kexec
> > if we would like to do that in the future (yes, this is small
> > chance but it still exists). Additionally, I think it could be
> > quite difficult because at this stage there is no simple reliable
> > RNGs. Although there are some CPUs with RNGs but they are not
> > very common right now. However, I will do not object if we find
> > another simple RNG.
>
> We surely wouldn't need a good quality random number here -
> the TSC would very likely already be more random than anything
> we need.

I forgot about TSC. This is OK in that case. Thanks. Personally I prefer
zeroing (I explained above and in other emails why) but if David do not
like it we could use TSC. David?

Daniel

Re: [PATCH] xen/kexec: Clear unused registers before jumping into an image

[Daniel Kiper]

On Mon, Nov 18, 2013 at 11:27:34AM +0000, Jan Beulich wrote:
> >>> On 18.11.13 at 12:08, Daniel Kiper <daniel.kiper@oracle.com> wrote:
> > On Mon, Nov 18, 2013 at 09:29:41AM +0000, Jan Beulich wrote:
> >> >>> On 15.11.13 at 21:07, David Vrabel <david.vrabel@citrix.com> wrote:
> >> > On 15/11/13 15:56, Daniel Kiper wrote:
> >> >> Clear unused registers before jumping into an image. This way
> >> >> loaded image could not assume that any register has an specific
> >> >> info about earlier running Xen hypervisor. However, it also
> >> >> does not mean that the image may expect that a given register
> >> >> is zeroed. The image MUST assume that every register has a random
> >> >> value or in other words it is uninitialized or has undefined state.
> >> >
> >> > I think this, where the specification (registers undefined) differs from
> >> > the implementation (registers zeroed), is the worst option.
> >> >
> >> > I also think it is more likely for an image to inadvertently rely on a
> >> > zero value that whatever junk Xen has left behind.
> >>
> >> Preventing users to rely on anything would likely make it
> >> desirable to put some random value into all unused registers.
> >
> > Right, but on the other hand this way we lose completely chance
> > to differentiate between old and new implementation of kexec
> > if we would like to do that in the future (yes, this is small
> > chance but it still exists). Additionally, I think it could be
> > quite difficult because at this stage there is no simple reliable
> > RNGs. Although there are some CPUs with RNGs but they are not
> > very common right now. However, I will do not object if we find
> > another simple RNG.
>
> We surely wouldn't need a good quality random number here -
> the TSC would very likely already be more random than anything
> we need.

I forgot about TSC. This is OK in that case. Thanks. Personally I prefer
zeroing (I explained above and in other emails why) but if David do not
like it we could use TSC. David?

Daniel

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH] xen/kexec: Clear unused registers before jumping into an image

[Jan Beulich]

>>> On 18.11.13 at 12:08, Daniel Kiper <daniel.kiper@oracle.com> wrote:
> On Mon, Nov 18, 2013 at 09:29:41AM +0000, Jan Beulich wrote:
>> >>> On 15.11.13 at 21:07, David Vrabel <david.vrabel@citrix.com> wrote:
>> > On 15/11/13 15:56, Daniel Kiper wrote:
>> >> Clear unused registers before jumping into an image. This way
>> >> loaded image could not assume that any register has an specific
>> >> info about earlier running Xen hypervisor. However, it also
>> >> does not mean that the image may expect that a given register
>> >> is zeroed. The image MUST assume that every register has a random
>> >> value or in other words it is uninitialized or has undefined state.
>> >
>> > I think this, where the specification (registers undefined) differs from
>> > the implementation (registers zeroed), is the worst option.
>> >
>> > I also think it is more likely for an image to inadvertently rely on a
>> > zero value that whatever junk Xen has left behind.
>>
>> Preventing users to rely on anything would likely make it
>> desirable to put some random value into all unused registers.
> 
> Right, but on the other hand this way we lose completely chance
> to differentiate between old and new implementation of kexec
> if we would like to do that in the future (yes, this is small
> chance but it still exists). Additionally, I think it could be
> quite difficult because at this stage there is no simple reliable
> RNGs. Although there are some CPUs with RNGs but they are not
> very common right now. However, I will do not object if we find
> another simple RNG.

We surely wouldn't need a good quality random number here -
the TSC would very likely already be more random than anything
we need.

Jan

Re: [PATCH] xen/kexec: Clear unused registers before jumping into an image

[David Vrabel]

On 18/11/13 09:29, Jan Beulich wrote:
>>>> On 15.11.13 at 21:07, David Vrabel <david.vrabel@citrix.com> wrote:
>> On 15/11/13 15:56, Daniel Kiper wrote:
>>> Clear unused registers before jumping into an image. This way
>>> loaded image could not assume that any register has an specific
>>> info about earlier running Xen hypervisor. However, it also
>>> does not mean that the image may expect that a given register
>>> is zeroed. The image MUST assume that every register has a random
>>> value or in other words it is uninitialized or has undefined state.
>>
>> I think this, where the specification (registers undefined) differs from
>> the implementation (registers zeroed), is the worst option.
>>
>> I also think it is more likely for an image to inadvertently rely on a
>> zero value that whatever junk Xen has left behind.
> 
> Preventing users to rely on anything would likely make it
> desirable to put some random value into all unused registers.

I don't think we need to go that far.

I would just like to avoid someone looking that the implementation (and
not the documentation) and concluding that zero-ing of the registers is
part of the specified behaviour, or looking at the implementation and
documentation and wondering why they don't agree.

I really don't think there is anything more to be said on this.

David

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH] xen/kexec: Clear unused registers before jumping into an image

[Daniel Kiper]

On Mon, Nov 18, 2013 at 11:23:27AM +0000, David Vrabel wrote:
> On 18/11/13 09:29, Jan Beulich wrote:
> >>>> On 15.11.13 at 21:07, David Vrabel <david.vrabel@citrix.com> wrote:
> >> On 15/11/13 15:56, Daniel Kiper wrote:
> >>> Clear unused registers before jumping into an image. This way
> >>> loaded image could not assume that any register has an specific
> >>> info about earlier running Xen hypervisor. However, it also
> >>> does not mean that the image may expect that a given register
> >>> is zeroed. The image MUST assume that every register has a random
> >>> value or in other words it is uninitialized or has undefined state.
> >>
> >> I think this, where the specification (registers undefined) differs from
> >> the implementation (registers zeroed), is the worst option.
> >>
> >> I also think it is more likely for an image to inadvertently rely on a
> >> zero value that whatever junk Xen has left behind.
> >
> > Preventing users to rely on anything would likely make it
> > desirable to put some random value into all unused registers.
>
> I don't think we need to go that far.
>
> I would just like to avoid someone looking that the implementation (and
> not the documentation) and concluding that zero-ing of the registers is
> part of the specified behaviour, or looking at the implementation and
> documentation and wondering why they don't agree.

David, my comment clearly states why we are doing that and what should
be expected. What is wrong with it? I could improve it but say how?

Daniel

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH] xen/kexec: Clear unused registers before jumping into an image

[George Dunlap]

On 18/11/13 11:47, Daniel Kiper wrote:
> On Mon, Nov 18, 2013 at 11:23:27AM +0000, David Vrabel wrote:
>> On 18/11/13 09:29, Jan Beulich wrote:
>>>>>> On 15.11.13 at 21:07, David Vrabel <david.vrabel@citrix.com> wrote:
>>>> On 15/11/13 15:56, Daniel Kiper wrote:
>>>>> Clear unused registers before jumping into an image. This way
>>>>> loaded image could not assume that any register has an specific
>>>>> info about earlier running Xen hypervisor. However, it also
>>>>> does not mean that the image may expect that a given register
>>>>> is zeroed. The image MUST assume that every register has a random
>>>>> value or in other words it is uninitialized or has undefined state.
>>>> I think this, where the specification (registers undefined) differs from
>>>> the implementation (registers zeroed), is the worst option.
>>>>
>>>> I also think it is more likely for an image to inadvertently rely on a
>>>> zero value that whatever junk Xen has left behind.
>>> Preventing users to rely on anything would likely make it
>>> desirable to put some random value into all unused registers.
>> I don't think we need to go that far.
>>
>> I would just like to avoid someone looking that the implementation (and
>> not the documentation) and concluding that zero-ing of the registers is
>> part of the specified behaviour, or looking at the implementation and
>> documentation and wondering why they don't agree.
> David, my comment clearly states why we are doing that and what should
> be expected. What is wrong with it? I could improve it but say how?

You seem to be saying that the registers may contain useful information 
about Xen that are not part of the spec, and you are worried that the 
image may decide to use these and come to rely on them, making it hard 
to change the interface in the future.

David has a similar concern -- that if the registers are zeroed, the 
image may come to rely on the registers being pre-zeroed, and not zero 
them itself.  This would also make it hard to change the interface in 
the future.

A simple solution would be to "poison" the registers with useless data: 
0xdeadbeef is a favorite.  Anyone seeing that in the registers will 
immediately know, "Someone used a value that they shouldn't have."

Of course, it's *possible* that then the images might come to rely on 
that poison being there; having a non-deterministic value there, like a 
hash of the TSC, would make this impossible.  But even then, you could 
make the argument that the image may come to rely on a *pseudo-random* 
number being there, which it uses for some other kind of seed 
somewhere.  At some point you just have to give up on this like of 
thought. :-)

Personally I think having a poison is likely to be more useful -- if you 
crash because your pointer is 0xdeadbeef, then it's immediately obvious 
what kind of bug you have; whereas if you crash with a random value that 
changes every time, it's not so obvious.

  -George

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH] xen/kexec: Clear unused registers before jumping into an image

[Daniel Kiper]

On Mon, Nov 18, 2013 at 12:05:38PM +0000, George Dunlap wrote:
> On 18/11/13 11:47, Daniel Kiper wrote:
> >On Mon, Nov 18, 2013 at 11:23:27AM +0000, David Vrabel wrote:
> >>On 18/11/13 09:29, Jan Beulich wrote:
> >>>>>>On 15.11.13 at 21:07, David Vrabel <david.vrabel@citrix.com> wrote:
> >>>>On 15/11/13 15:56, Daniel Kiper wrote:
> >>>>>Clear unused registers before jumping into an image. This way
> >>>>>loaded image could not assume that any register has an specific
> >>>>>info about earlier running Xen hypervisor. However, it also
> >>>>>does not mean that the image may expect that a given register
> >>>>>is zeroed. The image MUST assume that every register has a random
> >>>>>value or in other words it is uninitialized or has undefined state.
> >>>>I think this, where the specification (registers undefined) differs from
> >>>>the implementation (registers zeroed), is the worst option.
> >>>>
> >>>>I also think it is more likely for an image to inadvertently rely on a
> >>>>zero value that whatever junk Xen has left behind.
> >>>Preventing users to rely on anything would likely make it
> >>>desirable to put some random value into all unused registers.
> >>I don't think we need to go that far.
> >>
> >>I would just like to avoid someone looking that the implementation (and
> >>not the documentation) and concluding that zero-ing of the registers is
> >>part of the specified behaviour, or looking at the implementation and
> >>documentation and wondering why they don't agree.
> >David, my comment clearly states why we are doing that and what should
> >be expected. What is wrong with it? I could improve it but say how?
>
> You seem to be saying that the registers may contain useful
> information about Xen that are not part of the spec, and you are
> worried that the image may decide to use these and come to rely on
> them, making it hard to change the interface in the future.
>
> David has a similar concern -- that if the registers are zeroed, the
> image may come to rely on the registers being pre-zeroed, and not
> zero them itself.  This would also make it hard to change the
> interface in the future.
>
> A simple solution would be to "poison" the registers with useless
> data: 0xdeadbeef is a favorite.  Anyone seeing that in the registers
> will immediately know, "Someone used a value that they shouldn't
> have."
>
> Of course, it's *possible* that then the images might come to rely
> on that poison being there; having a non-deterministic value there,
> like a hash of the TSC, would make this impossible.  But even then,
> you could make the argument that the image may come to rely on a
> *pseudo-random* number being there, which it uses for some other
> kind of seed somewhere.  At some point you just have to give up on
> this like of thought. :-)

:-)))

> Personally I think having a poison is likely to be more useful -- if
> you crash because your pointer is 0xdeadbeef, then it's immediately
> obvious what kind of bug you have; whereas if you crash with a
> random value that changes every time, it's not so obvious.

It works for me too. Any solution has pros and cons. However, in general
I think that wiping registers in that case is nice idea. So we could zero
registers, write 0xdeadbeef into them or even use TSC. But please do not
leave registers as is.

Daniel

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH] xen/kexec: Clear unused registers before jumping into an image

[Petr Tesarik]

On Mon, 18 Nov 2013 13:41:02 +0100
Daniel Kiper <daniel.kiper@oracle.com> wrote:

> On Mon, Nov 18, 2013 at 12:05:38PM +0000, George Dunlap wrote:
> > On 18/11/13 11:47, Daniel Kiper wrote:
> > >On Mon, Nov 18, 2013 at 11:23:27AM +0000, David Vrabel wrote:
> > >>On 18/11/13 09:29, Jan Beulich wrote:
> > >>>>>>On 15.11.13 at 21:07, David Vrabel <david.vrabel@citrix.com> wrote:
> > >>>>On 15/11/13 15:56, Daniel Kiper wrote:
> > >>>>>Clear unused registers before jumping into an image. This way
> > >>>>>loaded image could not assume that any register has an specific
> > >>>>>info about earlier running Xen hypervisor. However, it also
> > >>>>>does not mean that the image may expect that a given register
> > >>>>>is zeroed. The image MUST assume that every register has a random
> > >>>>>value or in other words it is uninitialized or has undefined state.
> > >>>>I think this, where the specification (registers undefined) differs from
> > >>>>the implementation (registers zeroed), is the worst option.
> > >>>>
> > >>>>I also think it is more likely for an image to inadvertently rely on a
> > >>>>zero value that whatever junk Xen has left behind.
> > >>>Preventing users to rely on anything would likely make it
> > >>>desirable to put some random value into all unused registers.
> > >>I don't think we need to go that far.
> > >>
> > >>I would just like to avoid someone looking that the implementation (and
> > >>not the documentation) and concluding that zero-ing of the registers is
> > >>part of the specified behaviour, or looking at the implementation and
> > >>documentation and wondering why they don't agree.
> > >David, my comment clearly states why we are doing that and what should
> > >be expected. What is wrong with it? I could improve it but say how?
> >
> > You seem to be saying that the registers may contain useful
> > information about Xen that are not part of the spec, and you are
> > worried that the image may decide to use these and come to rely on
> > them, making it hard to change the interface in the future.
> >
> > David has a similar concern -- that if the registers are zeroed, the
> > image may come to rely on the registers being pre-zeroed, and not
> > zero them itself.  This would also make it hard to change the
> > interface in the future.
> >
> > A simple solution would be to "poison" the registers with useless
> > data: 0xdeadbeef is a favorite.  Anyone seeing that in the registers
> > will immediately know, "Someone used a value that they shouldn't
> > have."
> >
> > Of course, it's *possible* that then the images might come to rely
> > on that poison being there; having a non-deterministic value there,
> > like a hash of the TSC, would make this impossible.  But even then,
> > you could make the argument that the image may come to rely on a
> > *pseudo-random* number being there, which it uses for some other
> > kind of seed somewhere.  At some point you just have to give up on
> > this like of thought. :-)
> 
> :-)))
> 
> > Personally I think having a poison is likely to be more useful -- if
> > you crash because your pointer is 0xdeadbeef, then it's immediately
> > obvious what kind of bug you have; whereas if you crash with a
> > random value that changes every time, it's not so obvious.
> 
> It works for me too. Any solution has pros and cons. However, in general
> I think that wiping registers in that case is nice idea. So we could zero
> registers, write 0xdeadbeef into them or even use TSC. But please do not
> leave registers as is.

DEADBABE or DEADBEEF sounds best. It even makes it possible to
distinguish between an old version and a new version if we ever need
to extend the interface and actually use a register to pass down
something useful to the boot loader.

Hmm, so maybe we could poison all registers, except e.g. RDI (1st
argument in the standard ABI), which could be zeroed and _documented_
as containing the interface version.

Petr T

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH] xen/kexec: Clear unused registers before jumping into an image

[George Dunlap]

On 18/11/13 13:13, Petr Tesarik wrote:
> On Mon, 18 Nov 2013 13:41:02 +0100
> Daniel Kiper <daniel.kiper@oracle.com> wrote:
>
>> On Mon, Nov 18, 2013 at 12:05:38PM +0000, George Dunlap wrote:
>>> On 18/11/13 11:47, Daniel Kiper wrote:
>>>> On Mon, Nov 18, 2013 at 11:23:27AM +0000, David Vrabel wrote:
>>>>> On 18/11/13 09:29, Jan Beulich wrote:
>>>>>>>>> On 15.11.13 at 21:07, David Vrabel <david.vrabel@citrix.com> wrote:
>>>>>>> On 15/11/13 15:56, Daniel Kiper wrote:
>>>>>>>> Clear unused registers before jumping into an image. This way
>>>>>>>> loaded image could not assume that any register has an specific
>>>>>>>> info about earlier running Xen hypervisor. However, it also
>>>>>>>> does not mean that the image may expect that a given register
>>>>>>>> is zeroed. The image MUST assume that every register has a random
>>>>>>>> value or in other words it is uninitialized or has undefined state.
>>>>>>> I think this, where the specification (registers undefined) differs from
>>>>>>> the implementation (registers zeroed), is the worst option.
>>>>>>>
>>>>>>> I also think it is more likely for an image to inadvertently rely on a
>>>>>>> zero value that whatever junk Xen has left behind.
>>>>>> Preventing users to rely on anything would likely make it
>>>>>> desirable to put some random value into all unused registers.
>>>>> I don't think we need to go that far.
>>>>>
>>>>> I would just like to avoid someone looking that the implementation (and
>>>>> not the documentation) and concluding that zero-ing of the registers is
>>>>> part of the specified behaviour, or looking at the implementation and
>>>>> documentation and wondering why they don't agree.
>>>> David, my comment clearly states why we are doing that and what should
>>>> be expected. What is wrong with it? I could improve it but say how?
>>> You seem to be saying that the registers may contain useful
>>> information about Xen that are not part of the spec, and you are
>>> worried that the image may decide to use these and come to rely on
>>> them, making it hard to change the interface in the future.
>>>
>>> David has a similar concern -- that if the registers are zeroed, the
>>> image may come to rely on the registers being pre-zeroed, and not
>>> zero them itself.  This would also make it hard to change the
>>> interface in the future.
>>>
>>> A simple solution would be to "poison" the registers with useless
>>> data: 0xdeadbeef is a favorite.  Anyone seeing that in the registers
>>> will immediately know, "Someone used a value that they shouldn't
>>> have."
>>>
>>> Of course, it's *possible* that then the images might come to rely
>>> on that poison being there; having a non-deterministic value there,
>>> like a hash of the TSC, would make this impossible.  But even then,
>>> you could make the argument that the image may come to rely on a
>>> *pseudo-random* number being there, which it uses for some other
>>> kind of seed somewhere.  At some point you just have to give up on
>>> this like of thought. :-)
>> :-)))
>>
>>> Personally I think having a poison is likely to be more useful -- if
>>> you crash because your pointer is 0xdeadbeef, then it's immediately
>>> obvious what kind of bug you have; whereas if you crash with a
>>> random value that changes every time, it's not so obvious.
>> It works for me too. Any solution has pros and cons. However, in general
>> I think that wiping registers in that case is nice idea. So we could zero
>> registers, write 0xdeadbeef into them or even use TSC. But please do not
>> leave registers as is.
> DEADBABE or DEADBEEF sounds best.

"DEADBABE" sounds awful.  We have enough problems with getting women to 
contribute to the kernel without reminding them of the risks of violence 
they face on a regular basis.

  -George

Re: [PATCH] xen/kexec: Clear unused registers before jumping into an image

[George Dunlap]

On 18/11/13 13:13, Petr Tesarik wrote:
> On Mon, 18 Nov 2013 13:41:02 +0100
> Daniel Kiper <daniel.kiper@oracle.com> wrote:
>
>> On Mon, Nov 18, 2013 at 12:05:38PM +0000, George Dunlap wrote:
>>> On 18/11/13 11:47, Daniel Kiper wrote:
>>>> On Mon, Nov 18, 2013 at 11:23:27AM +0000, David Vrabel wrote:
>>>>> On 18/11/13 09:29, Jan Beulich wrote:
>>>>>>>>> On 15.11.13 at 21:07, David Vrabel <david.vrabel@citrix.com> wrote:
>>>>>>> On 15/11/13 15:56, Daniel Kiper wrote:
>>>>>>>> Clear unused registers before jumping into an image. This way
>>>>>>>> loaded image could not assume that any register has an specific
>>>>>>>> info about earlier running Xen hypervisor. However, it also
>>>>>>>> does not mean that the image may expect that a given register
>>>>>>>> is zeroed. The image MUST assume that every register has a random
>>>>>>>> value or in other words it is uninitialized or has undefined state.
>>>>>>> I think this, where the specification (registers undefined) differs from
>>>>>>> the implementation (registers zeroed), is the worst option.
>>>>>>>
>>>>>>> I also think it is more likely for an image to inadvertently rely on a
>>>>>>> zero value that whatever junk Xen has left behind.
>>>>>> Preventing users to rely on anything would likely make it
>>>>>> desirable to put some random value into all unused registers.
>>>>> I don't think we need to go that far.
>>>>>
>>>>> I would just like to avoid someone looking that the implementation (and
>>>>> not the documentation) and concluding that zero-ing of the registers is
>>>>> part of the specified behaviour, or looking at the implementation and
>>>>> documentation and wondering why they don't agree.
>>>> David, my comment clearly states why we are doing that and what should
>>>> be expected. What is wrong with it? I could improve it but say how?
>>> You seem to be saying that the registers may contain useful
>>> information about Xen that are not part of the spec, and you are
>>> worried that the image may decide to use these and come to rely on
>>> them, making it hard to change the interface in the future.
>>>
>>> David has a similar concern -- that if the registers are zeroed, the
>>> image may come to rely on the registers being pre-zeroed, and not
>>> zero them itself.  This would also make it hard to change the
>>> interface in the future.
>>>
>>> A simple solution would be to "poison" the registers with useless
>>> data: 0xdeadbeef is a favorite.  Anyone seeing that in the registers
>>> will immediately know, "Someone used a value that they shouldn't
>>> have."
>>>
>>> Of course, it's *possible* that then the images might come to rely
>>> on that poison being there; having a non-deterministic value there,
>>> like a hash of the TSC, would make this impossible.  But even then,
>>> you could make the argument that the image may come to rely on a
>>> *pseudo-random* number being there, which it uses for some other
>>> kind of seed somewhere.  At some point you just have to give up on
>>> this like of thought. :-)
>> :-)))
>>
>>> Personally I think having a poison is likely to be more useful -- if
>>> you crash because your pointer is 0xdeadbeef, then it's immediately
>>> obvious what kind of bug you have; whereas if you crash with a
>>> random value that changes every time, it's not so obvious.
>> It works for me too. Any solution has pros and cons. However, in general
>> I think that wiping registers in that case is nice idea. So we could zero
>> registers, write 0xdeadbeef into them or even use TSC. But please do not
>> leave registers as is.
> DEADBABE or DEADBEEF sounds best.

"DEADBABE" sounds awful.  We have enough problems with getting women to 
contribute to the kernel without reminding them of the risks of violence 
they face on a regular basis.

  -George

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH] xen/kexec: Clear unused registers before jumping into an image

[Konrad Rzeszutek Wilk]

On Mon, Nov 18, 2013 at 02:06:36PM +0000, George Dunlap wrote:
> On 18/11/13 13:13, Petr Tesarik wrote:
> >On Mon, 18 Nov 2013 13:41:02 +0100
> >Daniel Kiper <daniel.kiper@oracle.com> wrote:
> >
> >>On Mon, Nov 18, 2013 at 12:05:38PM +0000, George Dunlap wrote:
> >>>On 18/11/13 11:47, Daniel Kiper wrote:
> >>>>On Mon, Nov 18, 2013 at 11:23:27AM +0000, David Vrabel wrote:
> >>>>>On 18/11/13 09:29, Jan Beulich wrote:
> >>>>>>>>>On 15.11.13 at 21:07, David Vrabel <david.vrabel@citrix.com> wrote:
> >>>>>>>On 15/11/13 15:56, Daniel Kiper wrote:
> >>>>>>>>Clear unused registers before jumping into an image. This way
> >>>>>>>>loaded image could not assume that any register has an specific
> >>>>>>>>info about earlier running Xen hypervisor. However, it also
> >>>>>>>>does not mean that the image may expect that a given register
> >>>>>>>>is zeroed. The image MUST assume that every register has a random
> >>>>>>>>value or in other words it is uninitialized or has undefined state.
> >>>>>>>I think this, where the specification (registers undefined) differs 
> >>>>>>>from
> >>>>>>>the implementation (registers zeroed), is the worst option.
> >>>>>>>
> >>>>>>>I also think it is more likely for an image to inadvertently rely on 
> >>>>>>>a
> >>>>>>>zero value that whatever junk Xen has left behind.
> >>>>>>Preventing users to rely on anything would likely make it
> >>>>>>desirable to put some random value into all unused registers.
> >>>>>I don't think we need to go that far.
> >>>>>
> >>>>>I would just like to avoid someone looking that the implementation (and
> >>>>>not the documentation) and concluding that zero-ing of the registers is
> >>>>>part of the specified behaviour, or looking at the implementation and
> >>>>>documentation and wondering why they don't agree.
> >>>>David, my comment clearly states why we are doing that and what should
> >>>>be expected. What is wrong with it? I could improve it but say how?
> >>>You seem to be saying that the registers may contain useful
> >>>information about Xen that are not part of the spec, and you are
> >>>worried that the image may decide to use these and come to rely on
> >>>them, making it hard to change the interface in the future.
> >>>
> >>>David has a similar concern -- that if the registers are zeroed, the
> >>>image may come to rely on the registers being pre-zeroed, and not
> >>>zero them itself.  This would also make it hard to change the
> >>>interface in the future.
> >>>
> >>>A simple solution would be to "poison" the registers with useless
> >>>data: 0xdeadbeef is a favorite.  Anyone seeing that in the registers
> >>>will immediately know, "Someone used a value that they shouldn't
> >>>have."
> >>>
> >>>Of course, it's *possible* that then the images might come to rely
> >>>on that poison being there; having a non-deterministic value there,
> >>>like a hash of the TSC, would make this impossible.  But even then,
> >>>you could make the argument that the image may come to rely on a
> >>>*pseudo-random* number being there, which it uses for some other
> >>>kind of seed somewhere.  At some point you just have to give up on
> >>>this like of thought. :-)
> >>:-)))
> >>
> >>>Personally I think having a poison is likely to be more useful -- if
> >>>you crash because your pointer is 0xdeadbeef, then it's immediately
> >>>obvious what kind of bug you have; whereas if you crash with a
> >>>random value that changes every time, it's not so obvious.
> >>It works for me too. Any solution has pros and cons. However, in general
> >>I think that wiping registers in that case is nice idea. So we could zero
> >>registers, write 0xdeadbeef into them or even use TSC. But please do not
> >>leave registers as is.
> >DEADBABE or DEADBEEF sounds best.
> 
> "DEADBABE" sounds awful.  We have enough problems with getting women to 
> contribute to the kernel without reminding them of the risks of violence 
> they face on a regular basis.

<nods>

I would like to point out that Eric Biederman stated that the
reason for using 0 was:
 0/NULL is a good choice because if you are expecting pointer for some
strange reason interesting things happen.
" (See http://article.gmane.org/gmane.linux.kernel/1577040).

Which is a bit inline with - we don't want folks to depend on it,
so we make sure to poison it (with zeros in this case).

0xdeadbeef would get the same thing. If we are going that route
we perhaps we should do the same thing on Linux?

Re: [Xen-devel] [PATCH] xen/kexec: Clear unused registers before jumping into an image

[Konrad Rzeszutek Wilk]

On Mon, Nov 18, 2013 at 02:06:36PM +0000, George Dunlap wrote:
> On 18/11/13 13:13, Petr Tesarik wrote:
> >On Mon, 18 Nov 2013 13:41:02 +0100
> >Daniel Kiper <daniel.kiper@oracle.com> wrote:
> >
> >>On Mon, Nov 18, 2013 at 12:05:38PM +0000, George Dunlap wrote:
> >>>On 18/11/13 11:47, Daniel Kiper wrote:
> >>>>On Mon, Nov 18, 2013 at 11:23:27AM +0000, David Vrabel wrote:
> >>>>>On 18/11/13 09:29, Jan Beulich wrote:
> >>>>>>>>>On 15.11.13 at 21:07, David Vrabel <david.vrabel@citrix.com> wrote:
> >>>>>>>On 15/11/13 15:56, Daniel Kiper wrote:
> >>>>>>>>Clear unused registers before jumping into an image. This way
> >>>>>>>>loaded image could not assume that any register has an specific
> >>>>>>>>info about earlier running Xen hypervisor. However, it also
> >>>>>>>>does not mean that the image may expect that a given register
> >>>>>>>>is zeroed. The image MUST assume that every register has a random
> >>>>>>>>value or in other words it is uninitialized or has undefined state.
> >>>>>>>I think this, where the specification (registers undefined) differs 
> >>>>>>>from
> >>>>>>>the implementation (registers zeroed), is the worst option.
> >>>>>>>
> >>>>>>>I also think it is more likely for an image to inadvertently rely on 
> >>>>>>>a
> >>>>>>>zero value that whatever junk Xen has left behind.
> >>>>>>Preventing users to rely on anything would likely make it
> >>>>>>desirable to put some random value into all unused registers.
> >>>>>I don't think we need to go that far.
> >>>>>
> >>>>>I would just like to avoid someone looking that the implementation (and
> >>>>>not the documentation) and concluding that zero-ing of the registers is
> >>>>>part of the specified behaviour, or looking at the implementation and
> >>>>>documentation and wondering why they don't agree.
> >>>>David, my comment clearly states why we are doing that and what should
> >>>>be expected. What is wrong with it? I could improve it but say how?
> >>>You seem to be saying that the registers may contain useful
> >>>information about Xen that are not part of the spec, and you are
> >>>worried that the image may decide to use these and come to rely on
> >>>them, making it hard to change the interface in the future.
> >>>
> >>>David has a similar concern -- that if the registers are zeroed, the
> >>>image may come to rely on the registers being pre-zeroed, and not
> >>>zero them itself.  This would also make it hard to change the
> >>>interface in the future.
> >>>
> >>>A simple solution would be to "poison" the registers with useless
> >>>data: 0xdeadbeef is a favorite.  Anyone seeing that in the registers
> >>>will immediately know, "Someone used a value that they shouldn't
> >>>have."
> >>>
> >>>Of course, it's *possible* that then the images might come to rely
> >>>on that poison being there; having a non-deterministic value there,
> >>>like a hash of the TSC, would make this impossible.  But even then,
> >>>you could make the argument that the image may come to rely on a
> >>>*pseudo-random* number being there, which it uses for some other
> >>>kind of seed somewhere.  At some point you just have to give up on
> >>>this like of thought. :-)
> >>:-)))
> >>
> >>>Personally I think having a poison is likely to be more useful -- if
> >>>you crash because your pointer is 0xdeadbeef, then it's immediately
> >>>obvious what kind of bug you have; whereas if you crash with a
> >>>random value that changes every time, it's not so obvious.
> >>It works for me too. Any solution has pros and cons. However, in general
> >>I think that wiping registers in that case is nice idea. So we could zero
> >>registers, write 0xdeadbeef into them or even use TSC. But please do not
> >>leave registers as is.
> >DEADBABE or DEADBEEF sounds best.
> 
> "DEADBABE" sounds awful.  We have enough problems with getting women to 
> contribute to the kernel without reminding them of the risks of violence 
> they face on a regular basis.

<nods>

I would like to point out that Eric Biederman stated that the
reason for using 0 was:
 0/NULL is a good choice because if you are expecting pointer for some
strange reason interesting things happen.
" (See http://article.gmane.org/gmane.linux.kernel/1577040).

Which is a bit inline with - we don't want folks to depend on it,
so we make sure to poison it (with zeros in this case).

0xdeadbeef would get the same thing. If we are going that route
we perhaps we should do the same thing on Linux?

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH] xen/kexec: Clear unused registers before jumping into an image

[Konrad Rzeszutek Wilk]

On Tue, Nov 19, 2013 at 03:35:41PM -0400, Konrad Rzeszutek Wilk wrote:
> On Mon, Nov 18, 2013 at 02:06:36PM +0000, George Dunlap wrote:
> > On 18/11/13 13:13, Petr Tesarik wrote:
> > >On Mon, 18 Nov 2013 13:41:02 +0100
> > >Daniel Kiper <daniel.kiper@oracle.com> wrote:
> > >
> > >>On Mon, Nov 18, 2013 at 12:05:38PM +0000, George Dunlap wrote:
> > >>>On 18/11/13 11:47, Daniel Kiper wrote:
> > >>>>On Mon, Nov 18, 2013 at 11:23:27AM +0000, David Vrabel wrote:
> > >>>>>On 18/11/13 09:29, Jan Beulich wrote:
> > >>>>>>>>>On 15.11.13 at 21:07, David Vrabel <david.vrabel@citrix.com> wrote:
> > >>>>>>>On 15/11/13 15:56, Daniel Kiper wrote:
> > >>>>>>>>Clear unused registers before jumping into an image. This way
> > >>>>>>>>loaded image could not assume that any register has an specific
> > >>>>>>>>info about earlier running Xen hypervisor. However, it also
> > >>>>>>>>does not mean that the image may expect that a given register
> > >>>>>>>>is zeroed. The image MUST assume that every register has a random
> > >>>>>>>>value or in other words it is uninitialized or has undefined state.
> > >>>>>>>I think this, where the specification (registers undefined) differs 
> > >>>>>>>from
> > >>>>>>>the implementation (registers zeroed), is the worst option.
> > >>>>>>>
> > >>>>>>>I also think it is more likely for an image to inadvertently rely on 
> > >>>>>>>a
> > >>>>>>>zero value that whatever junk Xen has left behind.
> > >>>>>>Preventing users to rely on anything would likely make it
> > >>>>>>desirable to put some random value into all unused registers.
> > >>>>>I don't think we need to go that far.
> > >>>>>
> > >>>>>I would just like to avoid someone looking that the implementation (and
> > >>>>>not the documentation) and concluding that zero-ing of the registers is
> > >>>>>part of the specified behaviour, or looking at the implementation and
> > >>>>>documentation and wondering why they don't agree.
> > >>>>David, my comment clearly states why we are doing that and what should
> > >>>>be expected. What is wrong with it? I could improve it but say how?
> > >>>You seem to be saying that the registers may contain useful
> > >>>information about Xen that are not part of the spec, and you are
> > >>>worried that the image may decide to use these and come to rely on
> > >>>them, making it hard to change the interface in the future.
> > >>>
> > >>>David has a similar concern -- that if the registers are zeroed, the
> > >>>image may come to rely on the registers being pre-zeroed, and not
> > >>>zero them itself.  This would also make it hard to change the
> > >>>interface in the future.
> > >>>
> > >>>A simple solution would be to "poison" the registers with useless
> > >>>data: 0xdeadbeef is a favorite.  Anyone seeing that in the registers
> > >>>will immediately know, "Someone used a value that they shouldn't
> > >>>have."
> > >>>
> > >>>Of course, it's *possible* that then the images might come to rely
> > >>>on that poison being there; having a non-deterministic value there,
> > >>>like a hash of the TSC, would make this impossible.  But even then,
> > >>>you could make the argument that the image may come to rely on a
> > >>>*pseudo-random* number being there, which it uses for some other
> > >>>kind of seed somewhere.  At some point you just have to give up on
> > >>>this like of thought. :-)
> > >>:-)))
> > >>
> > >>>Personally I think having a poison is likely to be more useful -- if
> > >>>you crash because your pointer is 0xdeadbeef, then it's immediately
> > >>>obvious what kind of bug you have; whereas if you crash with a
> > >>>random value that changes every time, it's not so obvious.
> > >>It works for me too. Any solution has pros and cons. However, in general
> > >>I think that wiping registers in that case is nice idea. So we could zero
> > >>registers, write 0xdeadbeef into them or even use TSC. But please do not
> > >>leave registers as is.
> > >DEADBABE or DEADBEEF sounds best.
> > 
> > "DEADBABE" sounds awful.  We have enough problems with getting women to 
> > contribute to the kernel without reminding them of the risks of violence 
> > they face on a regular basis.
> 
> <nods>

Just to clarify - I completely agree with George. That 'DEADBABE' or any
other (see http://www.wired.com/wiredenterprise/2012/07/b16b00b5/) are
offensive.

> 
> I would like to point out that Eric Biederman stated that the
> reason for using 0 was:
>  0/NULL is a good choice because if you are expecting pointer for some
> strange reason interesting things happen.
> " (See http://article.gmane.org/gmane.linux.kernel/1577040).
> 
> Which is a bit inline with - we don't want folks to depend on it,
> so we make sure to poison it (with zeros in this case).
> 
> 0xdeadbeef would get the same thing. If we are going that route
> we perhaps we should do the same thing on Linux?
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] xen/kexec: Clear unused registers before jumping into an image

[Konrad Rzeszutek Wilk]

On Tue, Nov 19, 2013 at 03:35:41PM -0400, Konrad Rzeszutek Wilk wrote:
> On Mon, Nov 18, 2013 at 02:06:36PM +0000, George Dunlap wrote:
> > On 18/11/13 13:13, Petr Tesarik wrote:
> > >On Mon, 18 Nov 2013 13:41:02 +0100
> > >Daniel Kiper <daniel.kiper@oracle.com> wrote:
> > >
> > >>On Mon, Nov 18, 2013 at 12:05:38PM +0000, George Dunlap wrote:
> > >>>On 18/11/13 11:47, Daniel Kiper wrote:
> > >>>>On Mon, Nov 18, 2013 at 11:23:27AM +0000, David Vrabel wrote:
> > >>>>>On 18/11/13 09:29, Jan Beulich wrote:
> > >>>>>>>>>On 15.11.13 at 21:07, David Vrabel <david.vrabel@citrix.com> wrote:
> > >>>>>>>On 15/11/13 15:56, Daniel Kiper wrote:
> > >>>>>>>>Clear unused registers before jumping into an image. This way
> > >>>>>>>>loaded image could not assume that any register has an specific
> > >>>>>>>>info about earlier running Xen hypervisor. However, it also
> > >>>>>>>>does not mean that the image may expect that a given register
> > >>>>>>>>is zeroed. The image MUST assume that every register has a random
> > >>>>>>>>value or in other words it is uninitialized or has undefined state.
> > >>>>>>>I think this, where the specification (registers undefined) differs 
> > >>>>>>>from
> > >>>>>>>the implementation (registers zeroed), is the worst option.
> > >>>>>>>
> > >>>>>>>I also think it is more likely for an image to inadvertently rely on 
> > >>>>>>>a
> > >>>>>>>zero value that whatever junk Xen has left behind.
> > >>>>>>Preventing users to rely on anything would likely make it
> > >>>>>>desirable to put some random value into all unused registers.
> > >>>>>I don't think we need to go that far.
> > >>>>>
> > >>>>>I would just like to avoid someone looking that the implementation (and
> > >>>>>not the documentation) and concluding that zero-ing of the registers is
> > >>>>>part of the specified behaviour, or looking at the implementation and
> > >>>>>documentation and wondering why they don't agree.
> > >>>>David, my comment clearly states why we are doing that and what should
> > >>>>be expected. What is wrong with it? I could improve it but say how?
> > >>>You seem to be saying that the registers may contain useful
> > >>>information about Xen that are not part of the spec, and you are
> > >>>worried that the image may decide to use these and come to rely on
> > >>>them, making it hard to change the interface in the future.
> > >>>
> > >>>David has a similar concern -- that if the registers are zeroed, the
> > >>>image may come to rely on the registers being pre-zeroed, and not
> > >>>zero them itself.  This would also make it hard to change the
> > >>>interface in the future.
> > >>>
> > >>>A simple solution would be to "poison" the registers with useless
> > >>>data: 0xdeadbeef is a favorite.  Anyone seeing that in the registers
> > >>>will immediately know, "Someone used a value that they shouldn't
> > >>>have."
> > >>>
> > >>>Of course, it's *possible* that then the images might come to rely
> > >>>on that poison being there; having a non-deterministic value there,
> > >>>like a hash of the TSC, would make this impossible.  But even then,
> > >>>you could make the argument that the image may come to rely on a
> > >>>*pseudo-random* number being there, which it uses for some other
> > >>>kind of seed somewhere.  At some point you just have to give up on
> > >>>this like of thought. :-)
> > >>:-)))
> > >>
> > >>>Personally I think having a poison is likely to be more useful -- if
> > >>>you crash because your pointer is 0xdeadbeef, then it's immediately
> > >>>obvious what kind of bug you have; whereas if you crash with a
> > >>>random value that changes every time, it's not so obvious.
> > >>It works for me too. Any solution has pros and cons. However, in general
> > >>I think that wiping registers in that case is nice idea. So we could zero
> > >>registers, write 0xdeadbeef into them or even use TSC. But please do not
> > >>leave registers as is.
> > >DEADBABE or DEADBEEF sounds best.
> > 
> > "DEADBABE" sounds awful.  We have enough problems with getting women to 
> > contribute to the kernel without reminding them of the risks of violence 
> > they face on a regular basis.
> 
> <nods>

Just to clarify - I completely agree with George. That 'DEADBABE' or any
other (see http://www.wired.com/wiredenterprise/2012/07/b16b00b5/) are
offensive.

> 
> I would like to point out that Eric Biederman stated that the
> reason for using 0 was:
>  0/NULL is a good choice because if you are expecting pointer for some
> strange reason interesting things happen.
> " (See http://article.gmane.org/gmane.linux.kernel/1577040).
> 
> Which is a bit inline with - we don't want folks to depend on it,
> so we make sure to poison it (with zeros in this case).
> 
> 0xdeadbeef would get the same thing. If we are going that route
> we perhaps we should do the same thing on Linux?
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH] xen/kexec: Clear unused registers before jumping into an image

[Petr Tesarik]

On Mon, 18 Nov 2013 13:41:02 +0100
Daniel Kiper <daniel.kiper@oracle.com> wrote:

> On Mon, Nov 18, 2013 at 12:05:38PM +0000, George Dunlap wrote:
> > On 18/11/13 11:47, Daniel Kiper wrote:
> > >On Mon, Nov 18, 2013 at 11:23:27AM +0000, David Vrabel wrote:
> > >>On 18/11/13 09:29, Jan Beulich wrote:
> > >>>>>>On 15.11.13 at 21:07, David Vrabel <david.vrabel@citrix.com> wrote:
> > >>>>On 15/11/13 15:56, Daniel Kiper wrote:
> > >>>>>Clear unused registers before jumping into an image. This way
> > >>>>>loaded image could not assume that any register has an specific
> > >>>>>info about earlier running Xen hypervisor. However, it also
> > >>>>>does not mean that the image may expect that a given register
> > >>>>>is zeroed. The image MUST assume that every register has a random
> > >>>>>value or in other words it is uninitialized or has undefined state.
> > >>>>I think this, where the specification (registers undefined) differs from
> > >>>>the implementation (registers zeroed), is the worst option.
> > >>>>
> > >>>>I also think it is more likely for an image to inadvertently rely on a
> > >>>>zero value that whatever junk Xen has left behind.
> > >>>Preventing users to rely on anything would likely make it
> > >>>desirable to put some random value into all unused registers.
> > >>I don't think we need to go that far.
> > >>
> > >>I would just like to avoid someone looking that the implementation (and
> > >>not the documentation) and concluding that zero-ing of the registers is
> > >>part of the specified behaviour, or looking at the implementation and
> > >>documentation and wondering why they don't agree.
> > >David, my comment clearly states why we are doing that and what should
> > >be expected. What is wrong with it? I could improve it but say how?
> >
> > You seem to be saying that the registers may contain useful
> > information about Xen that are not part of the spec, and you are
> > worried that the image may decide to use these and come to rely on
> > them, making it hard to change the interface in the future.
> >
> > David has a similar concern -- that if the registers are zeroed, the
> > image may come to rely on the registers being pre-zeroed, and not
> > zero them itself.  This would also make it hard to change the
> > interface in the future.
> >
> > A simple solution would be to "poison" the registers with useless
> > data: 0xdeadbeef is a favorite.  Anyone seeing that in the registers
> > will immediately know, "Someone used a value that they shouldn't
> > have."
> >
> > Of course, it's *possible* that then the images might come to rely
> > on that poison being there; having a non-deterministic value there,
> > like a hash of the TSC, would make this impossible.  But even then,
> > you could make the argument that the image may come to rely on a
> > *pseudo-random* number being there, which it uses for some other
> > kind of seed somewhere.  At some point you just have to give up on
> > this like of thought. :-)
> 
> :-)))
> 
> > Personally I think having a poison is likely to be more useful -- if
> > you crash because your pointer is 0xdeadbeef, then it's immediately
> > obvious what kind of bug you have; whereas if you crash with a
> > random value that changes every time, it's not so obvious.
> 
> It works for me too. Any solution has pros and cons. However, in general
> I think that wiping registers in that case is nice idea. So we could zero
> registers, write 0xdeadbeef into them or even use TSC. But please do not
> leave registers as is.

DEADBABE or DEADBEEF sounds best. It even makes it possible to
distinguish between an old version and a new version if we ever need
to extend the interface and actually use a register to pass down
something useful to the boot loader.

Hmm, so maybe we could poison all registers, except e.g. RDI (1st
argument in the standard ABI), which could be zeroed and _documented_
as containing the interface version.

Petr T

Re: [PATCH] xen/kexec: Clear unused registers before jumping into an image

[Daniel Kiper]

On Mon, Nov 18, 2013 at 12:05:38PM +0000, George Dunlap wrote:
> On 18/11/13 11:47, Daniel Kiper wrote:
> >On Mon, Nov 18, 2013 at 11:23:27AM +0000, David Vrabel wrote:
> >>On 18/11/13 09:29, Jan Beulich wrote:
> >>>>>>On 15.11.13 at 21:07, David Vrabel <david.vrabel@citrix.com> wrote:
> >>>>On 15/11/13 15:56, Daniel Kiper wrote:
> >>>>>Clear unused registers before jumping into an image. This way
> >>>>>loaded image could not assume that any register has an specific
> >>>>>info about earlier running Xen hypervisor. However, it also
> >>>>>does not mean that the image may expect that a given register
> >>>>>is zeroed. The image MUST assume that every register has a random
> >>>>>value or in other words it is uninitialized or has undefined state.
> >>>>I think this, where the specification (registers undefined) differs from
> >>>>the implementation (registers zeroed), is the worst option.
> >>>>
> >>>>I also think it is more likely for an image to inadvertently rely on a
> >>>>zero value that whatever junk Xen has left behind.
> >>>Preventing users to rely on anything would likely make it
> >>>desirable to put some random value into all unused registers.
> >>I don't think we need to go that far.
> >>
> >>I would just like to avoid someone looking that the implementation (and
> >>not the documentation) and concluding that zero-ing of the registers is
> >>part of the specified behaviour, or looking at the implementation and
> >>documentation and wondering why they don't agree.
> >David, my comment clearly states why we are doing that and what should
> >be expected. What is wrong with it? I could improve it but say how?
>
> You seem to be saying that the registers may contain useful
> information about Xen that are not part of the spec, and you are
> worried that the image may decide to use these and come to rely on
> them, making it hard to change the interface in the future.
>
> David has a similar concern -- that if the registers are zeroed, the
> image may come to rely on the registers being pre-zeroed, and not
> zero them itself.  This would also make it hard to change the
> interface in the future.
>
> A simple solution would be to "poison" the registers with useless
> data: 0xdeadbeef is a favorite.  Anyone seeing that in the registers
> will immediately know, "Someone used a value that they shouldn't
> have."
>
> Of course, it's *possible* that then the images might come to rely
> on that poison being there; having a non-deterministic value there,
> like a hash of the TSC, would make this impossible.  But even then,
> you could make the argument that the image may come to rely on a
> *pseudo-random* number being there, which it uses for some other
> kind of seed somewhere.  At some point you just have to give up on
> this like of thought. :-)

:-)))

> Personally I think having a poison is likely to be more useful -- if
> you crash because your pointer is 0xdeadbeef, then it's immediately
> obvious what kind of bug you have; whereas if you crash with a
> random value that changes every time, it's not so obvious.

It works for me too. Any solution has pros and cons. However, in general
I think that wiping registers in that case is nice idea. So we could zero
registers, write 0xdeadbeef into them or even use TSC. But please do not
leave registers as is.

Daniel

Re: [PATCH] xen/kexec: Clear unused registers before jumping into an image

[George Dunlap]

On 18/11/13 11:47, Daniel Kiper wrote:
> On Mon, Nov 18, 2013 at 11:23:27AM +0000, David Vrabel wrote:
>> On 18/11/13 09:29, Jan Beulich wrote:
>>>>>> On 15.11.13 at 21:07, David Vrabel <david.vrabel@citrix.com> wrote:
>>>> On 15/11/13 15:56, Daniel Kiper wrote:
>>>>> Clear unused registers before jumping into an image. This way
>>>>> loaded image could not assume that any register has an specific
>>>>> info about earlier running Xen hypervisor. However, it also
>>>>> does not mean that the image may expect that a given register
>>>>> is zeroed. The image MUST assume that every register has a random
>>>>> value or in other words it is uninitialized or has undefined state.
>>>> I think this, where the specification (registers undefined) differs from
>>>> the implementation (registers zeroed), is the worst option.
>>>>
>>>> I also think it is more likely for an image to inadvertently rely on a
>>>> zero value that whatever junk Xen has left behind.
>>> Preventing users to rely on anything would likely make it
>>> desirable to put some random value into all unused registers.
>> I don't think we need to go that far.
>>
>> I would just like to avoid someone looking that the implementation (and
>> not the documentation) and concluding that zero-ing of the registers is
>> part of the specified behaviour, or looking at the implementation and
>> documentation and wondering why they don't agree.
> David, my comment clearly states why we are doing that and what should
> be expected. What is wrong with it? I could improve it but say how?

You seem to be saying that the registers may contain useful information 
about Xen that are not part of the spec, and you are worried that the 
image may decide to use these and come to rely on them, making it hard 
to change the interface in the future.

David has a similar concern -- that if the registers are zeroed, the 
image may come to rely on the registers being pre-zeroed, and not zero 
them itself.  This would also make it hard to change the interface in 
the future.

A simple solution would be to "poison" the registers with useless data: 
0xdeadbeef is a favorite.  Anyone seeing that in the registers will 
immediately know, "Someone used a value that they shouldn't have."

Of course, it's *possible* that then the images might come to rely on 
that poison being there; having a non-deterministic value there, like a 
hash of the TSC, would make this impossible.  But even then, you could 
make the argument that the image may come to rely on a *pseudo-random* 
number being there, which it uses for some other kind of seed 
somewhere.  At some point you just have to give up on this like of 
thought. :-)

Personally I think having a poison is likely to be more useful -- if you 
crash because your pointer is 0xdeadbeef, then it's immediately obvious 
what kind of bug you have; whereas if you crash with a random value that 
changes every time, it's not so obvious.

  -George

Re: [PATCH] xen/kexec: Clear unused registers before jumping into an image

[Daniel Kiper]

On Mon, Nov 18, 2013 at 11:23:27AM +0000, David Vrabel wrote:
> On 18/11/13 09:29, Jan Beulich wrote:
> >>>> On 15.11.13 at 21:07, David Vrabel <david.vrabel@citrix.com> wrote:
> >> On 15/11/13 15:56, Daniel Kiper wrote:
> >>> Clear unused registers before jumping into an image. This way
> >>> loaded image could not assume that any register has an specific
> >>> info about earlier running Xen hypervisor. However, it also
> >>> does not mean that the image may expect that a given register
> >>> is zeroed. The image MUST assume that every register has a random
> >>> value or in other words it is uninitialized or has undefined state.
> >>
> >> I think this, where the specification (registers undefined) differs from
> >> the implementation (registers zeroed), is the worst option.
> >>
> >> I also think it is more likely for an image to inadvertently rely on a
> >> zero value that whatever junk Xen has left behind.
> >
> > Preventing users to rely on anything would likely make it
> > desirable to put some random value into all unused registers.
>
> I don't think we need to go that far.
>
> I would just like to avoid someone looking that the implementation (and
> not the documentation) and concluding that zero-ing of the registers is
> part of the specified behaviour, or looking at the implementation and
> documentation and wondering why they don't agree.

David, my comment clearly states why we are doing that and what should
be expected. What is wrong with it? I could improve it but say how?

Daniel

Re: [PATCH] xen/kexec: Clear unused registers before jumping into an image

[David Vrabel]

On 18/11/13 09:29, Jan Beulich wrote:
>>>> On 15.11.13 at 21:07, David Vrabel <david.vrabel@citrix.com> wrote:
>> On 15/11/13 15:56, Daniel Kiper wrote:
>>> Clear unused registers before jumping into an image. This way
>>> loaded image could not assume that any register has an specific
>>> info about earlier running Xen hypervisor. However, it also
>>> does not mean that the image may expect that a given register
>>> is zeroed. The image MUST assume that every register has a random
>>> value or in other words it is uninitialized or has undefined state.
>>
>> I think this, where the specification (registers undefined) differs from
>> the implementation (registers zeroed), is the worst option.
>>
>> I also think it is more likely for an image to inadvertently rely on a
>> zero value that whatever junk Xen has left behind.
> 
> Preventing users to rely on anything would likely make it
> desirable to put some random value into all unused registers.

I don't think we need to go that far.

I would just like to avoid someone looking that the implementation (and
not the documentation) and concluding that zero-ing of the registers is
part of the specified behaviour, or looking at the implementation and
documentation and wondering why they don't agree.

I really don't think there is anything more to be said on this.

David

Re: [PATCH] xen/kexec: Clear unused registers before jumping into an image

[David Vrabel]

On 15/11/13 15:56, Daniel Kiper wrote:
> Clear unused registers before jumping into an image. This way
> loaded image could not assume that any register has an specific
> info about earlier running Xen hypervisor. However, it also
> does not mean that the image may expect that a given register
> is zeroed. The image MUST assume that every register has a random
> value or in other words it is uninitialized or has undefined state.

I think this, where the specification (registers undefined) differs from
the implementation (registers zeroed), is the worst option.

I also think it is more likely for an image to inadvertently rely on a
zero value that whatever junk Xen has left behind.

David

[PATCH] xen/kexec: Clear unused registers before jumping into an image

[Daniel Kiper]

Clear unused registers before jumping into an image. This way
loaded image could not assume that any register has an specific
info about earlier running Xen hypervisor. However, it also
does not mean that the image may expect that a given register
is zeroed. The image MUST assume that every register has a random
value or in other words it is uninitialized or has undefined state.

Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
---
 xen/arch/x86/x86_64/kexec_reloc.S |   37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/xen/arch/x86/x86_64/kexec_reloc.S b/xen/arch/x86/x86_64/kexec_reloc.S
index 7a16c85..e7eef79 100644
--- a/xen/arch/x86/x86_64/kexec_reloc.S
+++ b/xen/arch/x86/x86_64/kexec_reloc.S
@@ -71,6 +71,29 @@ identity_mapped:
         jnz     call_32_bit
 
 call_64_bit:
+        /*
+         * Clear unused registers before jumping into an image. This way
+         * loaded image could not assume that any register has an specific
+         * info about earlier running Xen hypervisor. However, it also
+         * does not mean that the image may expect that a given register
+         * is zeroed. The image MUST assume that every register has a random
+         * value or in other words it is uninitialized or has undefined state.
+         */
+        xorl    %eax, %eax
+        xorl    %ebx, %ebx
+        xorl    %ecx, %ecx
+        xorl    %edx, %edx
+        xorl    %esi, %esi
+        xorl    %edi, %edi
+        xorl    %r8d, %r8d
+        xorl    %r9d, %r9d
+        xorl    %r10d, %r10d
+        xorl    %r11d, %r11d
+        xorl    %r12d, %r12d
+        xorl    %r13d, %r13d
+        xorl    %r14d, %r14d
+        xorl    %r15d, %r15d
+
         /* Call the image entry point.  This should never return. */
         callq   *%rbp
         ud2
@@ -164,6 +187,20 @@ compatibility_mode:
         xorl    %eax, %eax
         movl    %eax, %cr4
 
+        /*
+         * Clear unused registers before jumping into an image. This way
+         * loaded image could not assume that any register has an specific
+         * info about earlier running Xen hypervisor. However, it also
+         * does not mean that the image may expect that a given register
+         * is zeroed. The image MUST assume that every register has a random
+         * value or in other words it is uninitialized or has undefined state.
+         */
+        xorl    %ebx, %ebx
+        xorl    %ecx, %ecx
+        xorl    %edx, %edx
+        xorl    %esi, %esi
+        xorl    %edi, %edi
+
         /* Call the image entry point.  This should never return. */
         call    *%ebp
         ud2
-- 
1.7.10.4