kexec.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Vivek Goyal <vgoyal@redhat.com>
To: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: mjg59@srcf.ucam.org, kexec@lists.infradead.org,
	linux-kernel@vger.kernel.org, greg@kroah.com, hpa@zytor.com
Subject: Re: [PATCH 0/6] kexec: A new system call to allow in kernel loading
Date: Wed, 4 Dec 2013 14:34:31 -0500	[thread overview]
Message-ID: <20131204193430.GF19087@redhat.com> (raw)
In-Reply-To: <87li0fvm10.fsf@xmission.com>

On Fri, Nov 22, 2013 at 07:23:39PM -0800, Eric W. Biederman wrote:
> 
> > [..]
> >> >> There is also a huge missing piece of this in that your purgatory is not
> >> >> checking a hash of the loaded image before jumping too it.  Without that
> >> >> this is a huge regression at least for the kexec on panic case.  We
> >> >> absolutely need to check that the kernel sitting around in memory has
> >> >> not been corrupted before we let it run very far.
> >> >
> >> > Agreed. This should not be hard. It is just a matter of calcualting
> >> > digest of segments. I will store it in kimge and verify digest again
> >> > before passing control to control page. Will fix it in next version.
> >> 
> >> Nak.  The verification needs to happen in purgatory. 
> >> 
> >> The verification needs to happen in code whose runtime environment is
> >> does not depend on random parts of the kernel.  Anything else is a
> >> regression in maintainability and reliability.
> >> 
> >> It is the wrong direction to add any code to what needs to run in the
> >> known broken environment of the kernel when a panic happens.
> >> 
> >> Which means that you almost certainly need to go to the trouble of
> >> supporting the complexity needed to support purgatory code written in C.
> >> 
> >> (For those just tuning in purgatory is our term for the code that runs
> >> between the kernels to do those things that can not happen a priori).
> >
> > In general, I agree with not using kernel parts after crash.
> >
> > But what protects against that purgatory itself has been scribbled over.
> > IOW, how different purgatory memory is as compared to kernel memory where
> > digest routines are stored. They have got equal probably of being scribbled
> > over and if that's the case one is not better than other?
> >
> > And if they both got equal probability to getting corrupted, then there does
> > not seem to be an advantage in moving digest verification inside
> > purgatory.
> 
> The primary reason is that maintenance of code in the kernel that is
> safe during a crash dump is hard.  That is why we boot a second kernel
> after all.  If the code to do the signature verification resides in
> machine_kexec on the kexec on panic code path in the kernel that has
> called panic it is almost a given that at some point or other someone
> will add an option that will add a weird dependency that makes the code
> unsafe when the kernel is crashing.  I have seen it happen several times
> on the existing kexec on panic code path.  I have seen it on other code
> paths like netconsole.  Which can currently on some kernels I have
> running cause the kernel go go into an endless printk loop if you call
> printk from interrupt context.  So what we really gain by moving the
> verification into purgatory is protection from inappropriate code reuse.
> 
> So having a completely separate piece of code may be a little harder to
> write initially but the code is much simpler and more reliable to
> maintain.  Essentially requiring no maintenance effort.  Further getting
> to the point where purgatory is written in C makes small changes much
> more approachable.

Hi Eric,

So you want a separate purgatory code and that purgatory should be self
contained and should not share any code with rest of the kernel. No
inclusion of header files, no linking against kernel libraries? That means
even re-implementing sha256 functions separately (like user space)?

If code maintenance is a concern, then I think I can reimplement some
of the functions to calculate sha256 in separate crash files and invoke
those to reduce code sharing with rest of the kernel. And we should be
able to link against the kernel and not have to create separate
relocatable purgatory object and relocate it.

IOW, does purgatory still have to be a relocatable object? I think
user space had no choice but given the fact that we are implementing
thing in kernel, I should be able to implement my own hash calculation
and segment verification code and link it to existing kernel and invoke
these outside purgatory. Anyway, we call so many other functions after
crash to stop cpus, save registers, etc.

Thanks
Vivek

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

  reply	other threads:[~2013-12-04 19:34 UTC|newest]

Thread overview: 90+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-11-20 17:50 [PATCH 0/6] kexec: A new system call to allow in kernel loading Vivek Goyal
2013-11-20 17:50 ` [PATCH 1/6] kexec: Export vmcoreinfo note size properly Vivek Goyal
2013-11-21 18:59   ` Greg KH
2013-11-21 19:08     ` Vivek Goyal
2013-11-20 17:50 ` [PATCH 2/6] kexec: Move segment verification code in a separate function Vivek Goyal
2013-11-20 17:50 ` [PATCH 3/6] resource: Provide new functions to walk through resources Vivek Goyal
2013-11-20 17:50 ` [PATCH 4/6] kexec: A new system call, kexec_file_load, for in kernel kexec Vivek Goyal
2013-11-21 19:03   ` Greg KH
2013-11-21 19:06     ` Matthew Garrett
2013-11-21 19:13       ` Vivek Goyal
2013-11-21 19:19         ` Matthew Garrett
2013-11-21 19:24           ` Vivek Goyal
2013-11-22 18:57           ` Vivek Goyal
2013-11-23  3:39             ` Eric W. Biederman
2013-11-25 16:39               ` Vivek Goyal
2013-11-26 12:23                 ` Eric W. Biederman
2013-11-26 14:27                   ` Vivek Goyal
2013-12-19 12:54                     ` Torsten Duwe
2013-12-20 14:19                       ` Vivek Goyal
2013-12-20 23:11                         ` Eric W. Biederman
2013-12-20 23:20                           ` Kees Cook
2013-12-21 11:38                             ` Torsten Duwe
2014-01-02 20:39                             ` Vivek Goyal
2014-01-02 20:56                               ` H. Peter Anvin
2014-01-06 21:33                                 ` Josh Boyer
2014-01-07  4:22                                   ` H. Peter Anvin
2013-12-20 23:20                           ` H. Peter Anvin
2013-12-21  1:32                             ` Eric W. Biederman
2013-12-21  3:32                               ` H. Peter Anvin
2013-12-21 12:15                                 ` Torsten Duwe
2013-11-21 19:16     ` Vivek Goyal
2013-11-22  1:03     ` Kees Cook
2013-11-22  2:13       ` Vivek Goyal
2013-11-22 20:42   ` Jiri Kosina
2014-01-17 19:17     ` Vivek Goyal
2013-11-29  3:10   ` Baoquan He
2013-12-02 15:27     ` WANG Chao
2013-12-02 15:44     ` Vivek Goyal
2013-12-04  1:35       ` Baoquan He
2013-12-04 17:19         ` Vivek Goyal
2013-12-04  1:56   ` Baoquan He
2013-12-04  8:19     ` Baoquan He
2013-12-04 17:32     ` Vivek Goyal
2013-11-20 17:50 ` [PATCH 5/6] kexec-bzImage: Support for loading bzImage using 64bit entry Vivek Goyal
2013-11-21 19:07   ` Greg KH
2013-11-21 19:21     ` Vivek Goyal
2013-11-22 15:24       ` H. Peter Anvin
2013-11-28 11:35   ` Baoquan He
2013-12-02 15:36     ` Vivek Goyal
2013-11-20 17:50 ` [PATCH 6/6] kexec: Support for Kexec on panic using new system call Vivek Goyal
2013-11-28 11:28   ` Baoquan He
2013-12-02 15:30     ` Vivek Goyal
2013-12-04  1:51       ` Baoquan He
2013-12-04 17:20         ` Vivek Goyal
2013-12-04  1:41   ` Baoquan He
2013-12-04 17:19     ` Vivek Goyal
2013-11-21 18:58 ` [PATCH 0/6] kexec: A new system call to allow in kernel loading Greg KH
2013-11-21 19:07   ` Vivek Goyal
2013-11-21 19:46     ` Vivek Goyal
2013-11-21 19:06 ` Geert Uytterhoeven
2013-11-21 19:14   ` Vivek Goyal
2013-11-21 23:07 ` Eric W. Biederman
2013-11-22  1:28   ` H. Peter Anvin
2013-11-22  2:35     ` Vivek Goyal
2013-11-22  2:40       ` H. Peter Anvin
2013-11-22  1:55   ` Vivek Goyal
2013-11-22  9:09     ` Geert Uytterhoeven
2013-11-22 13:30       ` Jiri Kosina
2013-11-22 13:46         ` Vivek Goyal
2013-11-22 13:50           ` Jiri Kosina
2013-11-22 15:33             ` Vivek Goyal
2013-11-22 17:45               ` Kees Cook
2013-11-22 13:43       ` Vivek Goyal
2013-11-22 15:25         ` Geert Uytterhoeven
2013-11-22 15:33           ` Jiri Kosina
2013-11-22 15:57             ` Eric Paris
2013-11-22 16:04               ` Jiri Kosina
2013-11-22 16:08                 ` Vivek Goyal
2013-11-22 13:34     ` Eric W. Biederman
2013-11-22 14:19       ` Vivek Goyal
2013-11-22 19:48         ` Greg KH
2013-11-23  3:23         ` Eric W. Biederman
2013-12-04 19:34           ` Vivek Goyal [this message]
2013-12-05  4:10             ` Eric W. Biederman
2013-11-25 10:04       ` Michael Holzheu
2013-11-25 15:36         ` Vivek Goyal
2013-11-25 16:15           ` Michael Holzheu
2013-11-22  0:55 ` HATAYAMA Daisuke
2013-11-22  2:03   ` Vivek Goyal
2013-12-03 13:23 ` Baoquan He

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20131204193430.GF19087@redhat.com \
    --to=vgoyal@redhat.com \
    --cc=ebiederm@xmission.com \
    --cc=greg@kroah.com \
    --cc=hpa@zytor.com \
    --cc=kexec@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mjg59@srcf.ucam.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).