linux-kernel - Re: [Intel-gfx] [PATCH v4] lib/scatterlist: Avoid potential scatterlist entry overflow

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sat, 14 Jan 2017 00:23:48 +0200
From:   Andy Shevchenko <andy.shevchenko@...il.com>
To:     Tvrtko Ursulin <tvrtko.ursulin@...ux.intel.com>
Cc:     Tvrtko Ursulin <tursulin@...ulin.net>,
        Masahiro Yamada <yamada.masahiro@...ionext.com>,
        Intel-gfx@...ts.freedesktop.org,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Joonas Lahtinen <joonas.lahtinen@...ux.intel.com>
Subject: Re: [Intel-gfx] [PATCH v4] lib/scatterlist: Avoid potential
 scatterlist entry overflow

>>> @@ -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++) ?
>
>
> I didn't get what you meant here?

Why do we start from 1? I see here two micro (?) optimizations:
1) starting from 1 on believe that compiler dumb enough to every time
do a calculation in condition;
2) ++i instead of i++, but this is just matter of style, it's not a c++.

>> 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;
>>   }
>> }
>
>
> Tried it in my unit tester but it doesn't work for all scenarios, guess
> there is a subtle bug somewhere. I don't find it that unreadable so would
> prefer to leave it since it works.

Last seems has to be
seg_len = 0;

-- 
With Best Regards,
Andy Shevchenko