lkml.org 
[lkml]   [2017]   [Jan]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH v4] lib/scatterlist: Avoid potential scatterlist entry overflow
On Wed, Jan 11, 2017 at 1:58 PM, Tvrtko Ursulin <tursulin@ursulin.net> wrote:
> Since the scatterlist length field is an unsigned int, make
> sure that sg_alloc_table_from_pages does not overflow it while
> coallescing pages to a single entry.


> /*
> + * Since the above length field is an unsigned int, below we define the maximum
> + * lenght in bytes that can be stored in one scatterlist entry.

length

> + */
> +#define SCATTERLIST_MAX_SEGMENT (0xfffff000)

Shouldn't be calculated from PAGE_SIZE (PAGE bits, etc)?

> --- a/lib/scatterlist.c
> +++ b/lib/scatterlist.c

> @@ -402,9 +403,16 @@ int sg_alloc_table_from_pages(struct sg_table *sgt,
>
> /* compute number of contiguous chunks */
> chunks = 1;
> - for (i = 1; i < n_pages; ++i)
> - if (page_to_pfn(pages[i]) != page_to_pfn(pages[i - 1]) + 1)
> + seg_len = PAGE_SIZE;
> + for (i = 1; i < n_pages; ++i) {
> + if (seg_len >= max_segment ||
> + page_to_pfn(pages[i]) != page_to_pfn(pages[i - 1]) + 1) {
> ++chunks;
> + seg_len = PAGE_SIZE;
> + } else {
> + seg_len += PAGE_SIZE;
> + }
> + }

Wouldn't be following looks more readable?

seg_len = 0;
// Are compilers so stupid doing calculation per iteration in for-conditional?
// for (i = 0; i + 1 < n_pages; i++) ?
for (i = 1; i < n_pages; ++i) {
seg_len += PAGE_SIZE;
if (seg_len >= max_segment || page_to_pfn(pages[i]) !=
page_to_pfn(pages[i - 1]) + 1) {
++chunks;
seg_len = PAGE_SIZE;
}
}

Perhaps while() or do-while() will increase readability even more, but
I didn't check.

> /* look for the end of the current chunk */
> + seg_len = PAGE_SIZE;
> for (j = cur_page + 1; j < n_pages; ++j)
> - if (page_to_pfn(pages[j]) !=
> + if (seg_len >= max_segment ||
> + page_to_pfn(pages[j]) !=
> page_to_pfn(pages[j - 1]) + 1)
> break;
> + else
> + seg_len += PAGE_SIZE;

Something similar here (didn't you get warning from checkpath about
curly braces?).

--
With Best Regards,
Andy Shevchenko

\
 
 \ /
  Last update: 2017-01-12 01:00    [W:0.049 / U:2.436 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site