Messages in this thread | | | From | Yueyi Li <> | Subject | Re: [PATCH v2] lzo: fix ip overrun during compress. | Date | Mon, 3 Dec 2018 02:46:38 +0000 |
| |
On 2018/11/30 20:20, Dave Rodgman wrote: >> On 2018/11/30 0:49, Dave Rodgman wrote: >>> On 28/11/2018 1:52 pm, David Sterba wrote: >>> >>>> The fix is adding a few branches to code that's supposed to be as fast >>>> as possible. The branches would be evaluated all the time while >>>> protecting against one signle bad page address. This does not look like >>>> a good performance tradeoff. >>> As an alternative, for all but the first case, instead of: >>> >>> if (unlikely(OVERFLOW_ADD_CHECK(ip, m_len) || (ip + m_len >= ip_end))) >>> >>> I'd suggest we do: >>> >>> if (unlikely((ip_end - ip) <= m_len)) >>> >>> which will be about as efficient as what's currently there, but doesn't >>> have issues with overflow. >> Ooh, yes, pretty good solution to this, thanks. > Np :-) > > Actually, looking more closely at the first case, something like this > works quite well: > > size_t inc = 1 + ((ip - ii) >> 5); > if (unlikely((ip_end - ip) <= inc)) > break; > ip += inc; > > On arm64, this generates only a single branch instruction, so it's only > two extra arithmetic operations more than the original code (using the > macro results in an additional compare & branch). > How about just instead of:
if (unlikely(ip >= ip_end)) break;
to:
if(unlikely((ip - ip_end) < ~in_len)) break;
This just generates only one more arithmetic operation than original code, not easy to grasp but more efficient.
Thanks, Yueyi
| |