Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1461472

Re: [PATCH v2 2/2] kexec: extend kexec_file_load system call

Show all headers | View raw

Hello Balbir,

Thank you for the review!

Am Freitag, 12 August 2016, 18:17:39 schrieb Balbir Singh:
> On Thu, Aug 11, 2016 at 08:03:58PM -0300, Thiago Jung Bauermann wrote:
> > diff --git a/include/linux/fs.h b/include/linux/fs.h
> > index 3523bf62f328..847d9c31f428 100644
> > --- a/include/linux/fs.h
> > +++ b/include/linux/fs.h
> > @@ -2656,6 +2656,7 @@ extern int do_pipe_flags(int *, int);
> > 
> >  	id(MODULE, kernel-module)		\
> >  	id(KEXEC_IMAGE, kexec-image)		\
> >  	id(KEXEC_INITRAMFS, kexec-initramfs)	\
> > 
> > +	id(KEXEC_PARTIAL_DTB, kexec-partial-dtb)		\
> 
> The backspace is over-indented?

Indeed, I'll fix that. But to keep it aligned with the other backslashes, 
there would be no spaces between it and the final closing parenthesis. 
Either that, or reindent the other backslashes one more level. I think I 
prefer the former.
 
> > @@ -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) {
> > +		int nr_fds, i;
> > +		size_t fdset_size;
> > +		char fdset_buf[MAX_FDSET_SIZE];
> 
> Do we really want this on the stack?  I presume the size is not large

It has 132 bytes. Would it be better to use kmalloc instead?

> > +		struct kexec_fdset *fdset = (struct kexec_fdset *) 
fdset_buf;
> > +
> > +		ret = copy_from_user(&nr_fds, ufdset, sizeof(int));
> > +		if (ret) {
> > +			ret = -EFAULT;
> > +			goto out;
> > +		}
> > +
> > +		if (nr_fds > KEXEC_SEGMENT_MAX) {
> 
> We need an nr_fds < 0 check as well

Indeed, I forgot to do that. I will add the check.

> > +			ret = -E2BIG;
> > +			goto out;
> > +		}
> > +
> > +		fdset_size = sizeof(struct kexec_fdset)
> > +				+ nr_fds * sizeof(struct kexec_file_fd);
> > +
> > +		ret = copy_from_user(fdset, ufdset, fdset_size);
> 
> Can the user change nr_fds between the two copy_from_users, ideally not,
> but we should validate it.

Good catch. I'll check if nr_fds == fdset->nr_fds and return with an error 
if they're different.

-- 
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH v2 0/2] extend kexec_file_load system call Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com> - 2016-08-12 01:10 +0200
  [PATCH v2 1/2] kexec: add dtb info to struct kimage Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com> - 2016-08-12 01:10 +0200
    Re: [PATCH v2 1/2] kexec: add dtb info to struct kimage Dave Young <dyoung@redhat.com> - 2016-08-18 10:40 +0200
  [PATCH v2 2/2] kexec: extend kexec_file_load system call Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com> - 2016-08-12 01:10 +0200
    Re: [PATCH v2 2/2] kexec: extend kexec_file_load system call Balbir Singh <bsingharora@gmail.com> - 2016-08-12 10:30 +0200
      Re: [PATCH v2 2/2] kexec: extend kexec_file_load system call Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com> - 2016-08-12 23:50 +0200
        [PATCH v2 2/2] kexec: extend kexec_file_load system call Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com> - 2016-08-16 02:20 +0200
    Re: [PATCH v2 2/2] kexec: extend kexec_file_load system call Dave Young <dyoung@redhat.com> - 2016-08-18 10:30 +0200
  Re: [PATCH v2 0/2] extend kexec_file_load system call Mark Rutland <mark.rutland@arm.com> - 2016-08-18 12:40 +0200

csiph-web