Re: [patch 1/3 v2] Add function get_bootparam

[Dave Young <dyoung@redhat.com>]

On 11/11/13 at 11:58pm, Greg KH wrote:
> On Mon, Nov 11, 2013 at 07:20:17PM -0800, H. Peter Anvin wrote:
> > On 11/11/2013 05:27 PM, Toshi Kani wrote:
> > > On Tue, 2013-11-05 at 16:29 +0800, dyoung@redhat.com wrote:
> > >> Not only setup_subarch will get data from debugfs file
> > >> boot_params/data, later code for adding efi_info will
> > >> also need do same thing. Thus add a common function here
> > >> for later use.
> > > 
> > > get_bootparam() calls find_mnt_by_fsname("debugfs"), which assumes that
> > > debugfs is mounted with device name "debugfs".  This function fails
> > > when:
> > >  - debugfs is not mounted, or
> > >  - debugfs is mounted with a different device name, such as nodev [1].
> > > 
> > > This issue is not introduced by this patch.  But the original code,
> > > which sets hardware_subarch, seems to work even if it failed to access
> > > debugfs (which is ignored) since hardware_subarch is zero most of the
> > > cases anyway.  With this change, however, this failure now makes the 2nd
> > > kernel unbootable.  So, it needs to be addressed to make this code path
> > > work reliably (or kexec should fail at least).
> > > 
> > 
> > Greg, Ingo,
> > 
> > Since there is now a legitimate user of this stuff, can we actually
> > export this in sysfs (or something else other than sploit^Wdebugfs)?
> 
> Hey, after Tejun is done with the kernelfs work, I'll be porting debugfs
> to it so you can't call it sploitfs anymore :)

Good news.

> 
> > kexec-tools can have a fallback to debugfs if we really need it, but
> > making people mount debugfs to have some essential piece of
> > functionality scares the heck out of me.
> 
> I agree, that would not be good.
> 
> I'm not sure what exactly the sysfs file is wanting to look like?  The
> efi section variable sysfs file isn't ok (multiple lines with multiple
> values.)  What is this going to look like?

The current structure in debugfs is like below, export every field of setup
header as one file will be too much IMHO:
[root@darkstar debug]# tree boot_params
boot_params
├── data /* binary data of setup header */
├── setup_data /* setup_data link list nodes */
│   ├── 0
│   │   ├── data /* binary data of setup data */
│   │   └── type /* type of setup data */
│   └── 1
│       ├── data
│       └── type
└── version /* boot protocol version */

Thanks
Dave

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