Re: [PATCH] kexec: add restriction on kexec_load() segment sizes

[Andrew Morton <akpm@linux-foundation.org>]

On Wed, 27 Jul 2016 12:10:13 +0800 zhong jiang <zhongjiang@huawei.com> wrote:

> >> +	for (i = 0; i < nr_segments; i++) {
> >> +		if (PAGE_COUNT(image->segment[i].memsz) > totalram_pages / 2
> >> +				|| PAGE_COUNT(total_segments) > totalram_pages / 2)
> >> +			return result;
> > And I don't think we need this?  Unless we're worried about the sum of
> > all segments overflowing an unsigned long, which I guess is possible. 
> > But if we care about that we should handle it in the next statement:
> >
> >> +		total_segments += image->segment[i].memsz;
> > Should this be 
> >
> > 		total_pages += PAGE_COUNT(image->segment[i].memsz);
>   ok
> > ?  I think "yes", if the segments are allocated separately and "no" if
> > they are all allocated in a big blob.
>    There is a possible that  most of segments size will exceed half of  the real memory.
> 
>   if (PAGE_COUNT(image->segment[i].memsz) > totalram_pages / 2
> 	|| total_pages > totalram_pages / 2)
>   I guess that it is ok , we should bail out timely when it happens to the condition.
>   
>   is right ?
> 
>  your mean that above condition is no need. In the end, we check the overflow just one time.
>   or I misunderstand.

It doesn't matter much.  Actually I misread the code a bit.  How about

	for (i = 0; i < nr_segments; i++) {
		unsigned long seg_pages = PAGE_COUNT(image->segment[i].memsz);

		if (seg_pages > totalram_pages / 2))
			return -EINVAL;

		total_pages += seg_pages;

		if (total_pages > totalram_pages / 2)
			return -EINVAL;
	}

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>