Device tree blob must be passed to a second kernel on DTB-capable
archs, like powerpc and arm64, but the current kernel interface
lacks this support.
This patch extends kexec_file_load system call by adding an extra
argument to this syscall so that an arbitrary number of file descriptors
can be handed out from user space to the kernel.
See the background [1].
Please note that the new interface looks quite similar to the current
system call, but that it won't always mean that it provides the "binary
compatibility."
[1] http://lists.infradead.org/pipermail/kexec/2016-June/016276.html
AKASHI Takahiro (3):
syscall: add kexec_file_load to generic unistd.h
kexec: add dtb info to struct kimage
kexec: extend kexec_file_load system call
include/linux/fs.h | 1 +
include/linux/kexec.h | 5 +++-
include/linux/syscalls.h | 4 ++-
include/uapi/asm-generic/unistd.h | 8 ++++-
include/uapi/linux/kexec.h | 17 +++++++++++
kernel/kexec_file.c | 62 ++++++++++++++++++++++++++++++++++-----
6 files changed, 87 insertions(+), 10 deletions(-)
--
2.9.0
Currently kexec_file_load is supported only on x86, but it will be
supported on powerpc and arm64 in near future. Since both archs
use asm-generic/unistd.h, this patch adds the entry to this file.
Signed-off-by: AKASHI Takahiro <redacted>
---
include/uapi/asm-generic/unistd.h | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
Device tree blob must be passed to a second kernel on DTB-capable
archs, like powerpc and arm64, but the current kernel interface
lacks this support.
This patch adds dtb buffer information to struct kimage.
When users don't specify dtb explicitly and the one used for the current
kernel can be re-used, this change will be good enough for implementing
kexec_file_load feature.
Signed-off-by: AKASHI Takahiro <redacted>
---
include/linux/kexec.h | 3 +++
kernel/kexec_file.c | 5 +++++
2 files changed, 8 insertions(+)
Device tree blob must be passed to a second kernel on DTB-capable
archs, like powerpc and arm64, but the current kernel interface
lacks this support.
This patch extends kexec_file_load system call by adding an extra
argument to this syscall so that an arbitrary number of file descriptors
can be handed out from user space to the kernel.
long sys_kexec_file_load(int kernel_fd, int initrd_fd,
unsigned long cmdline_len,
const char __user *cmdline_ptr,
unsigned long flags,
const struct kexec_fdset __user *ufdset);
If KEXEC_FILE_EXTRA_FDS is set to the "flags" argument, the "ufdset"
argument points to the following struct buffer:
struct kexec_fdset {
int nr_fds;
struct kexec_file_fd fds[0];
}
Signed-off-by: AKASHI Takahiro <redacted>
---
include/linux/fs.h | 1 +
include/linux/kexec.h | 2 +-
include/linux/syscalls.h | 4 +++-
include/uapi/linux/kexec.h | 17 ++++++++++++++
kernel/kexec_file.c | 57 ++++++++++++++++++++++++++++++++++++++++------
5 files changed, 72 insertions(+), 9 deletions(-)
@@ -321,7 +322,8 @@ asmlinkage long sys_kexec_load(unsigned long entry, unsigned long nr_segments,asmlinkagelongsys_kexec_file_load(intkernel_fd,intinitrd_fd,unsignedlongcmdline_len,constchar__user*cmdline_ptr,-unsignedlongflags);+unsignedlongflags,+conststructkexec_fdset__user*ufdset);asmlinkagelongsys_exit(interror_code);asmlinkagelongsys_exit_group(interror_code);
@@ -174,6 +177,42 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,image->initrd_buf_len=size;}+if(flags&KEXEC_FILE_EXTRA_FDS){+ret=copy_from_user(&nr_fds,ufdset,sizeof(int));+if(ret){+ret=-EFAULT;+gotoout;+}++fdset_size=sizeof(structkexec_fdset)++nr_fds*sizeof(structkexec_file_fd);+fdset=kmalloc(fdset_size,GFP_KERNEL);+if(!fdset){+ret=-ENOMEM;+gotoout;+}++ret=copy_from_user(fdset,ufdset,fdset_size);+if(ret){+ret=-EFAULT;+gotoout;+}++for(i=0;i<fdset->nr_fds;i++){+if(fdset->fds[i].type==KEXEC_FILE_TYPE_DTB){+ret=kernel_read_file_from_fd(fdset->fds[i].fd,+&image->dtb_buf,&size,INT_MAX,+READING_KEXEC_DTB);+if(ret)+gotoout;+image->dtb_buf_len=size;+}else{+pr_debug("unknown file type %d failed.\n",+fdset->fds[i].type);+}+}+}+if(cmdline_len){image->cmdline_buf=kzalloc(cmdline_len,GFP_KERNEL);if(!image->cmdline_buf){
@@ -208,6 +247,8 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,image->image_loader_data=ldata;out:/* In case of error, free up all allocated memory in this function */+kfree(fdset);+if(ret)kimage_file_post_load_cleanup(image);returnret;
Can one add more parameters to existing syscall. Can it break existing
programs with new kernel? I was of the impression that one can't do that.
But may be I am missing something.
Vivek
Can one add more parameters to existing syscall. Can it break existing
programs with new kernel? I was of the impression that one can't do that.
But may be I am missing something.
I think the idea was that we would only look at the new params if a new
flags was set, and otherwise it would behave as the old syscall.
Regardless, I think it makes far more sense to add a kexec_file_load2
syscall if we're going to modify the prototype at all. It's a rather
different proposition to the existing syscall, and needs to be treated
as such.
Thanks,
Mark.
Can one add more parameters to existing syscall. Can it break existing
programs with new kernel? I was of the impression that one can't do that.
But may be I am missing something.
I think the idea was that we would only look at the new params if a new
flags was set, and otherwise it would behave as the old syscall.
Regardless, I think it makes far more sense to add a kexec_file_load2
syscall if we're going to modify the prototype at all. It's a rather
different proposition to the existing syscall, and needs to be treated
as such.
I do not think it is worth to add another syscall for extra fds.
We have open(2) as an example for different numbers of arguments
already.
Thanks
Dave
Can one add more parameters to existing syscall. Can it break existing
programs with new kernel? I was of the impression that one can't do that.
But may be I am missing something.
It will not break existing programs because we can use the new param only
when the new flag is set.
But we have a case below, but I think it is fine?
Originally kexec_file_load with the new flags will fail, but now it will
succeed and will access the new argument.
Thanks
Dave
Can one add more parameters to existing syscall. Can it break existing
programs with new kernel? I was of the impression that one can't do that.
But may be I am missing something.
I think the idea was that we would only look at the new params if a new
flags was set, and otherwise it would behave as the old syscall.
Regardless, I think it makes far more sense to add a kexec_file_load2
syscall if we're going to modify the prototype at all. It's a rather
different proposition to the existing syscall, and needs to be treated
as such.
I do not think it is worth to add another syscall for extra fds.
We have open(2) as an example for different numbers of arguments
already.
Did we change the syscall interface for that?
I was under the impression that there was always one underlying syscall,
and the C library did the right thing to pass the expected information
to the underlying syscall.
That's rather different to changing the underlying syscall.
Regardless of how this is wrapped in userspace, I do not think modifying
the existing prototype is a good idea, and I think this kind of
extension needs to be a new syscall.
Thanks,
Mark.
Can one add more parameters to existing syscall. Can it break existing
programs with new kernel? I was of the impression that one can't do that.
But may be I am missing something.
I think the idea was that we would only look at the new params if a new
flags was set, and otherwise it would behave as the old syscall.
Regardless, I think it makes far more sense to add a kexec_file_load2
syscall if we're going to modify the prototype at all. It's a rather
different proposition to the existing syscall, and needs to be treated
as such.
I do not think it is worth to add another syscall for extra fds.
We have open(2) as an example for different numbers of arguments
already.
Did we change the syscall interface for that?
I was under the impression that there was always one underlying syscall,
and the C library did the right thing to pass the expected information
to the underlying syscall.
I'm not sure kexec_load and kexec_file_load were included in glibc, we use
syscall directly in kexec-tools.
kexec_load man pages says there are no wrappers for both kexec_load and
kexec_file_load in glibc.
That's rather different to changing the underlying syscall.
Regardless of how this is wrapped in userspace, I do not think modifying
the existing prototype is a good idea, and I think this kind of
extension needs to be a new syscall.
Hmm, as I replied to Vivek, there is one case about the flags, previously
the new flag will be regarded as invalid, but not we extend it it will be
valid, this maybe the only potential bad case.
Thanks
Dave
From: Mark Rutland <mark.rutland@arm.com> Date: 2016-07-19 10:52:09
On Tue, Jul 19, 2016 at 08:55:56AM +0800, Dave Young wrote:
On 07/18/16 at 11:07am, Mark Rutland wrote:
quoted
On Mon, Jul 18, 2016 at 10:30:24AM +0800, Dave Young wrote:
quoted
I do not think it is worth to add another syscall for extra fds.
We have open(2) as an example for different numbers of arguments
already.
Did we change the syscall interface for that?
I was under the impression that there was always one underlying syscall,
and the C library did the right thing to pass the expected information
to the underlying syscall.
I'm not sure kexec_load and kexec_file_load were included in glibc, we use
syscall directly in kexec-tools.
kexec_load man pages says there are no wrappers for both kexec_load and
kexec_file_load in glibc.
For the above, I was talking about how open() was handled.
If there are no userspace wrappers, then the two cases aren't comparable
in the first place...
quoted
That's rather different to changing the underlying syscall.
Regardless of how this is wrapped in userspace, I do not think modifying
the existing prototype is a good idea, and I think this kind of
extension needs to be a new syscall.
Hmm, as I replied to Vivek, there is one case about the flags, previously
the new flag will be regarded as invalid, but not we extend it it will be
valid, this maybe the only potential bad case.
It's true that adding suport for new flags will change the behaviour of
what used to be error cases. We generally expect real users to not be
making pointless calls for which they rely on an error being returned in
all cases.
Regardless, this extended syscall changes some underlying assumptions
made with the development of kexec_file_load, and I think treating this
as an extension is not a great idea. From a user's perspective there is
little difference between passing an additional flag or using a
different syscall number, so I don't think that we gain much by altering
the existing prototype relative to allocating a new syscall number.
Thus, I think that if this is necessary it should be treated as a new
syscall.
Thanks,
Mark.
On Tue, Jul 19, 2016 at 11:52:00AM +0100, Mark Rutland wrote:
On Tue, Jul 19, 2016 at 08:55:56AM +0800, Dave Young wrote:
quoted
On 07/18/16 at 11:07am, Mark Rutland wrote:
quoted
On Mon, Jul 18, 2016 at 10:30:24AM +0800, Dave Young wrote:
quoted
I do not think it is worth to add another syscall for extra fds.
We have open(2) as an example for different numbers of arguments
already.
Did we change the syscall interface for that?
I was under the impression that there was always one underlying syscall,
and the C library did the right thing to pass the expected information
to the underlying syscall.
I'm not sure kexec_load and kexec_file_load were included in glibc, we use
syscall directly in kexec-tools.
kexec_load man pages says there are no wrappers for both kexec_load and
kexec_file_load in glibc.
For the above, I was talking about how open() was handled.
If there are no userspace wrappers, then the two cases aren't comparable
in the first place...
quoted
quoted
That's rather different to changing the underlying syscall.
Regardless of how this is wrapped in userspace, I do not think modifying
the existing prototype is a good idea, and I think this kind of
extension needs to be a new syscall.
Hmm, as I replied to Vivek, there is one case about the flags, previously
the new flag will be regarded as invalid, but not we extend it it will be
valid, this maybe the only potential bad case.
It's true that adding suport for new flags will change the behaviour of
what used to be error cases. We generally expect real users to not be
making pointless calls for which they rely on an error being returned in
all cases.
Regardless, this extended syscall changes some underlying assumptions
made with the development of kexec_file_load, and I think treating this
as an extension is not a great idea. From a user's perspective there is
little difference between passing an additional flag or using a
different syscall number, so I don't think that we gain much by altering
the existing prototype relative to allocating a new syscall number.
If we are providing/opening up additional flags, I can't think what will
it break. Same flag was invalid in old kernel but new kernel supports
it and will accept it. So it sounds reasonable to me to add new flags.
If existing users are not broken, then I think it might be a good idea
to extend existing syscall. Otherwise userspace will have to be modified
to understand a 3rd syscall also and an additional option will show up
which asks users to specify which syscall to use. So extending existing
syscall might keep it little simple for users.
This is only if conclusion in the end is that DT needs to be passed in
from user space.
BTW, does kexec_load() needs to be modified too to handle DT?
Vivek
From: Mark Rutland <mark.rutland@arm.com> Date: 2016-07-19 12:47:41
On Tue, Jul 19, 2016 at 08:24:06AM -0400, Vivek Goyal wrote:
On Tue, Jul 19, 2016 at 11:52:00AM +0100, Mark Rutland wrote:
quoted
Regardless, this extended syscall changes some underlying assumptions
made with the development of kexec_file_load, and I think treating this
as an extension is not a great idea. From a user's perspective there is
little difference between passing an additional flag or using a
different syscall number, so I don't think that we gain much by altering
the existing prototype relative to allocating a new syscall number.
If we are providing/opening up additional flags, I can't think what will
it break. Same flag was invalid in old kernel but new kernel supports
it and will accept it. So it sounds reasonable to me to add new flags.
If existing users are not broken, then I think it might be a good idea
to extend existing syscall. Otherwise userspace will have to be modified
to understand a 3rd syscall also and an additional option will show up
which asks users to specify which syscall to use. So extending existing
syscall might keep it little simple for users.
I don't follow.
To use the new feature, you have to modify userspace anyway, as you
require userspace to pass information which it did not previously pass
(in the new arguments added to the syscall).
The presence of a new syscall does not imply the absence of the old
syscall, so you can always use that be default unless the user asks for
asomething only the new syscall provides. Regardless of the
syscall/flags difference, you still have to detect whether the new
functionality is present somehow.
BTW, does kexec_load() needs to be modified too to handle DT?
No, at least for arm64. In the kexec_load case userspace provides the
DTB as a raw segment, and the user-provided purgatory sets up registers
to pass that to the new kernel.
Thanks,
Mark.
On Tue, Jul 19, 2016 at 01:47:28PM +0100, Mark Rutland wrote:
On Tue, Jul 19, 2016 at 08:24:06AM -0400, Vivek Goyal wrote:
quoted
On Tue, Jul 19, 2016 at 11:52:00AM +0100, Mark Rutland wrote:
quoted
Regardless, this extended syscall changes some underlying assumptions
made with the development of kexec_file_load, and I think treating this
as an extension is not a great idea. From a user's perspective there is
little difference between passing an additional flag or using a
different syscall number, so I don't think that we gain much by altering
the existing prototype relative to allocating a new syscall number.
If we are providing/opening up additional flags, I can't think what will
it break. Same flag was invalid in old kernel but new kernel supports
it and will accept it. So it sounds reasonable to me to add new flags.
If existing users are not broken, then I think it might be a good idea
to extend existing syscall. Otherwise userspace will have to be modified
to understand a 3rd syscall also and an additional option will show up
which asks users to specify which syscall to use. So extending existing
syscall might keep it little simple for users.
I don't follow.
To use the new feature, you have to modify userspace anyway, as you
require userspace to pass information which it did not previously pass
(in the new arguments added to the syscall).
The presence of a new syscall does not imply the absence of the old
syscall, so you can always use that be default unless the user asks for
asomething only the new syscall provides. Regardless of the
syscall/flags difference, you still have to detect whether the new
functionality is present somehow.
Hmm., so current idea is that we have two syscalls() which are *ideally*
supposed to work for all arches. Difference between two is that first
one does not support kernel signature verification while second one does.
By default old syscall is used and user can force using new syscall using
option --kexec-file-load.
If a user DTB is present, I was hoping that it will continue to work the
same way. Both the sycalls can be used and can handle DTB. If we introduce
a 3rd syscall, that means only first and 3rd syscall can handle DTB and
we need to introduce one more option which tells whether to use
kexec_load() or use the 3rd new syscall. And that's what I am trying
to avoid.
Vivek
quoted
BTW, does kexec_load() needs to be modified too to handle DT?
No, at least for arm64. In the kexec_load case userspace provides the
DTB as a raw segment, and the user-provided purgatory sets up registers
to pass that to the new kernel.
Thanks,
Mark.
From: Russell King - ARM Linux <linux@armlinux.org.uk> Date: 2016-07-21 09:22:22
On Wed, Jul 20, 2016 at 11:41:35AM +0000, David Laight wrote:
From: Dave Young
quoted
I do not think it is worth to add another syscall for extra fds.
We have open(2) as an example for different numbers of arguments
already.
Probably works 'by luck' and no one has actually thought about why.
That ioctl() works is (probably) even more lucky.
There are ABI that use different calling conventions for varags functions
(eg always stack all the arguments). I guess linux doesn't run on any of them.
ioctl() is a particular problem because the 'arg' might be an integer or a pointer.
Fortunately all the 64bit ABI linux uses pass the arg parameter in a register
(and don't use different registers for pointer and data arguments).
You could have two 'libc' functions that refer to the same system call entry.
Certainly safer than a varargs function.
Don't forget that the syscall API is not a normal C function API - it's
special, because there's little point stacking arguments on the userspace
stack and then having the kernel function try and read them off the
kernelspace stack.
If an architecture does such a thing, then it needs special veneers to
handle that (reading off the userspace stack and placing them onto the
kernelspace stack, or the arch needs to define some other method of
handling the situation.)
So, really, the actual C APIs don't matter that much - what matters more
is the definition of a sane way to pass such arguments. Given the
extensive historical nature of open() and ioctl(), it would be completely
silly not to create something which allows these calls to work.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
Device tree blob must be passed to a second kernel on DTB-capable
archs, like powerpc and arm64, but the current kernel interface
lacks this support.
This patch extends kexec_file_load system call by adding an extra
argument to this syscall so that an arbitrary number of file descriptors
can be handed out from user space to the kernel.
long sys_kexec_file_load(int kernel_fd, int initrd_fd,
unsigned long cmdline_len,
const char __user *cmdline_ptr,
unsigned long flags,
const struct kexec_fdset __user *ufdset);
If KEXEC_FILE_EXTRA_FDS is set to the "flags" argument, the "ufdset"
argument points to the following struct buffer:
struct kexec_fdset {
int nr_fds;
struct kexec_file_fd fds[0];
}
Signed-off-by: AKASHI Takahiro <redacted>
Signed-off-by: Thiago Jung Bauermann <redacted>
---
Notes:
This is a new version of the last patch in this series which adds
a function where each architecture can verify if the DTB is safe
to load:
int __weak arch_kexec_verify_buffer(enum kexec_file_type type,
const void *buf,
unsigned long size)
{
return -EINVAL;
}
I will then provide an implementation in my powerpc patch series
which checks that the DTB only contains nodes and properties from a
whitelist. arch_kexec_kernel_image_load will copy these properties
to the device tree blob the kernel was booted with (and perform
other changes such as setting /chosen/bootargs, of course).
I made the following additional changes:
- renamed KEXEC_FILE_TYPE_DTB to KEXEC_FILE_TYPE_PARTIAL_DTB,
- limited max number of fds to KEXEC_SEGMENT_MAX,
- changed to use fixed size buffer for fdset instead of allocating it,
- changed to return -EINVAL if an unknown file type is found in fdset.
include/linux/fs.h | 1 +
include/linux/kexec.h | 7 ++--
include/linux/syscalls.h | 4 ++-
include/uapi/linux/kexec.h | 22 ++++++++++++
kernel/kexec_file.c | 83 ++++++++++++++++++++++++++++++++++++++++++----
5 files changed, 108 insertions(+), 9 deletions(-)
@@ -321,7 +322,8 @@ asmlinkage long sys_kexec_load(unsigned long entry, unsigned long nr_segments,asmlinkagelongsys_kexec_file_load(intkernel_fd,intinitrd_fd,unsignedlongcmdline_len,constchar__user*cmdline_ptr,-unsignedlongflags);+unsignedlongflags,+conststructkexec_fdset__user*ufdset);asmlinkagelongsys_exit(interror_code);asmlinkagelongsys_exit_group(interror_code);
@@ -160,6 +180,55 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,image->initrd_buf_len=size;}+if(flags&KEXEC_FILE_EXTRA_FDS){+intnr_fds,i;+size_tfdset_size;+charfdset_buf[MAX_FDSET_SIZE];+structkexec_fdset*fdset=(structkexec_fdset*)fdset_buf;++ret=copy_from_user(&nr_fds,ufdset,sizeof(int));+if(ret){+ret=-EFAULT;+gotoout;+}++if(nr_fds>KEXEC_SEGMENT_MAX){+ret=-E2BIG;+gotoout;+}++fdset_size=sizeof(structkexec_fdset)++nr_fds*sizeof(structkexec_file_fd);++ret=copy_from_user(fdset,ufdset,fdset_size);+if(ret){+ret=-EFAULT;+gotoout;+}++for(i=0;i<fdset->nr_fds;i++){+if(fdset->fds[i].type==KEXEC_FILE_TYPE_PARTIAL_DTB){+ret=kernel_read_file_from_fd(fdset->fds[i].fd,+&image->dtb_buf,&size,INT_MAX,+READING_KEXEC_PARTIAL_DTB);+if(ret)+gotoout;+image->dtb_buf_len=size;++ret=arch_kexec_verify_buffer(KEXEC_FILE_TYPE_PARTIAL_DTB,+image->dtb_buf,+image->dtb_buf_len);+if(ret)+gotoout;+}else{+pr_debug("unknown file type %d failed.\n",+fdset->fds[i].type);+ret=-EINVAL;+gotoout;+}+}+}+if(cmdline_len){image->cmdline_buf=kzalloc(cmdline_len,GFP_KERNEL);if(!image->cmdline_buf){
Hi,
Am Dienstag, 26 Juli 2016, 21:24:29 schrieb Thiago Jung Bauermann:
Notes:
This is a new version of the last patch in this series which adds
a function where each architecture can verify if the DTB is safe
to load:
int __weak arch_kexec_verify_buffer(enum kexec_file_type type,
const void *buf,
unsigned long size)
{
return -EINVAL;
}
I will then provide an implementation in my powerpc patch series
which checks that the DTB only contains nodes and properties from a
whitelist. arch_kexec_kernel_image_load will copy these properties
to the device tree blob the kernel was booted with (and perform
other changes such as setting /chosen/bootargs, of course).
Is this approach ok? If so, I'll post a patch next week adding an
arch_kexec_verify_buffer hook for powerpc to enforce the whitelist, and also
a new version of the patches implementing kexec_file_load for powerpc on top
of this series.
Eric, does this address your concerns?
--
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center