lkml.org 
[lkml]   [2014]   [Jun]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 07/13] kexec: Implementation of new syscall kexec_file_load
    On Fri, Jun 06, 2014 at 02:56:05PM +0800, WANG Chao wrote:
    > On 06/03/14 at 09:06am, Vivek Goyal wrote:
    > > Previous patch provided the interface definition and this patch prvides
    > > implementation of new syscall.
    > >
    > > 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 <vgoyal@redhat.com>
    >
    > [..]
    > > diff --git a/kernel/kexec.c b/kernel/kexec.c
    > > index a3044e6..1ad4d60 100644
    > > --- a/kernel/kexec.c
    > > +++ b/kernel/kexec.c
    >
    > > +static int kimage_file_prepare_segments(struct kimage *image, int kernel_fd,
    > > + int initrd_fd, const char __user *cmdline_ptr,
    > > + unsigned long cmdline_len)
    > > +{
    > > + int ret = 0;
    > > + void *ldata;
    > > +
    > > + ret = copy_file_from_fd(kernel_fd, &image->kernel_buf,
    > > + &image->kernel_buf_len);
    > > + if (ret)
    > > + return ret;
    > > +
    > > + /* Call arch image probe handlers */
    > > + ret = arch_kexec_kernel_image_probe(image, image->kernel_buf,
    > > + image->kernel_buf_len);
    > > +
    > > + if (ret)
    > > + goto out;
    > > +
    > > + ret = copy_file_from_fd(initrd_fd, &image->initrd_buf,
    > > + &image->initrd_buf_len);
    > > + if (ret)
    > > + goto out;
    > > +
    > > + image->cmdline_buf = vzalloc(cmdline_len);
    >
    > You should validate the upper/lower boundary of cmdline_len before
    > calling vzalloc. When cmdline_len is 0 or too large, vmalloc failure
    > message would be fired.

    What's the upper length of vzalloc(). I think if it is too big to alloc,
    then vzalloc() should return me an error?

    >
    > > + if (!image->cmdline_buf)
    > > + goto out;
    > > +
    > > + ret = copy_from_user(image->cmdline_buf, cmdline_ptr, cmdline_len);
    > > + if (ret) {
    > > + ret = -EFAULT;
    > > + goto out;
    > > + }
    > > +
    > > + image->cmdline_buf_len = cmdline_len;
    > > +
    > > + /* command line should be a string with last byte null */
    > > + if (image->cmdline_buf[cmdline_len - 1] != '\0') {
    > > + ret = -EINVAL;
    > > + goto out;
    > > + }
    >
    > Given the fact that command line is optional as well as initrd, I think
    > above chunk of code needs to update a bit for the case cmdline_len is 0
    > or cmdline_buf is pointing NULL?

    I agree. I think all this vzalloc(), copy_from_user() etc should be called
    only fir cmdline_len is non-zero. Will fix it.

    Thanks
    Vivek



    \
     
     \ /
      Last update: 2014-06-06 20:41    [W:3.469 / U:0.048 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site