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


Groups > linux.kernel > #1557020

Re: [PATCH v4] lib/scatterlist: Avoid potential scatterlist entry overflow

From Andy Shevchenko <andy.shevchenko@gmail.com>
Newsgroups linux.kernel
Subject Re: [PATCH v4] lib/scatterlist: Avoid potential scatterlist entry overflow
Date 2017-01-12 01:10 +0100
Message-ID <sYB7P-5mx-15@gated-at.bofh.it> (permalink)
References <sYn4R-52M-31@gated-at.bofh.it> <sYpJo-6v8-15@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


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

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


Thread

[PATCH 1/4] lib/scatterlist: Fix offset type in sg_alloc_table_from_pages Tvrtko Ursulin <tursulin@ursulin.net> - 2017-01-11 10:10 +0100
  [PATCH 3/4] lib/scatterlist: Introduce and export __sg_alloc_table_from_pages Tvrtko Ursulin <tursulin@ursulin.net> - 2017-01-11 10:10 +0100
  [PATCH 4/4] drm/i915: Use __sg_alloc_table_from_pages for userptr allocations Tvrtko Ursulin <tursulin@ursulin.net> - 2017-01-11 10:10 +0100
    [PATCH v6] drm/i915: Use __sg_alloc_table_from_pages for userptr allocations Tvrtko Ursulin <tursulin@ursulin.net> - 2017-01-11 13:10 +0100
  [PATCH 2/4] lib/scatterlist: Avoid potential scatterlist entry overflow Tvrtko Ursulin <tursulin@ursulin.net> - 2017-01-11 10:10 +0100
    [PATCH v4] lib/scatterlist: Avoid potential scatterlist entry overflow Tvrtko Ursulin <tursulin@ursulin.net> - 2017-01-11 13:00 +0100
      Re: [PATCH v4] lib/scatterlist: Avoid potential scatterlist entry overflow Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-01-12 01:10 +0100
        Re: [Intel-gfx] [PATCH v4] lib/scatterlist: Avoid potential  scatterlist entry overflow Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> - 2017-01-13 09:40 +0100
          Re: [Intel-gfx] [PATCH v4] lib/scatterlist: Avoid potential  scatterlist entry overflow Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-01-13 23:30 +0100
            Re: [Intel-gfx] [PATCH v4] lib/scatterlist: Avoid potential  scatterlist entry overflow Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> - 2017-01-16 11:10 +0100
              Re: [Intel-gfx] [PATCH v4] lib/scatterlist: Avoid potential  scatterlist entry overflow Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-01-16 11:20 +0100
  [PATCH v5] lib/scatterlist: Introduce and export __sg_alloc_table_from_pages Tvrtko Ursulin <tursulin@ursulin.net> - 2017-01-11 13:00 +0100

csiph-web