Re: [PATCH 4/6] kexec: A new system call, kexec_file_load, for in kernel kexec

2014-01-17 Thread Vivek Goyal
On Fri, Nov 22, 2013 at 09:42:52PM +0100, Jiri Kosina wrote:

[..]
> > @@ -843,7 +1075,11 @@ static int kimage_load_normal_segment(struct kimage 
> > *image,
> > PAGE_SIZE - (maddr & ~PAGE_MASK));
> > uchunk = min(ubytes, mchunk);
> >  
> > -   result = copy_from_user(ptr, buf, uchunk);
> > +   /* For file based kexec, source pages are in kernel memory */
> > +   if (image->file_mode)
> > +   memcpy(ptr, buf, uchunk);
> 
> Very minor nit I came across when going through the patchset -- can't we 
> use some different buffer for the file-based kexec that's not marked 
> __user here? This really causes some eye-pain when looking at the code.

Hi Jiri,

Sorry, responding to your comment after a very long time.

Now I have made buf field a union as it can either be a user pointer or
a kernel pointer depending on which kexec syscall has been used. Now
caller needs to either use segment->buf or segment->kbuf based on the
context of code.

kexec_segment {
union {
void __user *buf;
void *kbuf;
};
}

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


Re: [PATCH 4/6] kexec: A new system call, kexec_file_load, for in kernel kexec

2014-01-17 Thread Vivek Goyal
On Fri, Nov 22, 2013 at 09:42:52PM +0100, Jiri Kosina wrote:

[..]
  @@ -843,7 +1075,11 @@ static int kimage_load_normal_segment(struct kimage 
  *image,
  PAGE_SIZE - (maddr  ~PAGE_MASK));
  uchunk = min(ubytes, mchunk);
   
  -   result = copy_from_user(ptr, buf, uchunk);
  +   /* For file based kexec, source pages are in kernel memory */
  +   if (image-file_mode)
  +   memcpy(ptr, buf, uchunk);
 
 Very minor nit I came across when going through the patchset -- can't we 
 use some different buffer for the file-based kexec that's not marked 
 __user here? This really causes some eye-pain when looking at the code.

Hi Jiri,

Sorry, responding to your comment after a very long time.

Now I have made buf field a union as it can either be a user pointer or
a kernel pointer depending on which kexec syscall has been used. Now
caller needs to either use segment-buf or segment-kbuf based on the
context of code.

kexec_segment {
union {
void __user *buf;
void *kbuf;
};
}

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


Re: [PATCH 4/6] kexec: A new system call, kexec_file_load, for in kernel kexec

2014-01-06 Thread H. Peter Anvin
On 01/06/2014 01:33 PM, Josh Boyer wrote:
> On Thu, Jan 2, 2014 at 3:56 PM, H. Peter Anvin  wrote:
>> On 01/02/2014 12:39 PM, Vivek Goyal wrote:
>>>
>>> If secureboot is enabled, it enforces module signature verification. I
>>> think similar will happen for kexec too. How would kernel know that on
>>> a secureboot platform fd original verification will happen and it is
>>> sufficient.
>>>
>>> I personally want to support bzImage as well (apart from ELF) because
>>> distributions has been shipping bzImage for a long time and I don't
>>> want to enforce a change there because of secureboot. It is not necessary.
>>> Right now I am thinking more about storing detached bzImage signatures
>>> and passing those signatures to kexec system call.
>>>
>>
>> Since the secureboot scenario probably means people will be signing
>> those kernels, and those kernels are going to be EFI images, that in
>> order to have "one kernel, one signature" there will be a desire to
>> support signed PE images.  Yes, PE is ugly but it shouldn't be too bad.
>>  However, it is probably one of those things that can be dealt with one
>> bit at a time.
> 
> David Howells posted patches to support signed PE binaries early last
> year.  They were rejected rather quickly.
> 
> https://lkml.org/lkml/2013/2/21/196
> 
> That was for loading keys via PE binaries, but the parser is needed
> either way.  Unless I'm misunderstanding what you're suggesting?
> 

I know.  I think the kexec is a better motivation, though.

-hpa


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


Re: [PATCH 4/6] kexec: A new system call, kexec_file_load, for in kernel kexec

2014-01-06 Thread Josh Boyer
On Thu, Jan 2, 2014 at 3:56 PM, H. Peter Anvin  wrote:
> On 01/02/2014 12:39 PM, Vivek Goyal wrote:
>>
>> If secureboot is enabled, it enforces module signature verification. I
>> think similar will happen for kexec too. How would kernel know that on
>> a secureboot platform fd original verification will happen and it is
>> sufficient.
>>
>> I personally want to support bzImage as well (apart from ELF) because
>> distributions has been shipping bzImage for a long time and I don't
>> want to enforce a change there because of secureboot. It is not necessary.
>> Right now I am thinking more about storing detached bzImage signatures
>> and passing those signatures to kexec system call.
>>
>
> Since the secureboot scenario probably means people will be signing
> those kernels, and those kernels are going to be EFI images, that in
> order to have "one kernel, one signature" there will be a desire to
> support signed PE images.  Yes, PE is ugly but it shouldn't be too bad.
>  However, it is probably one of those things that can be dealt with one
> bit at a time.

David Howells posted patches to support signed PE binaries early last
year.  They were rejected rather quickly.

https://lkml.org/lkml/2013/2/21/196

That was for loading keys via PE binaries, but the parser is needed
either way.  Unless I'm misunderstanding what you're suggesting?

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


Re: [PATCH 4/6] kexec: A new system call, kexec_file_load, for in kernel kexec

2014-01-06 Thread Josh Boyer
On Thu, Jan 2, 2014 at 3:56 PM, H. Peter Anvin h...@zytor.com wrote:
 On 01/02/2014 12:39 PM, Vivek Goyal wrote:

 If secureboot is enabled, it enforces module signature verification. I
 think similar will happen for kexec too. How would kernel know that on
 a secureboot platform fd original verification will happen and it is
 sufficient.

 I personally want to support bzImage as well (apart from ELF) because
 distributions has been shipping bzImage for a long time and I don't
 want to enforce a change there because of secureboot. It is not necessary.
 Right now I am thinking more about storing detached bzImage signatures
 and passing those signatures to kexec system call.


 Since the secureboot scenario probably means people will be signing
 those kernels, and those kernels are going to be EFI images, that in
 order to have one kernel, one signature there will be a desire to
 support signed PE images.  Yes, PE is ugly but it shouldn't be too bad.
  However, it is probably one of those things that can be dealt with one
 bit at a time.

David Howells posted patches to support signed PE binaries early last
year.  They were rejected rather quickly.

https://lkml.org/lkml/2013/2/21/196

That was for loading keys via PE binaries, but the parser is needed
either way.  Unless I'm misunderstanding what you're suggesting?

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


Re: [PATCH 4/6] kexec: A new system call, kexec_file_load, for in kernel kexec

2014-01-06 Thread H. Peter Anvin
On 01/06/2014 01:33 PM, Josh Boyer wrote:
 On Thu, Jan 2, 2014 at 3:56 PM, H. Peter Anvin h...@zytor.com wrote:
 On 01/02/2014 12:39 PM, Vivek Goyal wrote:

 If secureboot is enabled, it enforces module signature verification. I
 think similar will happen for kexec too. How would kernel know that on
 a secureboot platform fd original verification will happen and it is
 sufficient.

 I personally want to support bzImage as well (apart from ELF) because
 distributions has been shipping bzImage for a long time and I don't
 want to enforce a change there because of secureboot. It is not necessary.
 Right now I am thinking more about storing detached bzImage signatures
 and passing those signatures to kexec system call.


 Since the secureboot scenario probably means people will be signing
 those kernels, and those kernels are going to be EFI images, that in
 order to have one kernel, one signature there will be a desire to
 support signed PE images.  Yes, PE is ugly but it shouldn't be too bad.
  However, it is probably one of those things that can be dealt with one
 bit at a time.
 
 David Howells posted patches to support signed PE binaries early last
 year.  They were rejected rather quickly.
 
 https://lkml.org/lkml/2013/2/21/196
 
 That was for loading keys via PE binaries, but the parser is needed
 either way.  Unless I'm misunderstanding what you're suggesting?
 

I know.  I think the kexec is a better motivation, though.

-hpa


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


Re: [PATCH 4/6] kexec: A new system call, kexec_file_load, for in kernel kexec

2014-01-02 Thread H. Peter Anvin
On 01/02/2014 12:39 PM, Vivek Goyal wrote:
> 
> If secureboot is enabled, it enforces module signature verification. I 
> think similar will happen for kexec too. How would kernel know that on
> a secureboot platform fd original verification will happen and it is
> sufficient.
> 
> I personally want to support bzImage as well (apart from ELF) because
> distributions has been shipping bzImage for a long time and I don't
> want to enforce a change there because of secureboot. It is not necessary.
> Right now I am thinking more about storing detached bzImage signatures
> and passing those signatures to kexec system call.
> 

Since the secureboot scenario probably means people will be signing
those kernels, and those kernels are going to be EFI images, that in
order to have "one kernel, one signature" there will be a desire to
support signed PE images.  Yes, PE is ugly but it shouldn't be too bad.
 However, it is probably one of those things that can be dealt with one
bit at a time.

-hpa


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


Re: [PATCH 4/6] kexec: A new system call, kexec_file_load, for in kernel kexec

2014-01-02 Thread Vivek Goyal
On Fri, Dec 20, 2013 at 03:20:16PM -0800, Kees Cook wrote:
> On Fri, Dec 20, 2013 at 3:11 PM, Eric W. Biederman
>  wrote:
> > Vivek Goyal  writes:
> >
> >> On Thu, Dec 19, 2013 at 01:54:39PM +0100, Torsten Duwe wrote:
> >>> On Tue, Nov 26, 2013 at 09:27:59AM -0500, Vivek Goyal wrote:
> >
> >>> IMO it's up to user land to search lists of certificates, and present
> >>> only the final chain of trust to the kernel for checking.
> >>>
> >>> ELF is the preferred format for most sane OSes and firmware, and a 
> >>> detached
> >>> signature would probably be simplest to check. If we have the choice,
> >>> without restrictions from braindead boot loaders, ELF should be first.
> >>> And if the pesigning isn't usable and another sig is needed anyway,
> >>> why not apply that to vmlinux(.gz) ?
> >>
> >> I have yet to look deeper into it that if we can sign elf images and
> >> just use elf loader. And can use space extract the elf image out of
> >> a bzImage and pass it to kernel.
> >>
> >> Even if it is doable, one disadvantage seemed to be that extracted
> >> elf images will have to be written to a file so thta it's file descriptor
> >> can be passed to kernel. And that assumed writable root and we chrome
> >> folks seems to have setups where root is not writable.
> >
> > In that case the chrome folks would simply have to use an ELF format
> > kernel and not a bzImage.
> 
> If we're doing fd origin verification (not signatures), can't we
> continue to use a regular bzImage?

If secureboot is enabled, it enforces module signature verification. I 
think similar will happen for kexec too. How would kernel know that on
a secureboot platform fd original verification will happen and it is
sufficient.

I personally want to support bzImage as well (apart from ELF) because
distributions has been shipping bzImage for a long time and I don't
want to enforce a change there because of secureboot. It is not necessary.
Right now I am thinking more about storing detached bzImage signatures
and passing those signatures to kexec system call.

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


Re: [PATCH 4/6] kexec: A new system call, kexec_file_load, for in kernel kexec

2014-01-02 Thread Vivek Goyal
On Fri, Dec 20, 2013 at 03:20:16PM -0800, Kees Cook wrote:
 On Fri, Dec 20, 2013 at 3:11 PM, Eric W. Biederman
 ebied...@xmission.com wrote:
  Vivek Goyal vgo...@redhat.com writes:
 
  On Thu, Dec 19, 2013 at 01:54:39PM +0100, Torsten Duwe wrote:
  On Tue, Nov 26, 2013 at 09:27:59AM -0500, Vivek Goyal wrote:
 
  IMO it's up to user land to search lists of certificates, and present
  only the final chain of trust to the kernel for checking.
 
  ELF is the preferred format for most sane OSes and firmware, and a 
  detached
  signature would probably be simplest to check. If we have the choice,
  without restrictions from braindead boot loaders, ELF should be first.
  And if the pesigning isn't usable and another sig is needed anyway,
  why not apply that to vmlinux(.gz) ?
 
  I have yet to look deeper into it that if we can sign elf images and
  just use elf loader. And can use space extract the elf image out of
  a bzImage and pass it to kernel.
 
  Even if it is doable, one disadvantage seemed to be that extracted
  elf images will have to be written to a file so thta it's file descriptor
  can be passed to kernel. And that assumed writable root and we chrome
  folks seems to have setups where root is not writable.
 
  In that case the chrome folks would simply have to use an ELF format
  kernel and not a bzImage.
 
 If we're doing fd origin verification (not signatures), can't we
 continue to use a regular bzImage?

If secureboot is enabled, it enforces module signature verification. I 
think similar will happen for kexec too. How would kernel know that on
a secureboot platform fd original verification will happen and it is
sufficient.

I personally want to support bzImage as well (apart from ELF) because
distributions has been shipping bzImage for a long time and I don't
want to enforce a change there because of secureboot. It is not necessary.
Right now I am thinking more about storing detached bzImage signatures
and passing those signatures to kexec system call.

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


Re: [PATCH 4/6] kexec: A new system call, kexec_file_load, for in kernel kexec

2014-01-02 Thread H. Peter Anvin
On 01/02/2014 12:39 PM, Vivek Goyal wrote:
 
 If secureboot is enabled, it enforces module signature verification. I 
 think similar will happen for kexec too. How would kernel know that on
 a secureboot platform fd original verification will happen and it is
 sufficient.
 
 I personally want to support bzImage as well (apart from ELF) because
 distributions has been shipping bzImage for a long time and I don't
 want to enforce a change there because of secureboot. It is not necessary.
 Right now I am thinking more about storing detached bzImage signatures
 and passing those signatures to kexec system call.
 

Since the secureboot scenario probably means people will be signing
those kernels, and those kernels are going to be EFI images, that in
order to have one kernel, one signature there will be a desire to
support signed PE images.  Yes, PE is ugly but it shouldn't be too bad.
 However, it is probably one of those things that can be dealt with one
bit at a time.

-hpa


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


Re: [PATCH 4/6] kexec: A new system call, kexec_file_load, for in kernel kexec

2013-12-21 Thread Torsten Duwe
On Fri, Dec 20, 2013 at 07:32:11PM -0800, H. Peter Anvin wrote:
> thing, as currently built there are megabytes of zeroes in it for no
> good reason.

Then remove them ;) AFAICS, that's x86 only? What a waste!

What's the reason? ALIGN_RODATA? Even if so, vmlinux.gz might be
a fair trade-off.

> 
> Even if you don't need the entry code, the additional metadata is
> meaningful.

Any idea, or maybe even a list of features that would get lost?
What are the blossoms of this organically grown structure?
Many architectures, even embedded x86, boot happily any ELF kernel.

I'm with Eric here: this is not about _not_ supporting bzImage, it's
about _do_ support ELF first. As I wrote: if the existing signature 
is, let's say, impractical, and a new one is needed anyway, why not 
(detached-) sign vmlinux or vmlinux.gz?

Every architecture can benefit from a secure boot or secure kexec
that is done right.

Torsten

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


Re: [PATCH 4/6] kexec: A new system call, kexec_file_load, for in kernel kexec

2013-12-21 Thread Torsten Duwe
On Fri, Dec 20, 2013 at 03:20:16PM -0800, Kees Cook wrote:
> 
> If we're doing fd origin verification (not signatures), can't we
> continue to use a regular bzImage?

I imagine the verification function is separated from the new kexec.
It gets passed the proposed new kernel file*, an optional signature
file*, and maybe hints about the kernel format and system security state.

With that function, you can do whatever you want. If you want to take
an early positive exit due to the location of the new kernel, you're fine.

Torsten

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


Re: [PATCH 4/6] kexec: A new system call, kexec_file_load, for in kernel kexec

2013-12-21 Thread Torsten Duwe
On Fri, Dec 20, 2013 at 03:20:16PM -0800, Kees Cook wrote:
 
 If we're doing fd origin verification (not signatures), can't we
 continue to use a regular bzImage?

I imagine the verification function is separated from the new kexec.
It gets passed the proposed new kernel file*, an optional signature
file*, and maybe hints about the kernel format and system security state.

With that function, you can do whatever you want. If you want to take
an early positive exit due to the location of the new kernel, you're fine.

Torsten

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


Re: [PATCH 4/6] kexec: A new system call, kexec_file_load, for in kernel kexec

2013-12-21 Thread Torsten Duwe
On Fri, Dec 20, 2013 at 07:32:11PM -0800, H. Peter Anvin wrote:
 thing, as currently built there are megabytes of zeroes in it for no
 good reason.

Then remove them ;) AFAICS, that's x86 only? What a waste!

What's the reason? ALIGN_RODATA? Even if so, vmlinux.gz might be
a fair trade-off.

 
 Even if you don't need the entry code, the additional metadata is
 meaningful.

Any idea, or maybe even a list of features that would get lost?
What are the blossoms of this organically grown structure?
Many architectures, even embedded x86, boot happily any ELF kernel.

I'm with Eric here: this is not about _not_ supporting bzImage, it's
about _do_ support ELF first. As I wrote: if the existing signature 
is, let's say, impractical, and a new one is needed anyway, why not 
(detached-) sign vmlinux or vmlinux.gz?

Every architecture can benefit from a secure boot or secure kexec
that is done right.

Torsten

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


Re: [PATCH 4/6] kexec: A new system call, kexec_file_load, for in kernel kexec

2013-12-20 Thread H. Peter Anvin
On 12/20/2013 05:32 PM, Eric W. Biederman wrote:
> 
> Stuff and nonsense.  bzImage is just an ugly wrapper around an ELF
> image.
> 

Not really.  We put the ELF image in there to help Xen and presumably
kexec, but there are actually quite a few issues with it... for one
thing, as currently built there are megabytes of zeroes in it for no
good reason.

Even if you don't need the entry code, the additional metadata is
meaningful.

-hpa


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


Re: [PATCH 4/6] kexec: A new system call, kexec_file_load, for in kernel kexec

2013-12-20 Thread Eric W. Biederman
"H. Peter Anvin"  writes:

> On 12/20/2013 03:11 PM, Eric W. Biederman wrote:
>> 
>> In that case the chrome folks would simply have to use an ELF format
>> kernel and not a bzImage.
>> 
>
> This is starting to feel like everything is going in the direction of a
> massive feature regression.  bzImage may be weird (it has definitely
> grown organically), but the features that have been added to it have
> generally been for a reason, e.g. kernel relocation and so on.

Stuff and nonsense.  bzImage is just an ugly wrapper around an ELF
image.

I am just arguing that we expose the clean portable underpinnings and
make that work.

It absolutely does not make sense to make a solution that only works for
x86.  ELF is what ever other architecture uses so we absolutely have to
make any feature we build work with ELF.

At a very basic level for this feature ELF is good enough.  bzImage
isn't.

Given that in the worst case distro's will have to package a second
binary of the same kernel in their kernel rpm.  I don't know that there
is any point in supporting anything else except ELF in the kernel.
Given that the package and distribution are going to have to change
anyway to include signing a change in file format hardly seems scary.

But my point above was really that ELF is sufficient for the use case of
doing file based verification base on fd's in addition to the use case
of using detached signatures.  Which is really a long winded way of
saying the argument "But but but my distro only ships a bzImage today"
is a horrible techinical argument.

I am not fundamentally opposed to supporting other file formats but
given that ELF wins on both practical and techincal grounds ELF should
be the primary file format for kexec_file_load.  We can worry about
other file formats once ELF is shown to work.

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


Re: [PATCH 4/6] kexec: A new system call, kexec_file_load, for in kernel kexec

2013-12-20 Thread H. Peter Anvin
On 12/20/2013 03:11 PM, Eric W. Biederman wrote:
> 
> In that case the chrome folks would simply have to use an ELF format
> kernel and not a bzImage.
> 

This is starting to feel like everything is going in the direction of a
massive feature regression.  bzImage may be weird (it has definitely
grown organically), but the features that have been added to it have
generally been for a reason, e.g. kernel relocation and so on.

-hpa


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


Re: [PATCH 4/6] kexec: A new system call, kexec_file_load, for in kernel kexec

2013-12-20 Thread Kees Cook
On Fri, Dec 20, 2013 at 3:11 PM, Eric W. Biederman
 wrote:
> Vivek Goyal  writes:
>
>> On Thu, Dec 19, 2013 at 01:54:39PM +0100, Torsten Duwe wrote:
>>> On Tue, Nov 26, 2013 at 09:27:59AM -0500, Vivek Goyal wrote:
>
>>> IMO it's up to user land to search lists of certificates, and present
>>> only the final chain of trust to the kernel for checking.
>>>
>>> ELF is the preferred format for most sane OSes and firmware, and a detached
>>> signature would probably be simplest to check. If we have the choice,
>>> without restrictions from braindead boot loaders, ELF should be first.
>>> And if the pesigning isn't usable and another sig is needed anyway,
>>> why not apply that to vmlinux(.gz) ?
>>
>> I have yet to look deeper into it that if we can sign elf images and
>> just use elf loader. And can use space extract the elf image out of
>> a bzImage and pass it to kernel.
>>
>> Even if it is doable, one disadvantage seemed to be that extracted
>> elf images will have to be written to a file so thta it's file descriptor
>> can be passed to kernel. And that assumed writable root and we chrome
>> folks seems to have setups where root is not writable.
>
> In that case the chrome folks would simply have to use an ELF format
> kernel and not a bzImage.

If we're doing fd origin verification (not signatures), can't we
continue to use a regular bzImage?

-Kees

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


Re: [PATCH 4/6] kexec: A new system call, kexec_file_load, for in kernel kexec

2013-12-20 Thread Eric W. Biederman
Vivek Goyal  writes:

> On Thu, Dec 19, 2013 at 01:54:39PM +0100, Torsten Duwe wrote:
>> On Tue, Nov 26, 2013 at 09:27:59AM -0500, Vivek Goyal wrote:

>> IMO it's up to user land to search lists of certificates, and present
>> only the final chain of trust to the kernel for checking.
>> 
>> ELF is the preferred format for most sane OSes and firmware, and a detached
>> signature would probably be simplest to check. If we have the choice,
>> without restrictions from braindead boot loaders, ELF should be first.
>> And if the pesigning isn't usable and another sig is needed anyway,
>> why not apply that to vmlinux(.gz) ?
>
> I have yet to look deeper into it that if we can sign elf images and
> just use elf loader. And can use space extract the elf image out of 
> a bzImage and pass it to kernel.
>
> Even if it is doable, one disadvantage seemed to be that extracted 
> elf images will have to be written to a file so thta it's file descriptor
> can be passed to kernel. And that assumed writable root and we chrome
> folks seems to have setups where root is not writable.

In that case the chrome folks would simply have to use an ELF format
kernel and not a bzImage.

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


Re: [PATCH 4/6] kexec: A new system call, kexec_file_load, for in kernel kexec

2013-12-20 Thread Vivek Goyal
On Thu, Dec 19, 2013 at 01:54:39PM +0100, Torsten Duwe wrote:
> On Tue, Nov 26, 2013 at 09:27:59AM -0500, Vivek Goyal wrote:
> > On Tue, Nov 26, 2013 at 04:23:36AM -0800, Eric W. Biederman wrote:
> > > Vivek Goyal  writes:
> > > 
> > > > On Fri, Nov 22, 2013 at 07:39:14PM -0800, Eric W. Biederman wrote:
> > > >
> > > 
> > > The init_size should be reflected in the .bss of the ELF segments.  If
> > > not it is a bug when generating the kernel ELF headers and should be
> > > fixed.
> > > 
> > > For use by kexec I don't see any issues with just signing the embedded
> > > ELF image.
> > 
> > Being able to write kernel to a file and then load it feels little odd to
> > me. Though this should be allowed but this should not be mandatory.
> > 
> > I think if we allow passing detached signature in kexec system call, then
> > it makes it much more flexible. We should be able to do what you are
> > suggesting at the same time it will also keep the possibility open for what
> > chromeOS developers are looking for.
> > 
> Support for detached signatures would be a big plus for kexec_file_load.
> 
> First I thought, some vendors are already shipping signed bzImages, why not
> verify these? But as it turns out, parsing MS-DOS, PE/COFF headers just to
> find the gaps is a lot of bloat for this little functionality. And David 
> Howells
> got flamed quite badly when he suggested to add pkcs#7 to the kernel.

Yep, that's the reason I am not proposing parsing and verifying PE/COFF
signatures.

> IMO it's up to user land to search lists of certificates, and present
> only the final chain of trust to the kernel for checking.
> 
> ELF is the preferred format for most sane OSes and firmware, and a detached
> signature would probably be simplest to check. If we have the choice,
> without restrictions from braindead boot loaders, ELF should be first.
> And if the pesigning isn't usable and another sig is needed anyway,
> why not apply that to vmlinux(.gz) ?

I have yet to look deeper into it that if we can sign elf images and
just use elf loader. And can use space extract the elf image out of 
a bzImage and pass it to kernel.

Even if it is doable, one disadvantage seemed to be that extracted 
elf images will have to be written to a file so thta it's file descriptor
can be passed to kernel. And that assumed writable root and we chrome
folks seems to have setups where root is not writable.

> 
> Another remaining issue is the root of trust. Should the kernel solely depend
> on UeFI to check certificates? I'd rather vote for a compile-time fallback 
> key.

I was thinking of signing kernel along the lines of modules. That is it is
RSA PKCS1 signatures. It does not contain certifiate chain. Signing key
should be in kernel and that key either can come from UEFI db, or user
might have added it in MOK during boot or we can embed one during kernel
build etc. Verification code will not care how did key show up in kernel.
There could be multiple ways.

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


Re: [PATCH 4/6] kexec: A new system call, kexec_file_load, for in kernel kexec

2013-12-20 Thread Vivek Goyal
On Thu, Dec 19, 2013 at 01:54:39PM +0100, Torsten Duwe wrote:
 On Tue, Nov 26, 2013 at 09:27:59AM -0500, Vivek Goyal wrote:
  On Tue, Nov 26, 2013 at 04:23:36AM -0800, Eric W. Biederman wrote:
   Vivek Goyal vgo...@redhat.com writes:
   
On Fri, Nov 22, 2013 at 07:39:14PM -0800, Eric W. Biederman wrote:
   
   
   The init_size should be reflected in the .bss of the ELF segments.  If
   not it is a bug when generating the kernel ELF headers and should be
   fixed.
   
   For use by kexec I don't see any issues with just signing the embedded
   ELF image.
  
  Being able to write kernel to a file and then load it feels little odd to
  me. Though this should be allowed but this should not be mandatory.
  
  I think if we allow passing detached signature in kexec system call, then
  it makes it much more flexible. We should be able to do what you are
  suggesting at the same time it will also keep the possibility open for what
  chromeOS developers are looking for.
  
 Support for detached signatures would be a big plus for kexec_file_load.
 
 First I thought, some vendors are already shipping signed bzImages, why not
 verify these? But as it turns out, parsing MS-DOS, PE/COFF headers just to
 find the gaps is a lot of bloat for this little functionality. And David 
 Howells
 got flamed quite badly when he suggested to add pkcs#7 to the kernel.

Yep, that's the reason I am not proposing parsing and verifying PE/COFF
signatures.

 IMO it's up to user land to search lists of certificates, and present
 only the final chain of trust to the kernel for checking.
 
 ELF is the preferred format for most sane OSes and firmware, and a detached
 signature would probably be simplest to check. If we have the choice,
 without restrictions from braindead boot loaders, ELF should be first.
 And if the pesigning isn't usable and another sig is needed anyway,
 why not apply that to vmlinux(.gz) ?

I have yet to look deeper into it that if we can sign elf images and
just use elf loader. And can use space extract the elf image out of 
a bzImage and pass it to kernel.

Even if it is doable, one disadvantage seemed to be that extracted 
elf images will have to be written to a file so thta it's file descriptor
can be passed to kernel. And that assumed writable root and we chrome
folks seems to have setups where root is not writable.

 
 Another remaining issue is the root of trust. Should the kernel solely depend
 on UeFI to check certificates? I'd rather vote for a compile-time fallback 
 key.

I was thinking of signing kernel along the lines of modules. That is it is
RSA PKCS1 signatures. It does not contain certifiate chain. Signing key
should be in kernel and that key either can come from UEFI db, or user
might have added it in MOK during boot or we can embed one during kernel
build etc. Verification code will not care how did key show up in kernel.
There could be multiple ways.

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


Re: [PATCH 4/6] kexec: A new system call, kexec_file_load, for in kernel kexec

2013-12-20 Thread Eric W. Biederman
Vivek Goyal vgo...@redhat.com writes:

 On Thu, Dec 19, 2013 at 01:54:39PM +0100, Torsten Duwe wrote:
 On Tue, Nov 26, 2013 at 09:27:59AM -0500, Vivek Goyal wrote:

 IMO it's up to user land to search lists of certificates, and present
 only the final chain of trust to the kernel for checking.
 
 ELF is the preferred format for most sane OSes and firmware, and a detached
 signature would probably be simplest to check. If we have the choice,
 without restrictions from braindead boot loaders, ELF should be first.
 And if the pesigning isn't usable and another sig is needed anyway,
 why not apply that to vmlinux(.gz) ?

 I have yet to look deeper into it that if we can sign elf images and
 just use elf loader. And can use space extract the elf image out of 
 a bzImage and pass it to kernel.

 Even if it is doable, one disadvantage seemed to be that extracted 
 elf images will have to be written to a file so thta it's file descriptor
 can be passed to kernel. And that assumed writable root and we chrome
 folks seems to have setups where root is not writable.

In that case the chrome folks would simply have to use an ELF format
kernel and not a bzImage.

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


Re: [PATCH 4/6] kexec: A new system call, kexec_file_load, for in kernel kexec

2013-12-20 Thread Kees Cook
On Fri, Dec 20, 2013 at 3:11 PM, Eric W. Biederman
ebied...@xmission.com wrote:
 Vivek Goyal vgo...@redhat.com writes:

 On Thu, Dec 19, 2013 at 01:54:39PM +0100, Torsten Duwe wrote:
 On Tue, Nov 26, 2013 at 09:27:59AM -0500, Vivek Goyal wrote:

 IMO it's up to user land to search lists of certificates, and present
 only the final chain of trust to the kernel for checking.

 ELF is the preferred format for most sane OSes and firmware, and a detached
 signature would probably be simplest to check. If we have the choice,
 without restrictions from braindead boot loaders, ELF should be first.
 And if the pesigning isn't usable and another sig is needed anyway,
 why not apply that to vmlinux(.gz) ?

 I have yet to look deeper into it that if we can sign elf images and
 just use elf loader. And can use space extract the elf image out of
 a bzImage and pass it to kernel.

 Even if it is doable, one disadvantage seemed to be that extracted
 elf images will have to be written to a file so thta it's file descriptor
 can be passed to kernel. And that assumed writable root and we chrome
 folks seems to have setups where root is not writable.

 In that case the chrome folks would simply have to use an ELF format
 kernel and not a bzImage.

If we're doing fd origin verification (not signatures), can't we
continue to use a regular bzImage?

-Kees

-- 
Kees Cook
Chrome OS Security
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 4/6] kexec: A new system call, kexec_file_load, for in kernel kexec

2013-12-20 Thread H. Peter Anvin
On 12/20/2013 03:11 PM, Eric W. Biederman wrote:
 
 In that case the chrome folks would simply have to use an ELF format
 kernel and not a bzImage.
 

This is starting to feel like everything is going in the direction of a
massive feature regression.  bzImage may be weird (it has definitely
grown organically), but the features that have been added to it have
generally been for a reason, e.g. kernel relocation and so on.

-hpa


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


Re: [PATCH 4/6] kexec: A new system call, kexec_file_load, for in kernel kexec

2013-12-20 Thread Eric W. Biederman
H. Peter Anvin h...@zytor.com writes:

 On 12/20/2013 03:11 PM, Eric W. Biederman wrote:
 
 In that case the chrome folks would simply have to use an ELF format
 kernel and not a bzImage.
 

 This is starting to feel like everything is going in the direction of a
 massive feature regression.  bzImage may be weird (it has definitely
 grown organically), but the features that have been added to it have
 generally been for a reason, e.g. kernel relocation and so on.

Stuff and nonsense.  bzImage is just an ugly wrapper around an ELF
image.

I am just arguing that we expose the clean portable underpinnings and
make that work.

It absolutely does not make sense to make a solution that only works for
x86.  ELF is what ever other architecture uses so we absolutely have to
make any feature we build work with ELF.

At a very basic level for this feature ELF is good enough.  bzImage
isn't.

Given that in the worst case distro's will have to package a second
binary of the same kernel in their kernel rpm.  I don't know that there
is any point in supporting anything else except ELF in the kernel.
Given that the package and distribution are going to have to change
anyway to include signing a change in file format hardly seems scary.

But my point above was really that ELF is sufficient for the use case of
doing file based verification base on fd's in addition to the use case
of using detached signatures.  Which is really a long winded way of
saying the argument But but but my distro only ships a bzImage today
is a horrible techinical argument.

I am not fundamentally opposed to supporting other file formats but
given that ELF wins on both practical and techincal grounds ELF should
be the primary file format for kexec_file_load.  We can worry about
other file formats once ELF is shown to work.

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


Re: [PATCH 4/6] kexec: A new system call, kexec_file_load, for in kernel kexec

2013-12-20 Thread H. Peter Anvin
On 12/20/2013 05:32 PM, Eric W. Biederman wrote:
 
 Stuff and nonsense.  bzImage is just an ugly wrapper around an ELF
 image.
 

Not really.  We put the ELF image in there to help Xen and presumably
kexec, but there are actually quite a few issues with it... for one
thing, as currently built there are megabytes of zeroes in it for no
good reason.

Even if you don't need the entry code, the additional metadata is
meaningful.

-hpa


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


Re: [PATCH 4/6] kexec: A new system call, kexec_file_load, for in kernel kexec

2013-12-19 Thread Torsten Duwe
On Tue, Nov 26, 2013 at 09:27:59AM -0500, Vivek Goyal wrote:
> On Tue, Nov 26, 2013 at 04:23:36AM -0800, Eric W. Biederman wrote:
> > Vivek Goyal  writes:
> > 
> > > On Fri, Nov 22, 2013 at 07:39:14PM -0800, Eric W. Biederman wrote:
> > >
> > 
> > The init_size should be reflected in the .bss of the ELF segments.  If
> > not it is a bug when generating the kernel ELF headers and should be
> > fixed.
> > 
> > For use by kexec I don't see any issues with just signing the embedded
> > ELF image.
> 
> Being able to write kernel to a file and then load it feels little odd to
> me. Though this should be allowed but this should not be mandatory.
> 
> I think if we allow passing detached signature in kexec system call, then
> it makes it much more flexible. We should be able to do what you are
> suggesting at the same time it will also keep the possibility open for what
> chromeOS developers are looking for.
> 
Support for detached signatures would be a big plus for kexec_file_load.

First I thought, some vendors are already shipping signed bzImages, why not
verify these? But as it turns out, parsing MS-DOS, PE/COFF headers just to
find the gaps is a lot of bloat for this little functionality. And David Howells
got flamed quite badly when he suggested to add pkcs#7 to the kernel.
IMO it's up to user land to search lists of certificates, and present
only the final chain of trust to the kernel for checking.

ELF is the preferred format for most sane OSes and firmware, and a detached
signature would probably be simplest to check. If we have the choice,
without restrictions from braindead boot loaders, ELF should be first.
And if the pesigning isn't usable and another sig is needed anyway,
why not apply that to vmlinux(.gz) ?

Another remaining issue is the root of trust. Should the kernel solely depend
on UeFI to check certificates? I'd rather vote for a compile-time fallback key.

Torsten

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


Re: [PATCH 4/6] kexec: A new system call, kexec_file_load, for in kernel kexec

2013-12-19 Thread Torsten Duwe
On Tue, Nov 26, 2013 at 09:27:59AM -0500, Vivek Goyal wrote:
 On Tue, Nov 26, 2013 at 04:23:36AM -0800, Eric W. Biederman wrote:
  Vivek Goyal vgo...@redhat.com writes:
  
   On Fri, Nov 22, 2013 at 07:39:14PM -0800, Eric W. Biederman wrote:
  
  
  The init_size should be reflected in the .bss of the ELF segments.  If
  not it is a bug when generating the kernel ELF headers and should be
  fixed.
  
  For use by kexec I don't see any issues with just signing the embedded
  ELF image.
 
 Being able to write kernel to a file and then load it feels little odd to
 me. Though this should be allowed but this should not be mandatory.
 
 I think if we allow passing detached signature in kexec system call, then
 it makes it much more flexible. We should be able to do what you are
 suggesting at the same time it will also keep the possibility open for what
 chromeOS developers are looking for.
 
Support for detached signatures would be a big plus for kexec_file_load.

First I thought, some vendors are already shipping signed bzImages, why not
verify these? But as it turns out, parsing MS-DOS, PE/COFF headers just to
find the gaps is a lot of bloat for this little functionality. And David Howells
got flamed quite badly when he suggested to add pkcs#7 to the kernel.
IMO it's up to user land to search lists of certificates, and present
only the final chain of trust to the kernel for checking.

ELF is the preferred format for most sane OSes and firmware, and a detached
signature would probably be simplest to check. If we have the choice,
without restrictions from braindead boot loaders, ELF should be first.
And if the pesigning isn't usable and another sig is needed anyway,
why not apply that to vmlinux(.gz) ?

Another remaining issue is the root of trust. Should the kernel solely depend
on UeFI to check certificates? I'd rather vote for a compile-time fallback key.

Torsten

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


Re: [PATCH 4/6] kexec: A new system call, kexec_file_load, for in kernel kexec

2013-12-04 Thread Vivek Goyal
On Wed, Dec 04, 2013 at 09:56:57AM +0800, Baoquan He wrote:
> On 11/20/13 at 12:50pm, Vivek Goyal wrote:
> > + * that kexec_mutex is held.
> > + */
> 
> I think kexec_add_buffer is guaranteed to be called before allocating
> control pages, why not updating image->control_page after each time
> kexec_add_buffer is called. Then when control page is needed, effective
> address in crash_kernel region can be given. This can be a little more
> efficient.

image->control_page controls the lowest address available for control
pages in crash kernel region. When we do kexec_add_buffer, we don't
necessarily know whether there is an empty page available between
segments or not.

Also, existing logic for kexec does not update the image->control_page
when segments are being copied.

So I think this does not offer any huge benefits and it is not performance
critical path. I will just leave it as it is.

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


Re: [PATCH 4/6] kexec: A new system call, kexec_file_load, for in kernel kexec

2013-12-04 Thread Vivek Goyal
On Wed, Dec 04, 2013 at 09:35:29AM +0800, Baoquan He wrote:
> On 12/02/13 at 10:44am, Vivek Goyal wrote:
> > On Fri, Nov 29, 2013 at 11:10:48AM +0800, Baoquan He wrote:
> > 
> > [..]
> > > > +void *arch_kexec_kernel_image_load(struct kimage *image, char *kernel,
> > > > +   unsigned long kernel_len, char *initrd,
> > > > +   unsigned long initrd_len, char *cmdline,
> > > > +   unsigned long cmdline_len)
> > > > +{
> > > > +   int idx = image->file_handler_idx;
> > > > +
> > > > +   if (idx < 0)
> > > > +   return ERR_PTR(-ENOEXEC);
> > > > +
> > > > +   return kexec_file_type[idx].load(image, kernel, kernel_len, 
> > > > initrd,
> > > > +   initrd_len, cmdline, 
> > > > cmdline_len);
> > > > +}
> > > > +
> > > > +int arch_image_file_post_load_cleanup(struct kimage *image)
> > > > +{
> > > 
> > > Hi Vivek,
> > > 
> > > This function is defined as one of arch specific fucntion set, why don't
> > > we name it in a unified prefix as others.
> > 
> > I am using "arch_" prefix. What else to use?
> 
> I mean in this function series, other functions have name like
> arch_kexec_kernel_image_xxx, why this function is lonely, and is named
> as arch_kimage_xxx. And here what does the 'k' mean in "kimage", kexec
> image or kernel image, I am confused.

kimage is name is structure which is containing all the data relevant
to loading of file. 

I guess I can use arch_kimage_file_post_load_cleanup().

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


Re: [PATCH 4/6] kexec: A new system call, kexec_file_load, for in kernel kexec

2013-12-04 Thread Baoquan He
On 12/04/13 at 09:56am, Baoquan He wrote:
> On 11/20/13 at 12:50pm, Vivek Goyal wrote:
> > + * that kexec_mutex is held.
> > + */
> 
> I think kexec_add_buffer is guaranteed to be called before allocating
> control pages, why not updating image->control_page after each time
> kexec_add_buffer is called. Then when control page is needed, effective
> address in crash_kernel region can be given. This can be a little more
> efficient.

Check again, it's not a good idea. Because buf_align is different when
kexec_add_buffer is called. Sometime the bug_align is larger than 4k.
Please ignore this comment.

> 
> > +int kexec_add_buffer(struct kimage *image, char *buffer,
> > +   unsigned long bufsz, unsigned long memsz,
> > +   unsigned long buf_align, unsigned long buf_min,
> > +   unsigned long buf_max, int top_down, unsigned long *load_addr)
> > +{
> > +
> > +   unsigned long nr_segments = image->nr_segments, new_nr_segments;
> > +   struct kexec_segment *ksegment;
> > +   struct kexec_buf *kbuf;
> > +
> > +   /* Currently adding segment this way is allowed only in file mode */
> > +   if (!image->file_mode)
> > +   return -EINVAL;
> > +
> > +   if (nr_segments >= KEXEC_SEGMENT_MAX)
> > +   return -EINVAL;
> > +
> > +   /*
> > +* Make sure we are not trying to add buffer after allocating
> > +* control pages. All segments need to be placed first before
> > +* any control pages are allocated. As control page allocation
> > +* logic goes through list of segments to make sure there are
> > +* no destination overlaps.
> > +*/
> > +   WARN_ONCE(!list_empty(>control_pages), "Adding kexec buffer"
> > +   " after allocating control pages\n");
> > +
> > +   kbuf = kzalloc(sizeof(struct kexec_buf), GFP_KERNEL);
> > +   if (!kbuf)
> > +   return -ENOMEM;
> > +

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


Re: [PATCH 4/6] kexec: A new system call, kexec_file_load, for in kernel kexec

2013-12-04 Thread Baoquan He
On 12/04/13 at 09:56am, Baoquan He wrote:
 On 11/20/13 at 12:50pm, Vivek Goyal wrote:
  + * that kexec_mutex is held.
  + */
 
 I think kexec_add_buffer is guaranteed to be called before allocating
 control pages, why not updating image-control_page after each time
 kexec_add_buffer is called. Then when control page is needed, effective
 address in crash_kernel region can be given. This can be a little more
 efficient.

Check again, it's not a good idea. Because buf_align is different when
kexec_add_buffer is called. Sometime the bug_align is larger than 4k.
Please ignore this comment.

 
  +int kexec_add_buffer(struct kimage *image, char *buffer,
  +   unsigned long bufsz, unsigned long memsz,
  +   unsigned long buf_align, unsigned long buf_min,
  +   unsigned long buf_max, int top_down, unsigned long *load_addr)
  +{
  +
  +   unsigned long nr_segments = image-nr_segments, new_nr_segments;
  +   struct kexec_segment *ksegment;
  +   struct kexec_buf *kbuf;
  +
  +   /* Currently adding segment this way is allowed only in file mode */
  +   if (!image-file_mode)
  +   return -EINVAL;
  +
  +   if (nr_segments = KEXEC_SEGMENT_MAX)
  +   return -EINVAL;
  +
  +   /*
  +* Make sure we are not trying to add buffer after allocating
  +* control pages. All segments need to be placed first before
  +* any control pages are allocated. As control page allocation
  +* logic goes through list of segments to make sure there are
  +* no destination overlaps.
  +*/
  +   WARN_ONCE(!list_empty(image-control_pages), Adding kexec buffer
  +after allocating control pages\n);
  +
  +   kbuf = kzalloc(sizeof(struct kexec_buf), GFP_KERNEL);
  +   if (!kbuf)
  +   return -ENOMEM;
  +

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


Re: [PATCH 4/6] kexec: A new system call, kexec_file_load, for in kernel kexec

2013-12-04 Thread Vivek Goyal
On Wed, Dec 04, 2013 at 09:35:29AM +0800, Baoquan He wrote:
 On 12/02/13 at 10:44am, Vivek Goyal wrote:
  On Fri, Nov 29, 2013 at 11:10:48AM +0800, Baoquan He wrote:
  
  [..]
+void *arch_kexec_kernel_image_load(struct kimage *image, char *kernel,
+   unsigned long kernel_len, char *initrd,
+   unsigned long initrd_len, char *cmdline,
+   unsigned long cmdline_len)
+{
+   int idx = image-file_handler_idx;
+
+   if (idx  0)
+   return ERR_PTR(-ENOEXEC);
+
+   return kexec_file_type[idx].load(image, kernel, kernel_len, 
initrd,
+   initrd_len, cmdline, 
cmdline_len);
+}
+
+int arch_image_file_post_load_cleanup(struct kimage *image)
+{
   
   Hi Vivek,
   
   This function is defined as one of arch specific fucntion set, why don't
   we name it in a unified prefix as others.
  
  I am using arch_ prefix. What else to use?
 
 I mean in this function series, other functions have name like
 arch_kexec_kernel_image_xxx, why this function is lonely, and is named
 as arch_kimage_xxx. And here what does the 'k' mean in kimage, kexec
 image or kernel image, I am confused.

kimage is name is structure which is containing all the data relevant
to loading of file. 

I guess I can use arch_kimage_file_post_load_cleanup().

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


Re: [PATCH 4/6] kexec: A new system call, kexec_file_load, for in kernel kexec

2013-12-04 Thread Vivek Goyal
On Wed, Dec 04, 2013 at 09:56:57AM +0800, Baoquan He wrote:
 On 11/20/13 at 12:50pm, Vivek Goyal wrote:
  + * that kexec_mutex is held.
  + */
 
 I think kexec_add_buffer is guaranteed to be called before allocating
 control pages, why not updating image-control_page after each time
 kexec_add_buffer is called. Then when control page is needed, effective
 address in crash_kernel region can be given. This can be a little more
 efficient.

image-control_page controls the lowest address available for control
pages in crash kernel region. When we do kexec_add_buffer, we don't
necessarily know whether there is an empty page available between
segments or not.

Also, existing logic for kexec does not update the image-control_page
when segments are being copied.

So I think this does not offer any huge benefits and it is not performance
critical path. I will just leave it as it is.

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


Re: [PATCH 4/6] kexec: A new system call, kexec_file_load, for in kernel kexec

2013-12-03 Thread Baoquan He
On 11/20/13 at 12:50pm, Vivek Goyal wrote:
> + * that kexec_mutex is held.
> + */

I think kexec_add_buffer is guaranteed to be called before allocating
control pages, why not updating image->control_page after each time
kexec_add_buffer is called. Then when control page is needed, effective
address in crash_kernel region can be given. This can be a little more
efficient.

> +int kexec_add_buffer(struct kimage *image, char *buffer,
> + unsigned long bufsz, unsigned long memsz,
> + unsigned long buf_align, unsigned long buf_min,
> + unsigned long buf_max, int top_down, unsigned long *load_addr)
> +{
> +
> + unsigned long nr_segments = image->nr_segments, new_nr_segments;
> + struct kexec_segment *ksegment;
> + struct kexec_buf *kbuf;
> +
> + /* Currently adding segment this way is allowed only in file mode */
> + if (!image->file_mode)
> + return -EINVAL;
> +
> + if (nr_segments >= KEXEC_SEGMENT_MAX)
> + return -EINVAL;
> +
> + /*
> +  * Make sure we are not trying to add buffer after allocating
> +  * control pages. All segments need to be placed first before
> +  * any control pages are allocated. As control page allocation
> +  * logic goes through list of segments to make sure there are
> +  * no destination overlaps.
> +  */
> + WARN_ONCE(!list_empty(>control_pages), "Adding kexec buffer"
> + " after allocating control pages\n");
> +
> + kbuf = kzalloc(sizeof(struct kexec_buf), GFP_KERNEL);
> + if (!kbuf)
> + return -ENOMEM;
> +
> + kbuf->image = image;
> + kbuf->buffer = buffer;
> + kbuf->bufsz = bufsz;
> + /* Align memsz to next page boundary */
> + kbuf->memsz = ALIGN(memsz, PAGE_SIZE);
> +
> + /* Align to atleast page size boundary */
> + kbuf->buf_align = max(buf_align, PAGE_SIZE);
> + kbuf->buf_min = buf_min;
> + kbuf->buf_max = buf_max;
> + kbuf->top_down = top_down;
> +
> + /* Walk the RAM ranges and allocate a suitable range for the buffer */
> + walk_system_ram_res(0, -1, kbuf, walk_ram_range_callback);
> +
> + kbuf->image = NULL;
> + kfree(kbuf);
> +
> + /*
> +  * If range could be found successfully, it would have incremented
> +  * the nr_segments value.
> +  */
> + new_nr_segments = image->nr_segments;
> +
> + /* A suitable memory range could not be found for buffer */
> + if (new_nr_segments == nr_segments)
> + return -EADDRNOTAVAIL;
> +
> + /* Found a suitable memory range */
> +
> + ksegment = >segment[new_nr_segments - 1];
> + *load_addr = ksegment->mem;
> + return 0;
> +}
> +
> +

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


Re: [PATCH 4/6] kexec: A new system call, kexec_file_load, for in kernel kexec

2013-12-03 Thread Baoquan He
On 12/02/13 at 10:44am, Vivek Goyal wrote:
> On Fri, Nov 29, 2013 at 11:10:48AM +0800, Baoquan He wrote:
> 
> [..]
> > > +void *arch_kexec_kernel_image_load(struct kimage *image, char *kernel,
> > > + unsigned long kernel_len, char *initrd,
> > > + unsigned long initrd_len, char *cmdline,
> > > + unsigned long cmdline_len)
> > > +{
> > > + int idx = image->file_handler_idx;
> > > +
> > > + if (idx < 0)
> > > + return ERR_PTR(-ENOEXEC);
> > > +
> > > + return kexec_file_type[idx].load(image, kernel, kernel_len, initrd,
> > > + initrd_len, cmdline, cmdline_len);
> > > +}
> > > +
> > > +int arch_image_file_post_load_cleanup(struct kimage *image)
> > > +{
> > 
> > Hi Vivek,
> > 
> > This function is defined as one of arch specific fucntion set, why don't
> > we name it in a unified prefix as others.
> 
> I am using "arch_" prefix. What else to use?

I mean in this function series, other functions have name like
arch_kexec_kernel_image_xxx, why this function is lonely, and is named
as arch_kimage_xxx. And here what does the 'k' mean in "kimage", kexec
image or kernel image, I am confused.

> 
> > 
> > And name of the default dummy function in kernel/kexec.c is not consistent
> > with the arch specific one, so currently
> > arch_image_file_post_load_cleanup of x86 arch is not called. Please
> > consider wihch one need be changed.
> 
> Good catch Bao. I should change arch_image_file_post_load_cleanup() to
> arch_kimage_file_post_load_cleanup(), otherwise it never gets called and
> memory leaks.
> 
> Thanks
> Vivek
> 
> ___
> kexec mailing list
> ke...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 4/6] kexec: A new system call, kexec_file_load, for in kernel kexec

2013-12-03 Thread Baoquan He
On 12/02/13 at 10:44am, Vivek Goyal wrote:
 On Fri, Nov 29, 2013 at 11:10:48AM +0800, Baoquan He wrote:
 
 [..]
   +void *arch_kexec_kernel_image_load(struct kimage *image, char *kernel,
   + unsigned long kernel_len, char *initrd,
   + unsigned long initrd_len, char *cmdline,
   + unsigned long cmdline_len)
   +{
   + int idx = image-file_handler_idx;
   +
   + if (idx  0)
   + return ERR_PTR(-ENOEXEC);
   +
   + return kexec_file_type[idx].load(image, kernel, kernel_len, initrd,
   + initrd_len, cmdline, cmdline_len);
   +}
   +
   +int arch_image_file_post_load_cleanup(struct kimage *image)
   +{
  
  Hi Vivek,
  
  This function is defined as one of arch specific fucntion set, why don't
  we name it in a unified prefix as others.
 
 I am using arch_ prefix. What else to use?

I mean in this function series, other functions have name like
arch_kexec_kernel_image_xxx, why this function is lonely, and is named
as arch_kimage_xxx. And here what does the 'k' mean in kimage, kexec
image or kernel image, I am confused.

 
  
  And name of the default dummy function in kernel/kexec.c is not consistent
  with the arch specific one, so currently
  arch_image_file_post_load_cleanup of x86 arch is not called. Please
  consider wihch one need be changed.
 
 Good catch Bao. I should change arch_image_file_post_load_cleanup() to
 arch_kimage_file_post_load_cleanup(), otherwise it never gets called and
 memory leaks.
 
 Thanks
 Vivek
 
 ___
 kexec mailing list
 ke...@lists.infradead.org
 http://lists.infradead.org/mailman/listinfo/kexec
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 4/6] kexec: A new system call, kexec_file_load, for in kernel kexec

2013-12-03 Thread Baoquan He
On 11/20/13 at 12:50pm, Vivek Goyal wrote:
 + * that kexec_mutex is held.
 + */

I think kexec_add_buffer is guaranteed to be called before allocating
control pages, why not updating image-control_page after each time
kexec_add_buffer is called. Then when control page is needed, effective
address in crash_kernel region can be given. This can be a little more
efficient.

 +int kexec_add_buffer(struct kimage *image, char *buffer,
 + unsigned long bufsz, unsigned long memsz,
 + unsigned long buf_align, unsigned long buf_min,
 + unsigned long buf_max, int top_down, unsigned long *load_addr)
 +{
 +
 + unsigned long nr_segments = image-nr_segments, new_nr_segments;
 + struct kexec_segment *ksegment;
 + struct kexec_buf *kbuf;
 +
 + /* Currently adding segment this way is allowed only in file mode */
 + if (!image-file_mode)
 + return -EINVAL;
 +
 + if (nr_segments = KEXEC_SEGMENT_MAX)
 + return -EINVAL;
 +
 + /*
 +  * Make sure we are not trying to add buffer after allocating
 +  * control pages. All segments need to be placed first before
 +  * any control pages are allocated. As control page allocation
 +  * logic goes through list of segments to make sure there are
 +  * no destination overlaps.
 +  */
 + WARN_ONCE(!list_empty(image-control_pages), Adding kexec buffer
 +  after allocating control pages\n);
 +
 + kbuf = kzalloc(sizeof(struct kexec_buf), GFP_KERNEL);
 + if (!kbuf)
 + return -ENOMEM;
 +
 + kbuf-image = image;
 + kbuf-buffer = buffer;
 + kbuf-bufsz = bufsz;
 + /* Align memsz to next page boundary */
 + kbuf-memsz = ALIGN(memsz, PAGE_SIZE);
 +
 + /* Align to atleast page size boundary */
 + kbuf-buf_align = max(buf_align, PAGE_SIZE);
 + kbuf-buf_min = buf_min;
 + kbuf-buf_max = buf_max;
 + kbuf-top_down = top_down;
 +
 + /* Walk the RAM ranges and allocate a suitable range for the buffer */
 + walk_system_ram_res(0, -1, kbuf, walk_ram_range_callback);
 +
 + kbuf-image = NULL;
 + kfree(kbuf);
 +
 + /*
 +  * If range could be found successfully, it would have incremented
 +  * the nr_segments value.
 +  */
 + new_nr_segments = image-nr_segments;
 +
 + /* A suitable memory range could not be found for buffer */
 + if (new_nr_segments == nr_segments)
 + return -EADDRNOTAVAIL;
 +
 + /* Found a suitable memory range */
 +
 + ksegment = image-segment[new_nr_segments - 1];
 + *load_addr = ksegment-mem;
 + return 0;
 +}
 +
 +

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


Re: [PATCH 4/6] kexec: A new system call, kexec_file_load, for in kernel kexec

2013-12-02 Thread Vivek Goyal
On Fri, Nov 29, 2013 at 11:10:48AM +0800, Baoquan He wrote:

[..]
> > +void *arch_kexec_kernel_image_load(struct kimage *image, char *kernel,
> > +   unsigned long kernel_len, char *initrd,
> > +   unsigned long initrd_len, char *cmdline,
> > +   unsigned long cmdline_len)
> > +{
> > +   int idx = image->file_handler_idx;
> > +
> > +   if (idx < 0)
> > +   return ERR_PTR(-ENOEXEC);
> > +
> > +   return kexec_file_type[idx].load(image, kernel, kernel_len, initrd,
> > +   initrd_len, cmdline, cmdline_len);
> > +}
> > +
> > +int arch_image_file_post_load_cleanup(struct kimage *image)
> > +{
> 
> Hi Vivek,
> 
> This function is defined as one of arch specific fucntion set, why don't
> we name it in a unified prefix as others.

I am using "arch_" prefix. What else to use?

> 
> And name of the default dummy function in kernel/kexec.c is not consistent
> with the arch specific one, so currently
> arch_image_file_post_load_cleanup of x86 arch is not called. Please
> consider wihch one need be changed.

Good catch Bao. I should change arch_image_file_post_load_cleanup() to
arch_kimage_file_post_load_cleanup(), otherwise it never gets called and
memory leaks.

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


Re: [PATCH 4/6] kexec: A new system call, kexec_file_load, for in kernel kexec

2013-12-02 Thread WANG Chao
On 11/29/13 at 11:10am, Baoquan He wrote:
> On 11/20/13 at 12:50pm, Vivek Goyal wrote:
> > diff --git a/arch/x86/kernel/machine_kexec_64.c 
> > b/arch/x86/kernel/machine_kexec_64.c
> > index 4eabc16..fb41b73 100644
> > --- a/arch/x86/kernel/machine_kexec_64.c
> > +++ b/arch/x86/kernel/machine_kexec_64.c
> > @@ -22,6 +22,13 @@
> >  #include 
> >  #include 
> >  
> > +/* arch dependent functionality related to kexec file based syscall */
> > +static struct kexec_file_type kexec_file_type[]={
> > +   {"", NULL, NULL, NULL, NULL},
> > +};
> > +
> > +
> > +void *arch_kexec_kernel_image_load(struct kimage *image, char *kernel,
> > +   unsigned long kernel_len, char *initrd,
> > +   unsigned long initrd_len, char *cmdline,
> > +   unsigned long cmdline_len)
> > +{
> > +   int idx = image->file_handler_idx;
> > +
> > +   if (idx < 0)
> > +   return ERR_PTR(-ENOEXEC);
> > +
> > +   return kexec_file_type[idx].load(image, kernel, kernel_len, initrd,
> > +   initrd_len, cmdline, cmdline_len);
> > +}
> > +
> > +int arch_image_file_post_load_cleanup(struct kimage *image)
> > +{
> 
> Hi Vivek,
> 
> This function is defined as one of arch specific fucntion set, why don't
> we name it in a unified prefix as others.
> 
> And name of the default dummy function in kernel/kexec.c is not consistent
> with the arch specific one, so currently
> arch_image_file_post_load_cleanup of x86 arch is not called. Please
> consider wihch one need be changed.

I think arch_kimage_file_post_load_cleanup should be used here.

And Good catch!

Thanks
WANG Chao

> 
> > +
> > +void __attribute__ ((weak))
> > +arch_kimage_file_post_load_cleanup(struct kimage *image)
> > +{
> > +   return;
> > +}
> > +
> 
> 
> 
> ___
> kexec mailing list
> ke...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 4/6] kexec: A new system call, kexec_file_load, for in kernel kexec

2013-12-02 Thread WANG Chao
On 11/29/13 at 11:10am, Baoquan He wrote:
 On 11/20/13 at 12:50pm, Vivek Goyal wrote:
  diff --git a/arch/x86/kernel/machine_kexec_64.c 
  b/arch/x86/kernel/machine_kexec_64.c
  index 4eabc16..fb41b73 100644
  --- a/arch/x86/kernel/machine_kexec_64.c
  +++ b/arch/x86/kernel/machine_kexec_64.c
  @@ -22,6 +22,13 @@
   #include asm/mmu_context.h
   #include asm/debugreg.h
   
  +/* arch dependent functionality related to kexec file based syscall */
  +static struct kexec_file_type kexec_file_type[]={
  +   {, NULL, NULL, NULL, NULL},
  +};
  +
  +
  +void *arch_kexec_kernel_image_load(struct kimage *image, char *kernel,
  +   unsigned long kernel_len, char *initrd,
  +   unsigned long initrd_len, char *cmdline,
  +   unsigned long cmdline_len)
  +{
  +   int idx = image-file_handler_idx;
  +
  +   if (idx  0)
  +   return ERR_PTR(-ENOEXEC);
  +
  +   return kexec_file_type[idx].load(image, kernel, kernel_len, initrd,
  +   initrd_len, cmdline, cmdline_len);
  +}
  +
  +int arch_image_file_post_load_cleanup(struct kimage *image)
  +{
 
 Hi Vivek,
 
 This function is defined as one of arch specific fucntion set, why don't
 we name it in a unified prefix as others.
 
 And name of the default dummy function in kernel/kexec.c is not consistent
 with the arch specific one, so currently
 arch_image_file_post_load_cleanup of x86 arch is not called. Please
 consider wihch one need be changed.

I think arch_kimage_file_post_load_cleanup should be used here.

And Good catch!

Thanks
WANG Chao

 
  +
  +void __attribute__ ((weak))
  +arch_kimage_file_post_load_cleanup(struct kimage *image)
  +{
  +   return;
  +}
  +
 
 
 
 ___
 kexec mailing list
 ke...@lists.infradead.org
 http://lists.infradead.org/mailman/listinfo/kexec
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 4/6] kexec: A new system call, kexec_file_load, for in kernel kexec

2013-12-02 Thread Vivek Goyal
On Fri, Nov 29, 2013 at 11:10:48AM +0800, Baoquan He wrote:

[..]
  +void *arch_kexec_kernel_image_load(struct kimage *image, char *kernel,
  +   unsigned long kernel_len, char *initrd,
  +   unsigned long initrd_len, char *cmdline,
  +   unsigned long cmdline_len)
  +{
  +   int idx = image-file_handler_idx;
  +
  +   if (idx  0)
  +   return ERR_PTR(-ENOEXEC);
  +
  +   return kexec_file_type[idx].load(image, kernel, kernel_len, initrd,
  +   initrd_len, cmdline, cmdline_len);
  +}
  +
  +int arch_image_file_post_load_cleanup(struct kimage *image)
  +{
 
 Hi Vivek,
 
 This function is defined as one of arch specific fucntion set, why don't
 we name it in a unified prefix as others.

I am using arch_ prefix. What else to use?

 
 And name of the default dummy function in kernel/kexec.c is not consistent
 with the arch specific one, so currently
 arch_image_file_post_load_cleanup of x86 arch is not called. Please
 consider wihch one need be changed.

Good catch Bao. I should change arch_image_file_post_load_cleanup() to
arch_kimage_file_post_load_cleanup(), otherwise it never gets called and
memory leaks.

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


Re: [PATCH 4/6] kexec: A new system call, kexec_file_load, for in kernel kexec

2013-11-28 Thread Baoquan He
On 11/20/13 at 12:50pm, Vivek Goyal wrote:
> diff --git a/arch/x86/kernel/machine_kexec_64.c 
> b/arch/x86/kernel/machine_kexec_64.c
> index 4eabc16..fb41b73 100644
> --- a/arch/x86/kernel/machine_kexec_64.c
> +++ b/arch/x86/kernel/machine_kexec_64.c
> @@ -22,6 +22,13 @@
>  #include 
>  #include 
>  
> +/* arch dependent functionality related to kexec file based syscall */
> +static struct kexec_file_type kexec_file_type[]={
> + {"", NULL, NULL, NULL, NULL},
> +};
> +
> +
> +void *arch_kexec_kernel_image_load(struct kimage *image, char *kernel,
> + unsigned long kernel_len, char *initrd,
> + unsigned long initrd_len, char *cmdline,
> + unsigned long cmdline_len)
> +{
> + int idx = image->file_handler_idx;
> +
> + if (idx < 0)
> + return ERR_PTR(-ENOEXEC);
> +
> + return kexec_file_type[idx].load(image, kernel, kernel_len, initrd,
> + initrd_len, cmdline, cmdline_len);
> +}
> +
> +int arch_image_file_post_load_cleanup(struct kimage *image)
> +{

Hi Vivek,

This function is defined as one of arch specific fucntion set, why don't
we name it in a unified prefix as others.

And name of the default dummy function in kernel/kexec.c is not consistent
with the arch specific one, so currently
arch_image_file_post_load_cleanup of x86 arch is not called. Please
consider wihch one need be changed.

> +
> +void __attribute__ ((weak))
> +arch_kimage_file_post_load_cleanup(struct kimage *image)
> +{
> + return;
> +}
> +


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


Re: [PATCH 4/6] kexec: A new system call, kexec_file_load, for in kernel kexec

2013-11-28 Thread Baoquan He
On 11/20/13 at 12:50pm, Vivek Goyal wrote:
 diff --git a/arch/x86/kernel/machine_kexec_64.c 
 b/arch/x86/kernel/machine_kexec_64.c
 index 4eabc16..fb41b73 100644
 --- a/arch/x86/kernel/machine_kexec_64.c
 +++ b/arch/x86/kernel/machine_kexec_64.c
 @@ -22,6 +22,13 @@
  #include asm/mmu_context.h
  #include asm/debugreg.h
  
 +/* arch dependent functionality related to kexec file based syscall */
 +static struct kexec_file_type kexec_file_type[]={
 + {, NULL, NULL, NULL, NULL},
 +};
 +
 +
 +void *arch_kexec_kernel_image_load(struct kimage *image, char *kernel,
 + unsigned long kernel_len, char *initrd,
 + unsigned long initrd_len, char *cmdline,
 + unsigned long cmdline_len)
 +{
 + int idx = image-file_handler_idx;
 +
 + if (idx  0)
 + return ERR_PTR(-ENOEXEC);
 +
 + return kexec_file_type[idx].load(image, kernel, kernel_len, initrd,
 + initrd_len, cmdline, cmdline_len);
 +}
 +
 +int arch_image_file_post_load_cleanup(struct kimage *image)
 +{

Hi Vivek,

This function is defined as one of arch specific fucntion set, why don't
we name it in a unified prefix as others.

And name of the default dummy function in kernel/kexec.c is not consistent
with the arch specific one, so currently
arch_image_file_post_load_cleanup of x86 arch is not called. Please
consider wihch one need be changed.

 +
 +void __attribute__ ((weak))
 +arch_kimage_file_post_load_cleanup(struct kimage *image)
 +{
 + return;
 +}
 +


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


Re: [PATCH 4/6] kexec: A new system call, kexec_file_load, for in kernel kexec

2013-11-26 Thread Vivek Goyal
On Tue, Nov 26, 2013 at 04:23:36AM -0800, Eric W. Biederman wrote:
> Vivek Goyal  writes:
> 
> > On Fri, Nov 22, 2013 at 07:39:14PM -0800, Eric W. Biederman wrote:
> >
> > [..]
> >> > Hmm..., I am running out of ideas here. This is what I understand.
> >> >
> >> > - If I sign the bzImage (using PKCS1.5 signature), and later it is signed
> >> >   with authenticode format signatures, then PKCS1.5 signatures will not 
> >> > be
> >> >   valid as PE/COFF signing will do some modification to PE/COFF header in
> >> >   bzImage. And another problem is that then I don't have a way to find
> >> >   PKCS1.5 signature.
> >> >
> >> > - If bzImage is first signed with authenticode format signature and then
> >> >   signed using PKCS1.5 signature, then  authenticode format signature
> >> >   will become invalid as it will also hash the data appened at the end
> >> >   of file.
> >> >
> >> > So looks like both signatures can't co-exist on same file. That means
> >> > one signature has to be detached.
> >> >
> >> > I am beginning to think that create a kernel option which allows to 
> >> > choose
> >> > between attached and detached signatures. Extend kexec syscall to allow
> >> > a parameter to pass in detached signatures. If detached signatures are
> >> > not passed, then look for signatures at the end of file. That way, those
> >> > who are signing kernels using platform specific format (authenticode) in 
> >> > this case, they can generate detached signature while others can just
> >> > use attached signatures.
> >> >
> >> > Any thoughts on how this should be handled?
> >> 
> >> Inside of a modern bzImage there is an embedded ELF image.  How about in
> >> userspace we just strip out the embedded ELF image and write that to a
> >> file.  Then we can use the same signature checking scheme as we do for
> >> kernel modules.  And you only have to support one file format.
> >
> > I think there is a problem with that. And that we lose the additional
> > metadata info present in bzImage which is important.
> >
> > For example, knowing how much memory kernel will consume before it
> > sets up its own GDT and page tables (init_size) is very important. That
> > gives image loader lot of flexibility in figuring out where to place rest
> > of the components safely (initrd, GDT, page tables, ELF header segment, 
> > backup region etc).
> 
> The init_size should be reflected in the .bss of the ELF segments.  If
> not it is a bug when generating the kernel ELF headers and should be
> fixed.
> 
> For use by kexec I don't see any issues with just signing the embedded
> ELF image.

Hmm..., I will check this.

I have another concern though. If we extract it and write it to a file,
this assumes that we have a good destination where file can be written.
This atleast does not seem to be useful to chrome OS people where root
is read only and if a kernel is coming from root, then they know it
can be trusted.

Being able to write kernel to a file and then load it feels little odd to
me. Though this should be allowed but this should not be mandatory.

I think if we allow passing detached signature in kexec system call, then
it makes it much more flexible. We should be able to do what you are
suggesting at the same time it will also keep the possibility open for what
chromeOS developers are looking for.

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


Re: [PATCH 4/6] kexec: A new system call, kexec_file_load, for in kernel kexec

2013-11-26 Thread Eric W. Biederman
Vivek Goyal  writes:

> On Fri, Nov 22, 2013 at 07:39:14PM -0800, Eric W. Biederman wrote:
>
> [..]
>> > Hmm..., I am running out of ideas here. This is what I understand.
>> >
>> > - If I sign the bzImage (using PKCS1.5 signature), and later it is signed
>> >   with authenticode format signatures, then PKCS1.5 signatures will not be
>> >   valid as PE/COFF signing will do some modification to PE/COFF header in
>> >   bzImage. And another problem is that then I don't have a way to find
>> >   PKCS1.5 signature.
>> >
>> > - If bzImage is first signed with authenticode format signature and then
>> >   signed using PKCS1.5 signature, then  authenticode format signature
>> >   will become invalid as it will also hash the data appened at the end
>> >   of file.
>> >
>> > So looks like both signatures can't co-exist on same file. That means
>> > one signature has to be detached.
>> >
>> > I am beginning to think that create a kernel option which allows to choose
>> > between attached and detached signatures. Extend kexec syscall to allow
>> > a parameter to pass in detached signatures. If detached signatures are
>> > not passed, then look for signatures at the end of file. That way, those
>> > who are signing kernels using platform specific format (authenticode) in 
>> > this case, they can generate detached signature while others can just
>> > use attached signatures.
>> >
>> > Any thoughts on how this should be handled?
>> 
>> Inside of a modern bzImage there is an embedded ELF image.  How about in
>> userspace we just strip out the embedded ELF image and write that to a
>> file.  Then we can use the same signature checking scheme as we do for
>> kernel modules.  And you only have to support one file format.
>
> I think there is a problem with that. And that we lose the additional
> metadata info present in bzImage which is important.
>
> For example, knowing how much memory kernel will consume before it
> sets up its own GDT and page tables (init_size) is very important. That
> gives image loader lot of flexibility in figuring out where to place rest
> of the components safely (initrd, GDT, page tables, ELF header segment, 
> backup region etc).

The init_size should be reflected in the .bss of the ELF segments.  If
not it is a bug when generating the kernel ELF headers and should be
fixed.

For use by kexec I don't see any issues with just signing the embedded
ELF image.

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


Re: [PATCH 4/6] kexec: A new system call, kexec_file_load, for in kernel kexec

2013-11-26 Thread Eric W. Biederman
Vivek Goyal vgo...@redhat.com writes:

 On Fri, Nov 22, 2013 at 07:39:14PM -0800, Eric W. Biederman wrote:

 [..]
  Hmm..., I am running out of ideas here. This is what I understand.
 
  - If I sign the bzImage (using PKCS1.5 signature), and later it is signed
with authenticode format signatures, then PKCS1.5 signatures will not be
valid as PE/COFF signing will do some modification to PE/COFF header in
bzImage. And another problem is that then I don't have a way to find
PKCS1.5 signature.
 
  - If bzImage is first signed with authenticode format signature and then
signed using PKCS1.5 signature, then  authenticode format signature
will become invalid as it will also hash the data appened at the end
of file.
 
  So looks like both signatures can't co-exist on same file. That means
  one signature has to be detached.
 
  I am beginning to think that create a kernel option which allows to choose
  between attached and detached signatures. Extend kexec syscall to allow
  a parameter to pass in detached signatures. If detached signatures are
  not passed, then look for signatures at the end of file. That way, those
  who are signing kernels using platform specific format (authenticode) in 
  this case, they can generate detached signature while others can just
  use attached signatures.
 
  Any thoughts on how this should be handled?
 
 Inside of a modern bzImage there is an embedded ELF image.  How about in
 userspace we just strip out the embedded ELF image and write that to a
 file.  Then we can use the same signature checking scheme as we do for
 kernel modules.  And you only have to support one file format.

 I think there is a problem with that. And that we lose the additional
 metadata info present in bzImage which is important.

 For example, knowing how much memory kernel will consume before it
 sets up its own GDT and page tables (init_size) is very important. That
 gives image loader lot of flexibility in figuring out where to place rest
 of the components safely (initrd, GDT, page tables, ELF header segment, 
 backup region etc).

The init_size should be reflected in the .bss of the ELF segments.  If
not it is a bug when generating the kernel ELF headers and should be
fixed.

For use by kexec I don't see any issues with just signing the embedded
ELF image.

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


Re: [PATCH 4/6] kexec: A new system call, kexec_file_load, for in kernel kexec

2013-11-26 Thread Vivek Goyal
On Tue, Nov 26, 2013 at 04:23:36AM -0800, Eric W. Biederman wrote:
 Vivek Goyal vgo...@redhat.com writes:
 
  On Fri, Nov 22, 2013 at 07:39:14PM -0800, Eric W. Biederman wrote:
 
  [..]
   Hmm..., I am running out of ideas here. This is what I understand.
  
   - If I sign the bzImage (using PKCS1.5 signature), and later it is signed
 with authenticode format signatures, then PKCS1.5 signatures will not 
   be
 valid as PE/COFF signing will do some modification to PE/COFF header in
 bzImage. And another problem is that then I don't have a way to find
 PKCS1.5 signature.
  
   - If bzImage is first signed with authenticode format signature and then
 signed using PKCS1.5 signature, then  authenticode format signature
 will become invalid as it will also hash the data appened at the end
 of file.
  
   So looks like both signatures can't co-exist on same file. That means
   one signature has to be detached.
  
   I am beginning to think that create a kernel option which allows to 
   choose
   between attached and detached signatures. Extend kexec syscall to allow
   a parameter to pass in detached signatures. If detached signatures are
   not passed, then look for signatures at the end of file. That way, those
   who are signing kernels using platform specific format (authenticode) in 
   this case, they can generate detached signature while others can just
   use attached signatures.
  
   Any thoughts on how this should be handled?
  
  Inside of a modern bzImage there is an embedded ELF image.  How about in
  userspace we just strip out the embedded ELF image and write that to a
  file.  Then we can use the same signature checking scheme as we do for
  kernel modules.  And you only have to support one file format.
 
  I think there is a problem with that. And that we lose the additional
  metadata info present in bzImage which is important.
 
  For example, knowing how much memory kernel will consume before it
  sets up its own GDT and page tables (init_size) is very important. That
  gives image loader lot of flexibility in figuring out where to place rest
  of the components safely (initrd, GDT, page tables, ELF header segment, 
  backup region etc).
 
 The init_size should be reflected in the .bss of the ELF segments.  If
 not it is a bug when generating the kernel ELF headers and should be
 fixed.
 
 For use by kexec I don't see any issues with just signing the embedded
 ELF image.

Hmm..., I will check this.

I have another concern though. If we extract it and write it to a file,
this assumes that we have a good destination where file can be written.
This atleast does not seem to be useful to chrome OS people where root
is read only and if a kernel is coming from root, then they know it
can be trusted.

Being able to write kernel to a file and then load it feels little odd to
me. Though this should be allowed but this should not be mandatory.

I think if we allow passing detached signature in kexec system call, then
it makes it much more flexible. We should be able to do what you are
suggesting at the same time it will also keep the possibility open for what
chromeOS developers are looking for.

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


Re: [PATCH 4/6] kexec: A new system call, kexec_file_load, for in kernel kexec

2013-11-25 Thread Vivek Goyal
On Fri, Nov 22, 2013 at 07:39:14PM -0800, Eric W. Biederman wrote:

[..]
> > Hmm..., I am running out of ideas here. This is what I understand.
> >
> > - If I sign the bzImage (using PKCS1.5 signature), and later it is signed
> >   with authenticode format signatures, then PKCS1.5 signatures will not be
> >   valid as PE/COFF signing will do some modification to PE/COFF header in
> >   bzImage. And another problem is that then I don't have a way to find
> >   PKCS1.5 signature.
> >
> > - If bzImage is first signed with authenticode format signature and then
> >   signed using PKCS1.5 signature, then  authenticode format signature
> >   will become invalid as it will also hash the data appened at the end
> >   of file.
> >
> > So looks like both signatures can't co-exist on same file. That means
> > one signature has to be detached.
> >
> > I am beginning to think that create a kernel option which allows to choose
> > between attached and detached signatures. Extend kexec syscall to allow
> > a parameter to pass in detached signatures. If detached signatures are
> > not passed, then look for signatures at the end of file. That way, those
> > who are signing kernels using platform specific format (authenticode) in 
> > this case, they can generate detached signature while others can just
> > use attached signatures.
> >
> > Any thoughts on how this should be handled?
> 
> Inside of a modern bzImage there is an embedded ELF image.  How about in
> userspace we just strip out the embedded ELF image and write that to a
> file.  Then we can use the same signature checking scheme as we do for
> kernel modules.  And you only have to support one file format.

I think there is a problem with that. And that we lose the additional
metadata info present in bzImage which is important.

For example, knowing how much memory kernel will consume before it
sets up its own GDT and page tables (init_size) is very important. That
gives image loader lot of flexibility in figuring out where to place rest
of the components safely (initrd, GDT, page tables, ELF header segment, 
backup region etc).

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


Re: [PATCH 4/6] kexec: A new system call, kexec_file_load, for in kernel kexec

2013-11-25 Thread Vivek Goyal
On Fri, Nov 22, 2013 at 07:39:14PM -0800, Eric W. Biederman wrote:

[..]
  Hmm..., I am running out of ideas here. This is what I understand.
 
  - If I sign the bzImage (using PKCS1.5 signature), and later it is signed
with authenticode format signatures, then PKCS1.5 signatures will not be
valid as PE/COFF signing will do some modification to PE/COFF header in
bzImage. And another problem is that then I don't have a way to find
PKCS1.5 signature.
 
  - If bzImage is first signed with authenticode format signature and then
signed using PKCS1.5 signature, then  authenticode format signature
will become invalid as it will also hash the data appened at the end
of file.
 
  So looks like both signatures can't co-exist on same file. That means
  one signature has to be detached.
 
  I am beginning to think that create a kernel option which allows to choose
  between attached and detached signatures. Extend kexec syscall to allow
  a parameter to pass in detached signatures. If detached signatures are
  not passed, then look for signatures at the end of file. That way, those
  who are signing kernels using platform specific format (authenticode) in 
  this case, they can generate detached signature while others can just
  use attached signatures.
 
  Any thoughts on how this should be handled?
 
 Inside of a modern bzImage there is an embedded ELF image.  How about in
 userspace we just strip out the embedded ELF image and write that to a
 file.  Then we can use the same signature checking scheme as we do for
 kernel modules.  And you only have to support one file format.

I think there is a problem with that. And that we lose the additional
metadata info present in bzImage which is important.

For example, knowing how much memory kernel will consume before it
sets up its own GDT and page tables (init_size) is very important. That
gives image loader lot of flexibility in figuring out where to place rest
of the components safely (initrd, GDT, page tables, ELF header segment, 
backup region etc).

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


Re: [PATCH 4/6] kexec: A new system call, kexec_file_load, for in kernel kexec

2013-11-22 Thread Eric W. Biederman
Vivek Goyal  writes:

> On Thu, Nov 21, 2013 at 07:19:07PM +, Matthew Garrett wrote:
>> On Thu, Nov 21, 2013 at 02:13:05PM -0500, Vivek Goyal wrote:
>> > On Thu, Nov 21, 2013 at 07:06:20PM +, Matthew Garrett wrote:
>> > > That would require a certain degree of massaging from userspace if we 
>> > > want to be able to use the existing Authenticode signatures. Otherwise 
>> > > we need to sign kernels twice.
>> > 
>> > I was thinking oof signing the same kernel twice. Can I sign authenticode
>> > signed kernel again (using RSA signature as we do for modules) and append
>> > the signature to bzImage. 
>> 
>> No, you'd need to do it the other way around.
>
> Hmm..., I am running out of ideas here. This is what I understand.
>
> - If I sign the bzImage (using PKCS1.5 signature), and later it is signed
>   with authenticode format signatures, then PKCS1.5 signatures will not be
>   valid as PE/COFF signing will do some modification to PE/COFF header in
>   bzImage. And another problem is that then I don't have a way to find
>   PKCS1.5 signature.
>
> - If bzImage is first signed with authenticode format signature and then
>   signed using PKCS1.5 signature, then  authenticode format signature
>   will become invalid as it will also hash the data appened at the end
>   of file.
>
> So looks like both signatures can't co-exist on same file. That means
> one signature has to be detached.
>
> I am beginning to think that create a kernel option which allows to choose
> between attached and detached signatures. Extend kexec syscall to allow
> a parameter to pass in detached signatures. If detached signatures are
> not passed, then look for signatures at the end of file. That way, those
> who are signing kernels using platform specific format (authenticode) in 
> this case, they can generate detached signature while others can just
> use attached signatures.
>
> Any thoughts on how this should be handled?

Inside of a modern bzImage there is an embedded ELF image.  How about in
userspace we just strip out the embedded ELF image and write that to a
file.  Then we can use the same signature checking scheme as we do for
kernel modules.  And you only have to support one file format.

As I recall there are already some platforms on x86 like Xen that
already need to strip out the embedded ELF image for their loaders to
have all of the information they need to load the image.

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


Re: [PATCH 4/6] kexec: A new system call, kexec_file_load, for in kernel kexec

2013-11-22 Thread Jiri Kosina
On Wed, 20 Nov 2013, Vivek Goyal wrote:

> This patch implements the in kernel kexec functionality. It implements a
> new system call kexec_file_load. I think parameter list of this system
> call will change as I have not done the kernel image signature handling
> yet. I have been told that I might have to pass the detached signature
> and size as part of system call.
> 
> Previously segment list was prepared in user space. Now user space just
> passes kernel fd, initrd fd and command line and kernel will create a
> segment list internally.
> 
> This patch contains generic part of the code. Actual segment preparation
> and loading is done by arch and image specific loader. Which comes in
> next patch.
> 
> Signed-off-by: Vivek Goyal 
[ ... snip ... ]
> diff --git a/kernel/kexec.c b/kernel/kexec.c
> index 6238927..50bcaa8 100644
> --- a/kernel/kexec.c
> +++ b/kernel/kexec.c
[ ... snip ... ]
> @@ -843,7 +1075,11 @@ static int kimage_load_normal_segment(struct kimage 
> *image,
>   PAGE_SIZE - (maddr & ~PAGE_MASK));
>   uchunk = min(ubytes, mchunk);
>  
> - result = copy_from_user(ptr, buf, uchunk);
> + /* For file based kexec, source pages are in kernel memory */
> + if (image->file_mode)
> + memcpy(ptr, buf, uchunk);

Very minor nit I came across when going through the patchset -- can't we 
use some different buffer for the file-based kexec that's not marked 
__user here? This really causes some eye-pain when looking at the code.

Thanks a lot for pushing this patchset forward,

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


Re: [PATCH 4/6] kexec: A new system call, kexec_file_load, for in kernel kexec

2013-11-22 Thread Vivek Goyal
On Thu, Nov 21, 2013 at 07:19:07PM +, Matthew Garrett wrote:
> On Thu, Nov 21, 2013 at 02:13:05PM -0500, Vivek Goyal wrote:
> > On Thu, Nov 21, 2013 at 07:06:20PM +, Matthew Garrett wrote:
> > > That would require a certain degree of massaging from userspace if we 
> > > want to be able to use the existing Authenticode signatures. Otherwise 
> > > we need to sign kernels twice.
> > 
> > I was thinking oof signing the same kernel twice. Can I sign authenticode
> > signed kernel again (using RSA signature as we do for modules) and append
> > the signature to bzImage. 
> 
> No, you'd need to do it the other way around.

Hmm..., I am running out of ideas here. This is what I understand.

- If I sign the bzImage (using PKCS1.5 signature), and later it is signed
  with authenticode format signatures, then PKCS1.5 signatures will not be
  valid as PE/COFF signing will do some modification to PE/COFF header in
  bzImage. And another problem is that then I don't have a way to find
  PKCS1.5 signature.

- If bzImage is first signed with authenticode format signature and then
  signed using PKCS1.5 signature, then  authenticode format signature
  will become invalid as it will also hash the data appened at the end
  of file.

So looks like both signatures can't co-exist on same file. That means
one signature has to be detached.

I am beginning to think that create a kernel option which allows to choose
between attached and detached signatures. Extend kexec syscall to allow
a parameter to pass in detached signatures. If detached signatures are
not passed, then look for signatures at the end of file. That way, those
who are signing kernels using platform specific format (authenticode) in 
this case, they can generate detached signature while others can just
use attached signatures.

Any thoughts on how this should be handled?

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


Re: [PATCH 4/6] kexec: A new system call, kexec_file_load, for in kernel kexec

2013-11-22 Thread Vivek Goyal
On Thu, Nov 21, 2013 at 07:19:07PM +, Matthew Garrett wrote:
 On Thu, Nov 21, 2013 at 02:13:05PM -0500, Vivek Goyal wrote:
  On Thu, Nov 21, 2013 at 07:06:20PM +, Matthew Garrett wrote:
   That would require a certain degree of massaging from userspace if we 
   want to be able to use the existing Authenticode signatures. Otherwise 
   we need to sign kernels twice.
  
  I was thinking oof signing the same kernel twice. Can I sign authenticode
  signed kernel again (using RSA signature as we do for modules) and append
  the signature to bzImage. 
 
 No, you'd need to do it the other way around.

Hmm..., I am running out of ideas here. This is what I understand.

- If I sign the bzImage (using PKCS1.5 signature), and later it is signed
  with authenticode format signatures, then PKCS1.5 signatures will not be
  valid as PE/COFF signing will do some modification to PE/COFF header in
  bzImage. And another problem is that then I don't have a way to find
  PKCS1.5 signature.

- If bzImage is first signed with authenticode format signature and then
  signed using PKCS1.5 signature, then  authenticode format signature
  will become invalid as it will also hash the data appened at the end
  of file.

So looks like both signatures can't co-exist on same file. That means
one signature has to be detached.

I am beginning to think that create a kernel option which allows to choose
between attached and detached signatures. Extend kexec syscall to allow
a parameter to pass in detached signatures. If detached signatures are
not passed, then look for signatures at the end of file. That way, those
who are signing kernels using platform specific format (authenticode) in 
this case, they can generate detached signature while others can just
use attached signatures.

Any thoughts on how this should be handled?

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


Re: [PATCH 4/6] kexec: A new system call, kexec_file_load, for in kernel kexec

2013-11-22 Thread Jiri Kosina
On Wed, 20 Nov 2013, Vivek Goyal wrote:

 This patch implements the in kernel kexec functionality. It implements a
 new system call kexec_file_load. I think parameter list of this system
 call will change as I have not done the kernel image signature handling
 yet. I have been told that I might have to pass the detached signature
 and size as part of system call.
 
 Previously segment list was prepared in user space. Now user space just
 passes kernel fd, initrd fd and command line and kernel will create a
 segment list internally.
 
 This patch contains generic part of the code. Actual segment preparation
 and loading is done by arch and image specific loader. Which comes in
 next patch.
 
 Signed-off-by: Vivek Goyal vgo...@redhat.com
[ ... snip ... ]
 diff --git a/kernel/kexec.c b/kernel/kexec.c
 index 6238927..50bcaa8 100644
 --- a/kernel/kexec.c
 +++ b/kernel/kexec.c
[ ... snip ... ]
 @@ -843,7 +1075,11 @@ static int kimage_load_normal_segment(struct kimage 
 *image,
   PAGE_SIZE - (maddr  ~PAGE_MASK));
   uchunk = min(ubytes, mchunk);
  
 - result = copy_from_user(ptr, buf, uchunk);
 + /* For file based kexec, source pages are in kernel memory */
 + if (image-file_mode)
 + memcpy(ptr, buf, uchunk);

Very minor nit I came across when going through the patchset -- can't we 
use some different buffer for the file-based kexec that's not marked 
__user here? This really causes some eye-pain when looking at the code.

Thanks a lot for pushing this patchset forward,

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


Re: [PATCH 4/6] kexec: A new system call, kexec_file_load, for in kernel kexec

2013-11-22 Thread Eric W. Biederman
Vivek Goyal vgo...@redhat.com writes:

 On Thu, Nov 21, 2013 at 07:19:07PM +, Matthew Garrett wrote:
 On Thu, Nov 21, 2013 at 02:13:05PM -0500, Vivek Goyal wrote:
  On Thu, Nov 21, 2013 at 07:06:20PM +, Matthew Garrett wrote:
   That would require a certain degree of massaging from userspace if we 
   want to be able to use the existing Authenticode signatures. Otherwise 
   we need to sign kernels twice.
  
  I was thinking oof signing the same kernel twice. Can I sign authenticode
  signed kernel again (using RSA signature as we do for modules) and append
  the signature to bzImage. 
 
 No, you'd need to do it the other way around.

 Hmm..., I am running out of ideas here. This is what I understand.

 - If I sign the bzImage (using PKCS1.5 signature), and later it is signed
   with authenticode format signatures, then PKCS1.5 signatures will not be
   valid as PE/COFF signing will do some modification to PE/COFF header in
   bzImage. And another problem is that then I don't have a way to find
   PKCS1.5 signature.

 - If bzImage is first signed with authenticode format signature and then
   signed using PKCS1.5 signature, then  authenticode format signature
   will become invalid as it will also hash the data appened at the end
   of file.

 So looks like both signatures can't co-exist on same file. That means
 one signature has to be detached.

 I am beginning to think that create a kernel option which allows to choose
 between attached and detached signatures. Extend kexec syscall to allow
 a parameter to pass in detached signatures. If detached signatures are
 not passed, then look for signatures at the end of file. That way, those
 who are signing kernels using platform specific format (authenticode) in 
 this case, they can generate detached signature while others can just
 use attached signatures.

 Any thoughts on how this should be handled?

Inside of a modern bzImage there is an embedded ELF image.  How about in
userspace we just strip out the embedded ELF image and write that to a
file.  Then we can use the same signature checking scheme as we do for
kernel modules.  And you only have to support one file format.

As I recall there are already some platforms on x86 like Xen that
already need to strip out the embedded ELF image for their loaders to
have all of the information they need to load the image.

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


[PATCH 4/6] kexec: A new system call, kexec_file_load, for in kernel kexec

2013-11-20 Thread Vivek Goyal
This patch implements the in kernel kexec functionality. It implements a
new system call kexec_file_load. I think parameter list of this system
call will change as I have not done the kernel image signature handling
yet. I have been told that I might have to pass the detached signature
and size as part of system call.

Previously segment list was prepared in user space. Now user space just
passes kernel fd, initrd fd and command line and kernel will create a
segment list internally.

This patch contains generic part of the code. Actual segment preparation
and loading is done by arch and image specific loader. Which comes in
next patch.

Signed-off-by: Vivek Goyal 
---
 arch/x86/kernel/machine_kexec_64.c |   57 -
 arch/x86/syscalls/syscall_64.tbl   |1 +
 include/linux/kexec.h  |   57 +
 include/linux/syscalls.h   |3 +
 include/uapi/linux/kexec.h |4 +
 kernel/kexec.c |  486 +++-
 kernel/sys_ni.c|1 +
 7 files changed, 607 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/machine_kexec_64.c 
b/arch/x86/kernel/machine_kexec_64.c
index 4eabc16..fb41b73 100644
--- a/arch/x86/kernel/machine_kexec_64.c
+++ b/arch/x86/kernel/machine_kexec_64.c
@@ -22,6 +22,13 @@
 #include 
 #include 
 
+/* arch dependent functionality related to kexec file based syscall */
+static struct kexec_file_type kexec_file_type[]={
+   {"", NULL, NULL, NULL, NULL},
+};
+
+static int nr_file_types = sizeof(kexec_file_type)/sizeof(kexec_file_type[0]);
+
 static void free_transition_pgtable(struct kimage *image)
 {
free_page((unsigned long)image->arch.pud);
@@ -200,7 +207,7 @@ void machine_kexec(struct kimage *image)
 {
unsigned long page_list[PAGES_NR];
void *control_page;
-   int save_ftrace_enabled;
+   int save_ftrace_enabled, idx;
 
 #ifdef CONFIG_KEXEC_JUMP
if (image->preserve_context)
@@ -226,6 +233,11 @@ void machine_kexec(struct kimage *image)
 #endif
}
 
+   /* Call image loader to prepare for entry */
+   idx = image->file_handler_idx;
+   if (kexec_file_type[idx].prep_entry)
+   kexec_file_type[idx].prep_entry(image);
+
control_page = page_address(image->control_code_page) + PAGE_SIZE;
memcpy(control_page, relocate_kernel, KEXEC_CONTROL_CODE_MAX_SIZE);
 
@@ -281,3 +293,46 @@ void arch_crash_save_vmcoreinfo(void)
 #endif
 }
 
+/* arch dependent functionality related to kexec file based syscall */
+
+int arch_kexec_kernel_image_probe(struct kimage *image, void *buf,
+   unsigned long buf_len)
+{
+   int i, ret = -ENOEXEC;
+
+   for (i = 0; i < nr_file_types; i++) {
+   if (!kexec_file_type[i].probe)
+   continue;
+
+   ret = kexec_file_type[i].probe(buf, buf_len);
+   if (!ret) {
+   image->file_handler_idx = i;
+   return ret;
+   }
+   }
+
+   return ret;
+}
+
+void *arch_kexec_kernel_image_load(struct kimage *image, char *kernel,
+   unsigned long kernel_len, char *initrd,
+   unsigned long initrd_len, char *cmdline,
+   unsigned long cmdline_len)
+{
+   int idx = image->file_handler_idx;
+
+   if (idx < 0)
+   return ERR_PTR(-ENOEXEC);
+
+   return kexec_file_type[idx].load(image, kernel, kernel_len, initrd,
+   initrd_len, cmdline, cmdline_len);
+}
+
+int arch_image_file_post_load_cleanup(struct kimage *image)
+{
+   int idx = image->file_handler_idx;
+
+   if (kexec_file_type[idx].cleanup)
+   return kexec_file_type[idx].cleanup(image);
+   return 0;
+}
diff --git a/arch/x86/syscalls/syscall_64.tbl b/arch/x86/syscalls/syscall_64.tbl
index 38ae65d..6f37cc9 100644
--- a/arch/x86/syscalls/syscall_64.tbl
+++ b/arch/x86/syscalls/syscall_64.tbl
@@ -320,6 +320,7 @@
 31164  process_vm_writev   sys_process_vm_writev
 312common  kcmpsys_kcmp
 313common  finit_modulesys_finit_module
+314common  kexec_file_load sys_kexec_file_load
 
 #
 # x32-specific system call numbers start at 512 to avoid cache impact
diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index d78d28a..a2baf96 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -110,13 +110,60 @@ struct kimage {
 #define KEXEC_TYPE_DEFAULT 0
 #define KEXEC_TYPE_CRASH   1
unsigned int preserve_context : 1;
+   /* If set, we are using file mode kexec syscall */
+   unsigned int file_mode : 1;
 
 #ifdef ARCH_HAS_KIMAGE_ARCH
struct kimage_arch arch;
 #endif
+
+   /* Additional Fields for file based kexec syscall */
+   void *kernel_buf;
+   unsigned long kernel_buf_len;
+
+   void *initrd_buf;
+   unsigned long initrd_buf_len;
+
+   char 

[PATCH 4/6] kexec: A new system call, kexec_file_load, for in kernel kexec

2013-11-20 Thread Vivek Goyal
This patch implements the in kernel kexec functionality. It implements a
new system call kexec_file_load. I think parameter list of this system
call will change as I have not done the kernel image signature handling
yet. I have been told that I might have to pass the detached signature
and size as part of system call.

Previously segment list was prepared in user space. Now user space just
passes kernel fd, initrd fd and command line and kernel will create a
segment list internally.

This patch contains generic part of the code. Actual segment preparation
and loading is done by arch and image specific loader. Which comes in
next patch.

Signed-off-by: Vivek Goyal vgo...@redhat.com
---
 arch/x86/kernel/machine_kexec_64.c |   57 -
 arch/x86/syscalls/syscall_64.tbl   |1 +
 include/linux/kexec.h  |   57 +
 include/linux/syscalls.h   |3 +
 include/uapi/linux/kexec.h |4 +
 kernel/kexec.c |  486 +++-
 kernel/sys_ni.c|1 +
 7 files changed, 607 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/machine_kexec_64.c 
b/arch/x86/kernel/machine_kexec_64.c
index 4eabc16..fb41b73 100644
--- a/arch/x86/kernel/machine_kexec_64.c
+++ b/arch/x86/kernel/machine_kexec_64.c
@@ -22,6 +22,13 @@
 #include asm/mmu_context.h
 #include asm/debugreg.h
 
+/* arch dependent functionality related to kexec file based syscall */
+static struct kexec_file_type kexec_file_type[]={
+   {, NULL, NULL, NULL, NULL},
+};
+
+static int nr_file_types = sizeof(kexec_file_type)/sizeof(kexec_file_type[0]);
+
 static void free_transition_pgtable(struct kimage *image)
 {
free_page((unsigned long)image-arch.pud);
@@ -200,7 +207,7 @@ void machine_kexec(struct kimage *image)
 {
unsigned long page_list[PAGES_NR];
void *control_page;
-   int save_ftrace_enabled;
+   int save_ftrace_enabled, idx;
 
 #ifdef CONFIG_KEXEC_JUMP
if (image-preserve_context)
@@ -226,6 +233,11 @@ void machine_kexec(struct kimage *image)
 #endif
}
 
+   /* Call image loader to prepare for entry */
+   idx = image-file_handler_idx;
+   if (kexec_file_type[idx].prep_entry)
+   kexec_file_type[idx].prep_entry(image);
+
control_page = page_address(image-control_code_page) + PAGE_SIZE;
memcpy(control_page, relocate_kernel, KEXEC_CONTROL_CODE_MAX_SIZE);
 
@@ -281,3 +293,46 @@ void arch_crash_save_vmcoreinfo(void)
 #endif
 }
 
+/* arch dependent functionality related to kexec file based syscall */
+
+int arch_kexec_kernel_image_probe(struct kimage *image, void *buf,
+   unsigned long buf_len)
+{
+   int i, ret = -ENOEXEC;
+
+   for (i = 0; i  nr_file_types; i++) {
+   if (!kexec_file_type[i].probe)
+   continue;
+
+   ret = kexec_file_type[i].probe(buf, buf_len);
+   if (!ret) {
+   image-file_handler_idx = i;
+   return ret;
+   }
+   }
+
+   return ret;
+}
+
+void *arch_kexec_kernel_image_load(struct kimage *image, char *kernel,
+   unsigned long kernel_len, char *initrd,
+   unsigned long initrd_len, char *cmdline,
+   unsigned long cmdline_len)
+{
+   int idx = image-file_handler_idx;
+
+   if (idx  0)
+   return ERR_PTR(-ENOEXEC);
+
+   return kexec_file_type[idx].load(image, kernel, kernel_len, initrd,
+   initrd_len, cmdline, cmdline_len);
+}
+
+int arch_image_file_post_load_cleanup(struct kimage *image)
+{
+   int idx = image-file_handler_idx;
+
+   if (kexec_file_type[idx].cleanup)
+   return kexec_file_type[idx].cleanup(image);
+   return 0;
+}
diff --git a/arch/x86/syscalls/syscall_64.tbl b/arch/x86/syscalls/syscall_64.tbl
index 38ae65d..6f37cc9 100644
--- a/arch/x86/syscalls/syscall_64.tbl
+++ b/arch/x86/syscalls/syscall_64.tbl
@@ -320,6 +320,7 @@
 31164  process_vm_writev   sys_process_vm_writev
 312common  kcmpsys_kcmp
 313common  finit_modulesys_finit_module
+314common  kexec_file_load sys_kexec_file_load
 
 #
 # x32-specific system call numbers start at 512 to avoid cache impact
diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index d78d28a..a2baf96 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -110,13 +110,60 @@ struct kimage {
 #define KEXEC_TYPE_DEFAULT 0
 #define KEXEC_TYPE_CRASH   1
unsigned int preserve_context : 1;
+   /* If set, we are using file mode kexec syscall */
+   unsigned int file_mode : 1;
 
 #ifdef ARCH_HAS_KIMAGE_ARCH
struct kimage_arch arch;
 #endif
+
+   /* Additional Fields for file based kexec syscall */
+   void *kernel_buf;
+   unsigned long kernel_buf_len;
+
+   void *initrd_buf;
+   unsigned