| Message ID | 20190813192126.122370-4-matthewgarrett@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | lockdown fixups | expand |
On 08/13/19 at 12:21pm, Matthew Garrett wrote: > Refactor the signature validation and lockdown integration a little in > order to avoid an unused variable. > > Signed-off-by: Matthew Garrett <mjg59@google.com> > Cc: Jiri Bohac <jbohac@suse.cz> > Cc: Dave Young <dyoung@redhat.com> > Cc: kexec@lists.infradead.org > --- > kernel/kexec_file.c | 72 ++++++++++++++++++++++++++++----------------- > 1 file changed, 45 insertions(+), 27 deletions(-) > > diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c > index be0c13076056..e878587715b9 100644 > --- a/kernel/kexec_file.c > +++ b/kernel/kexec_file.c > @@ -177,36 +177,13 @@ void kimage_file_post_load_cleanup(struct kimage *image) > image->image_loader_data = NULL; > } > > -/* > - * In file mode list of segments is prepared by kernel. Copy relevant > - * data from user space, do error checking, prepare segment list > - */ > +#ifdef CONFIG_KEXEC_SIG > static int > -kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd, > - const char __user *cmdline_ptr, > - unsigned long cmdline_len, unsigned flags) > +kimage_validate_signature(struct kimage *image) > { > const char *reason; > int ret; > - void *ldata; > - loff_t size; > - > - ret = kernel_read_file_from_fd(kernel_fd, &image->kernel_buf, > - &size, INT_MAX, READING_KEXEC_IMAGE); > - if (ret) > - return ret; > - image->kernel_buf_len = size; > - > - /* IMA needs to pass the measurement list to the next kernel. */ > - ima_add_kexec_buffer(image); > > - /* Call arch image probe handlers */ > - ret = arch_kexec_kernel_image_probe(image, image->kernel_buf, > - image->kernel_buf_len); > - if (ret) > - goto out; > - > -#ifdef CONFIG_KEXEC_SIG > ret = arch_kexec_kernel_verify_sig(image, image->kernel_buf, > image->kernel_buf_len); > switch (ret) { > @@ -228,7 +205,7 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd, > decide: > if (IS_ENABLED(CONFIG_KEXEC_SIG_FORCE)) { > pr_notice("%s rejected\n", reason); > - goto out; > + break; > } > > ret = 0; > @@ -251,9 +228,44 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd, > */ > default: > pr_notice("kernel signature verification failed (%d).\n", ret); > - goto out; > + break; > } > + > + return ret; > +} > +#endif > + > +/* > + * In file mode list of segments is prepared by kernel. Copy relevant > + * data from user space, do error checking, prepare segment list > + */ > +static int > +kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd, > + const char __user *cmdline_ptr, > + unsigned long cmdline_len, unsigned flags) > +{ > + int ret; > + void *ldata; > + loff_t size; > + > + ret = kernel_read_file_from_fd(kernel_fd, &image->kernel_buf, > + &size, INT_MAX, READING_KEXEC_IMAGE); > + if (ret) > + return ret; > + image->kernel_buf_len = size; > + > + /* Call arch image probe handlers */ > + ret = arch_kexec_kernel_image_probe(image, image->kernel_buf, > + image->kernel_buf_len); > + if (ret) > + goto out; > + > +#ifdef CONFIG_KEXEC_SIG > + ret = kimage_validate_signature(image); > + if (ret) > + goto out; > #endif > + > /* It is possible that there no initramfs is being loaded */ > if (!(flags & KEXEC_FILE_NO_INITRAMFS)) { > ret = kernel_read_file_from_fd(initrd_fd, &image->initrd_buf, > @@ -279,8 +291,14 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd, > ret = -EINVAL; > goto out; > } > + > + ima_kexec_cmdline(image->cmdline_buf, > + image->cmdline_buf_len - 1); > } > > + /* IMA needs to pass the measurement list to the next kernel. */ > + ima_add_kexec_buffer(image); > + > /* Call arch image load handlers */ > ldata = arch_kexec_kernel_image_load(image); > I can not get the whole thread, also not sure which tree it should apply. I assume it is based on your series for lockdown. But it is really hard to review because of no context in this patch. Also I think it should be good to split the preparation piece of kexec and send them separately. Since it is improve the signature verification logic, they do not necessarily depend on the lockdown series. Thanks Dave
On Tue, Aug 13, 2019 at 10:24 PM Dave Young <dyoung@redhat.com> wrote: > I can not get the whole thread, also not sure which tree it should > apply. This should be against -next. > Also I think it should be good to split the preparation piece of kexec > and send them separately. Since it is improve the signature > verification logic, they do not necessarily depend on the lockdown > series. Sorry, which preparation piece? There shouldn't be any logic change in this patch.
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c index be0c13076056..e878587715b9 100644 --- a/kernel/kexec_file.c +++ b/kernel/kexec_file.c @@ -177,36 +177,13 @@ void kimage_file_post_load_cleanup(struct kimage *image) image->image_loader_data = NULL; } -/* - * In file mode list of segments is prepared by kernel. Copy relevant - * data from user space, do error checking, prepare segment list - */ +#ifdef CONFIG_KEXEC_SIG static int -kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd, - const char __user *cmdline_ptr, - unsigned long cmdline_len, unsigned flags) +kimage_validate_signature(struct kimage *image) { const char *reason; int ret; - void *ldata; - loff_t size; - - ret = kernel_read_file_from_fd(kernel_fd, &image->kernel_buf, - &size, INT_MAX, READING_KEXEC_IMAGE); - if (ret) - return ret; - image->kernel_buf_len = size; - - /* IMA needs to pass the measurement list to the next kernel. */ - ima_add_kexec_buffer(image); - /* Call arch image probe handlers */ - ret = arch_kexec_kernel_image_probe(image, image->kernel_buf, - image->kernel_buf_len); - if (ret) - goto out; - -#ifdef CONFIG_KEXEC_SIG ret = arch_kexec_kernel_verify_sig(image, image->kernel_buf, image->kernel_buf_len); switch (ret) { @@ -228,7 +205,7 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd, decide: if (IS_ENABLED(CONFIG_KEXEC_SIG_FORCE)) { pr_notice("%s rejected\n", reason); - goto out; + break; } ret = 0; @@ -251,9 +228,44 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd, */ default: pr_notice("kernel signature verification failed (%d).\n", ret); - goto out; + break; } + + return ret; +} +#endif + +/* + * In file mode list of segments is prepared by kernel. Copy relevant + * data from user space, do error checking, prepare segment list + */ +static int +kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd, + const char __user *cmdline_ptr, + unsigned long cmdline_len, unsigned flags) +{ + int ret; + void *ldata; + loff_t size; + + ret = kernel_read_file_from_fd(kernel_fd, &image->kernel_buf, + &size, INT_MAX, READING_KEXEC_IMAGE); + if (ret) + return ret; + image->kernel_buf_len = size; + + /* Call arch image probe handlers */ + ret = arch_kexec_kernel_image_probe(image, image->kernel_buf, + image->kernel_buf_len); + if (ret) + goto out; + +#ifdef CONFIG_KEXEC_SIG + ret = kimage_validate_signature(image); + if (ret) + goto out; #endif + /* It is possible that there no initramfs is being loaded */ if (!(flags & KEXEC_FILE_NO_INITRAMFS)) { ret = kernel_read_file_from_fd(initrd_fd, &image->initrd_buf, @@ -279,8 +291,14 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd, ret = -EINVAL; goto out; } + + ima_kexec_cmdline(image->cmdline_buf, + image->cmdline_buf_len - 1); } + /* IMA needs to pass the measurement list to the next kernel. */ + ima_add_kexec_buffer(image); + /* Call arch image load handlers */ ldata = arch_kexec_kernel_image_load(image);
Refactor the signature validation and lockdown integration a little in order to avoid an unused variable. Signed-off-by: Matthew Garrett <mjg59@google.com> Cc: Jiri Bohac <jbohac@suse.cz> Cc: Dave Young <dyoung@redhat.com> Cc: kexec@lists.infradead.org --- kernel/kexec_file.c | 72 ++++++++++++++++++++++++++++----------------- 1 file changed, 45 insertions(+), 27 deletions(-)