[SSSD] [sssd PR#455][comment] mmap_cache: make checks independent of input size

2017-11-27 Thread jhrozek
URL: https://github.com/SSSD/sssd/pull/455 Title: #455: mmap_cache: make checks independent of input size jhrozek commented: """ OK, good. There is a downstream request for a release that is based on the sssd-1-13 branch, so I'm quite sure I won't forget about the backport. btw it's not needed

[SSSD] [sssd PR#455][comment] mmap_cache: make checks independent of input size

2017-11-27 Thread fidencio
URL: https://github.com/SSSD/sssd/pull/455 Title: #455: mmap_cache: make checks independent of input size fidencio commented: """ I haven't ran coverity on sssd-1-13 branch. I'd say no. Let me run coverity at some point this week and then I can submit a PR with all the fixes. """ See the full

[SSSD] [sssd PR#455][comment] mmap_cache: make checks independent of input size

2017-11-27 Thread jhrozek
URL: https://github.com/SSSD/sssd/pull/455 Title: #455: mmap_cache: make checks independent of input size jhrozek commented: """ @fidencio do you know if the Coverity patch should be backported to sssd-1-13 as well? """ See the full comment at https://github.com/SSSD/sssd/pull/455#issuecommen

[SSSD] [sssd PR#455][comment] mmap_cache: make checks independent of input size

2017-11-27 Thread lslebodn
URL: https://github.com/SSSD/sssd/pull/455 Title: #455: mmap_cache: make checks independent of input size lslebodn commented: """ http://vm-031.$ABC/logs/job/81/85/summary.html """ See the full comment at https://github.com/SSSD/sssd/pull/455#issuecomment-347320388

[SSSD] [sssd PR#455][comment] mmap_cache: make checks independent of input size

2017-11-27 Thread lslebodn
URL: https://github.com/SSSD/sssd/pull/455 Title: #455: mmap_cache: make checks independent of input size lslebodn commented: """ master: * ec13304e0c4dc1ffe8c6abe6d0741fa7f6da3ceb * c4027058427d79d81404331ad0717907e56e87c4 """ See the full comment at https://github.com/SSSD/sssd/pull/455#issu

[SSSD] [sssd PR#455][comment] mmap_cache: make checks independent of input size

2017-11-27 Thread jhrozek
URL: https://github.com/SSSD/sssd/pull/455 Title: #455: mmap_cache: make checks independent of input size jhrozek commented: """ * master: * 1d88a0591ce8445ea3b6a88845a5997d61c915b4

[SSSD] [sssd PR#455][comment] mmap_cache: make checks independent of input size

2017-11-27 Thread lslebodn
URL: https://github.com/SSSD/sssd/pull/455 Title: #455: mmap_cache: make checks independent of input size lslebodn commented: """ master: * ec13304e0c4dc1ffe8c6abe6d0741fa7f6da3ceb * c4027058427d79d81404331ad0717907e56e87c4 """ See the full comment at https://github.com/SSSD/sssd/pull/455#issu

[SSSD] [sssd PR#455][comment] mmap_cache: make checks independent of input size

2017-11-27 Thread lslebodn
URL: https://github.com/SSSD/sssd/pull/455 Title: #455: mmap_cache: make checks independent of input size lslebodn commented: """ On (27/11/17 14:23), sumit-bose wrote: >I think it has to be at least > >- if (key->len > strs_len) { >+ if (key->len + name_ptr - strs_offset > strs_len) { > >becaus

[SSSD] [sssd PR#455][comment] mmap_cache: make checks independent of input size

2017-11-27 Thread sumit-bose
URL: https://github.com/SSSD/sssd/pull/455 Title: #455: mmap_cache: make checks independent of input size sumit-bose commented: """ I think it has to be at least - if (key->len > strs_len) { + if (key->len + name_ptr - strs_offset > strs_len) { because name_ptr is relative to rec->data and inc

[SSSD] [sssd PR#455][comment] mmap_cache: make checks independent of input size

2017-11-27 Thread lslebodn
URL: https://github.com/SSSD/sssd/pull/455 Title: #455: mmap_cache: make checks independent of input size lslebodn commented: """ On (24/11/17 15:42), sumit-bose wrote: >I agree this is nitpicking and artificial, but I think strcmp() can run >outside the data table. > >If the last slot in the d

[SSSD] [sssd PR#455][comment] mmap_cache: make checks independent of input size

2017-11-24 Thread sumit-bose
URL: https://github.com/SSSD/sssd/pull/455 Title: #455: mmap_cache: make checks independent of input size sumit-bose commented: """ latest version has memchr() removed. """ See the full comment at https://github.com/SSSD/sssd/pull/455#issuecomment-346856048

[SSSD] [sssd PR#455][comment] mmap_cache: make checks independent of input size

2017-11-24 Thread sumit-bose
URL: https://github.com/SSSD/sssd/pull/455 Title: #455: mmap_cache: make checks independent of input size sumit-bose commented: """ I agree this is nitpicking and artificial, but I think strcmp() can run outside the data table. If the last slot in the data table is the last of an initgr record

[SSSD] [sssd PR#455][comment] mmap_cache: make checks independent of input size

2017-11-24 Thread lslebodn
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 ar

[SSSD] [sssd PR#455][comment] mmap_cache: make checks independent of input size

2017-11-23 Thread sumit-bose
URL: https://github.com/SSSD/sssd/pull/455 Title: #455: mmap_cache: make checks independent of input size sumit-bose commented: """ In the latest version I included the 'key->len > strs_len' check and modified the check if strs_len points out of the data section to avoid an overrun. I kept the

[SSSD] [sssd PR#455][comment] mmap_cache: make checks independent of input size

2017-11-23 Thread mzidek-rh
URL: https://github.com/SSSD/sssd/pull/455 Title: #455: mmap_cache: make checks independent of input size mzidek-rh commented: """ On 11/23/2017 12:50 PM, lslebodn wrote: > On (23/11/17 11:31), mzidek-rh wrote: > >mzidek-rh commented on this pull request. > >I do not think such optimisation is

[SSSD] [sssd PR#455][comment] mmap_cache: make checks independent of input size

2017-11-23 Thread lslebodn
URL: https://github.com/SSSD/sssd/pull/455 Title: #455: mmap_cache: make checks independent of input size lslebodn commented: """ On (23/11/17 11:31), mzidek-rh wrote: >mzidek-rh commented on this pull request. >I do not think such optimisation is needed And I do not think that these checks for

[SSSD] [sssd PR#455][comment] mmap_cache: make checks independent of input size

2017-11-22 Thread mzidek-rh
URL: https://github.com/SSSD/sssd/pull/455 Title: #455: mmap_cache: make checks independent of input size mzidek-rh commented: """ The CI results for @fidencio 's change arrived. ACK to the patchset. """ See the full comment at https://github.com/SSSD/sssd/pull/455#issuecomment-346316533

[SSSD] [sssd PR#455][comment] mmap_cache: make checks independent of input size

2017-11-21 Thread jhrozek
URL: https://github.com/SSSD/sssd/pull/455 Title: #455: mmap_cache: make checks independent of input size jhrozek commented: """ I'm confused, this PR has still the accepted label, but I guess we're still waiting for the CI results before pushing, right? """ See the full comment at https://gi

[SSSD] [sssd PR#455][comment] mmap_cache: make checks independent of input size

2017-11-21 Thread mzidek-rh
URL: https://github.com/SSSD/sssd/pull/455 Title: #455: mmap_cache: make checks independent of input size mzidek-rh commented: """ Fabiano's change LGTM. I resubmitted the patchset to the CI. """ See the full comment at https://github.com/SSSD/sssd/pull/455#issuecomment-346100524 _

[SSSD] [sssd PR#455][comment] mmap_cache: make checks independent of input size

2017-11-21 Thread sumit-bose
URL: https://github.com/SSSD/sssd/pull/455 Title: #455: mmap_cache: make checks independent of input size sumit-bose commented: """ Ok, the new version includes @fidencio's patch as well. """ See the full comment at https://github.com/SSSD/sssd/pull/455#issuecomment-346097469 _

[SSSD] [sssd PR#455][comment] mmap_cache: make checks independent of input size

2017-11-21 Thread fidencio
URL: https://github.com/SSSD/sssd/pull/455 Title: #455: mmap_cache: make checks independent of input size fidencio commented: """ I personally would just re-submit this PR with that patch atop, push this one and I can close that one. Does it work for you? """ See the full comment at https://

[SSSD] [sssd PR#455][comment] mmap_cache: make checks independent of input size

2017-11-21 Thread sumit-bose
URL: https://github.com/SSSD/sssd/pull/455 Title: #455: mmap_cache: make checks independent of input size sumit-bose commented: """ @fidencio, ok, better safe than sorry. How to proceed? Close this and merge #456? """ See the full comment at https://github.com/SSSD/sssd/pull/455#issuecomment-

[SSSD] [sssd PR#455][comment] mmap_cache: make checks independent of input size

2017-11-21 Thread fidencio
URL: https://github.com/SSSD/sssd/pull/455 Title: #455: mmap_cache: make checks independent of input size fidencio commented: """ And I totally agree it's a false positive. """ See the full comment at https://github.com/SSSD/sssd/pull/455#issuecomment-346081751

[SSSD] [sssd PR#455][comment] mmap_cache: make checks independent of input size

2017-11-21 Thread fidencio
URL: https://github.com/SSSD/sssd/pull/455 Title: #455: mmap_cache: make checks independent of input size fidencio commented: """ @sumit-bose, I did a new run and coverity did not detected this as a dead-code. :-/ """ See the full comment at https://github.com/SSSD/sssd/pull/455#issuecomment-

[SSSD] [sssd PR#455][comment] mmap_cache: make checks independent of input size

2017-11-21 Thread sumit-bose
URL: https://github.com/SSSD/sssd/pull/455 Title: #455: mmap_cache: make checks independent of input size sumit-bose commented: """ @fidencio, can you do a new Coverity run with your patch? I'm asking because I wonder if Coverity would be in the presence of your patch, clever enough to detect

[SSSD] [sssd PR#455][comment] mmap_cache: make checks independent of input size

2017-11-21 Thread sumit-bose
URL: https://github.com/SSSD/sssd/pull/455 Title: #455: mmap_cache: make checks independent of input size sumit-bose commented: """ @fidencio, can you do a nee Coverity run with your patch? I'm asking because I wonder if Coverity would be in the presence of your patch, clever enough to detect

[SSSD] [sssd PR#455][comment] mmap_cache: make checks independent of input size

2017-11-21 Thread fidencio
URL: https://github.com/SSSD/sssd/pull/455 Title: #455: mmap_cache: make checks independent of input size fidencio commented: """ @mzidek-rh, my patch does fix the warning and I do agree with you that it's just a false positive. Knowing that and agreeing with @jhrozek's comment, I'd like to ha

[SSSD] [sssd PR#455][comment] mmap_cache: make checks independent of input size

2017-11-21 Thread jhrozek
URL: https://github.com/SSSD/sssd/pull/455 Title: #455: mmap_cache: make checks independent of input size jhrozek commented: """ I think a general rule of thumb is - if there is a simple way to make the code analyzers happy, then the code should be written so that even a machine understands i

[SSSD] [sssd PR#455][comment] mmap_cache: make checks independent of input size

2017-11-21 Thread mzidek-rh
URL: https://github.com/SSSD/sssd/pull/455 Title: #455: mmap_cache: make checks independent of input size mzidek-rh commented: """ Hi @fidencio , at first look I also thought covscan found an issue here, but looking closer this seems to be a false positive match. Few lines above the check that

[SSSD] [sssd PR#455][comment] mmap_cache: make checks independent of input size

2017-11-21 Thread fidencio
URL: https://github.com/SSSD/sssd/pull/455 Title: #455: mmap_cache: make checks independent of input size fidencio commented: """ I will **not** add the changes request label as the issue found is **not** related to the code itself and could be fixed in a different commit. Do @sumit-bose and

[SSSD] [sssd PR#455][comment] mmap_cache: make checks independent of input size

2017-11-21 Thread fidencio
URL: https://github.com/SSSD/sssd/pull/455 Title: #455: mmap_cache: make checks independent of input size fidencio commented: """ Sorry for jumping in a patch set which I'm neither the author nor the reviewer. Covscan has found an issue with this patchset: ``` Error: NULL_RETURNS (CWE-476): [#d

[SSSD] [sssd PR#455][comment] mmap_cache: make checks independent of input size

2017-11-21 Thread mzidek-rh
URL: https://github.com/SSSD/sssd/pull/455 Title: #455: mmap_cache: make checks independent of input size mzidek-rh commented: """ Ack. """ See the full comment at https://github.com/SSSD/sssd/pull/455#issuecomment-346023383 ___ sssd-devel mailing li

[SSSD] [sssd PR#455][comment] mmap_cache: make checks independent of input size

2017-11-21 Thread mzidek-rh
URL: https://github.com/SSSD/sssd/pull/455 Title: #455: mmap_cache: make checks independent of input size mzidek-rh commented: """ Thanks Sumit. Just waiting for the CI to finish before ACKing. """ See the full comment at https://github.com/SSSD/sssd/pull/455#issuecomment-345974016 ___

[SSSD] [sssd PR#455][comment] mmap_cache: make checks independent of input size

2017-11-21 Thread sumit-bose
URL: https://github.com/SSSD/sssd/pull/455 Title: #455: mmap_cache: make checks independent of input size sumit-bose commented: """ Thank you for the review, I included your changes in the latest version. """ See the full comment at https://github.com/SSSD/sssd/pull/455#issuecomment-345956489

[SSSD] [sssd PR#455][comment] mmap_cache: make checks independent of input size

2017-11-20 Thread mzidek-rh
URL: https://github.com/SSSD/sssd/pull/455 Title: #455: mmap_cache: make checks independent of input size mzidek-rh commented: """ Just two really minor nitpicks (typo in word 'relative' and missing space around '+'): ``` diff --git a/src/responder/nss/nsssrv_mmap_cache.c b/src/responder/nss/

[SSSD] [sssd PR#455][comment] mmap_cache: make checks independent of input size

2017-11-20 Thread mzidek-rh
URL: https://github.com/SSSD/sssd/pull/455 Title: #455: mmap_cache: make checks independent of input size mzidek-rh commented: """ I will review this today. """ See the full comment at https://github.com/SSSD/sssd/pull/455#issuecomment-345658954 ___

[SSSD] [sssd PR#455][comment] mmap_cache: make checks independent of input size

2017-11-20 Thread mzidek-rh
URL: https://github.com/SSSD/sssd/pull/455 Title: #455: mmap_cache: make checks independent of input size mzidek-rh commented: """ Will review this today. """ See the full comment at https://github.com/SSSD/sssd/pull/455#issuecomment-345658954 ___ ss

[SSSD] [sssd PR#455][comment] mmap_cache: make checks independent of input size

2017-11-17 Thread jhrozek
URL: https://github.com/SSSD/sssd/pull/455 Title: #455: mmap_cache: make checks independent of input size jhrozek commented: """ Since @mzidek-rh was already looking into the issue earlier, I hope Michal can help with the review as well. """ See the full comment at https://github.com/SSSD/sss