URL: https://github.com/SSSD/sssd/pull/455
Title: #455: mmap_cache: make checks independent of input size

lslebodn commented:
"""
On (24/11/17 07:10), sumit-bose wrote:
>I kept the memchr() check mainly for the initgroups cache data. For passwd and 
>group the first element in the string/data area is the name and hence 
>'key->len > strs_len' makes sure the strcmp() will not read pass the current 
>object. For the initgroups data the name is 'somewhere' in the data area 
>because the data starts with a list of GIDs. So to avoid that strcmp() goes 
>pass the end of the data area with a long key in the case of a corruption the 
>"length" of t_key has to be determined. Using strnlen() would be possible as 
>well but imo memchr() is more clear here.
>

I'm sorry I cannot see any benefit for initgroups case.
I just can see performance impact in 99.9999% cases
with iterating over data twice (firstly with `memchr` secondly with `strcmp`)

It is not 100% cases because we still assume that memory cache can be
corrupted.

As I wrote in previous comment all checks before `memchr` are sufficient
to ensure that comparing data with NUL terminated input will not overflow area
of data table.

I'm fine with checking bounds because comparing numbers are fast. But I cannot
see any benefit of checking data twice.

Or did I miss something why directly using `strcmp` is unsafe?

LS

"""

See the full comment at 
https://github.com/SSSD/sssd/pull/455#issuecomment-346824326
_______________________________________________
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

Reply via email to