[SSSD] [sssd PR#141][comment] PAM: Use cache_req to perform initgroups lookups
URL: https://github.com/SSSD/sssd/pull/141 Title: #141: PAM: Use cache_req to perform initgroups lookups jhrozek commented: """ master: * c99bcc91e58988ff37403d362d4322469413c2ff * 5aaaf081765b3f23e1518b5f299c289afb9d3f13 * ed891c0c55985cd25de05f65e82debf4452987e1 * 189db5329db0277425bb3bdd785c19aee23bb364 """ See the full comment at https://github.com/SSSD/sssd/pull/141#issuecomment-283946815 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#141][comment] PAM: Use cache_req to perform initgroups lookups
URL: https://github.com/SSSD/sssd/pull/141 Title: #141: PAM: Use cache_req to perform initgroups lookups jhrozek commented: """ ACK CI: http://sssd-ci.duckdns.org/logs/job/63/74/summary.html """ See the full comment at https://github.com/SSSD/sssd/pull/141#issuecomment-283935188 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#141][comment] PAM: Use cache_req to perform initgroups lookups
URL: https://github.com/SSSD/sssd/pull/141 Title: #141: PAM: Use cache_req to perform initgroups lookups fidencio commented: """ @jhrozek: done! """ See the full comment at https://github.com/SSSD/sssd/pull/141#issuecomment-283803546 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#141][comment] PAM: Use cache_req to perform initgroups lookups
URL: https://github.com/SSSD/sssd/pull/141 Title: #141: PAM: Use cache_req to perform initgroups lookups jhrozek commented: """ I found one bug in the patches that I pushed into my branch: https://github.com/jhrozek/sssd/tree/pam_cache_req @fidencio if you agree, please squash it in, squash the two fixups and resubmit the patches. There were also two unrelated bugs I found, one is in PR#179 and one in PR#176 """ See the full comment at https://github.com/SSSD/sssd/pull/141#issuecomment-283714150 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#141][comment] PAM: Use cache_req to perform initgroups lookups
URL: https://github.com/SSSD/sssd/pull/141 Title: #141: PAM: Use cache_req to perform initgroups lookups jhrozek commented: """ retest this please """ See the full comment at https://github.com/SSSD/sssd/pull/141#issuecomment-283624101 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#141][comment] PAM: Use cache_req to perform initgroups lookups
URL: https://github.com/SSSD/sssd/pull/141 Title: #141: PAM: Use cache_req to perform initgroups lookups jhrozek commented: """ (this was for Jenkins, I don't know why the centos CI failed, so I'm just re-running it) """ See the full comment at https://github.com/SSSD/sssd/pull/141#issuecomment-283624181 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#141][comment] PAM: Use cache_req to perform initgroups lookups
URL: https://github.com/SSSD/sssd/pull/141 Title: #141: PAM: Use cache_req to perform initgroups lookups jhrozek commented: """ CI: http://sssd-ci.duckdns.org/logs/job/63/64/summary.html (yes, I know we need to squash patches later and I'm running some manual tests, but so far so good) """ See the full comment at https://github.com/SSSD/sssd/pull/141#issuecomment-283624071 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#141][comment] PAM: Use cache_req to perform initgroups lookups
URL: https://github.com/SSSD/sssd/pull/141 Title: #141: PAM: Use cache_req to perform initgroups lookups fidencio commented: """ New patch set pushed. """ See the full comment at https://github.com/SSSD/sssd/pull/141#issuecomment-283163599 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#141][comment] PAM: Use cache_req to perform initgroups lookups
URL: https://github.com/SSSD/sssd/pull/141
Title: #141: PAM: Use cache_req to perform initgroups lookups
jhrozek commented:
"""
I'm afraid Coverity found something it doesn't like now:
Error: NULL_RETURNS (CWE-476):
sssd-1.15.1/src/responder/pam/pamsrv_cmd.c:1462: returned_null:
"cache_req_data_name" returns null (checked 9 out of 10 times).
sssd-1.15.1/src/responder/common/cache_req/cache_req_data.c:250:5: null_return:
Calling "cache_req_data_create" which might return null.
sssd-1.15.1/src/responder/common/cache_req/cache_req_data.c:78:9: return_null:
Explicitly returning null.
sssd-1.15.1/src/responder/common/cache_req/cache_req_data.c:250:5:
return_null_fn: Returning the return value of "cache_req_data_create", which
might be null.
sssd-1.15.1/src/responder/common/cache_req/plugins/cache_req_group_by_filter.c:148:
example_assign: Example 1: Assigning: "data" = return value from
"cache_req_data_name(mem_ctx, CACHE_REQ_GROUP_BY_FILTER, filter)".
sssd-1.15.1/src/responder/common/cache_req/plugins/cache_req_group_by_filter.c:149:
example_checked: Example 1 (cont.): "data" has its value checked in "data ==
NULL".
sssd-1.15.1/src/responder/common/cache_req/plugins/cache_req_group_by_name.c:213:
example_assign: Example 2: Assigning: "data" = return value from
"cache_req_data_name(mem_ctx, CACHE_REQ_GROUP_BY_NAME, name)".
sssd-1.15.1/src/responder/common/cache_req/plugins/cache_req_group_by_name.c:214:
example_checked: Example 2 (cont.): "data" has its value checked in "data ==
NULL".
sssd-1.15.1/src/responder/common/cache_req/plugins/cache_req_initgroups_by_name.c:228:
example_assign: Example 3: Assigning: "data" = return value from
"cache_req_data_name(mem_ctx, CACHE_REQ_INITGROUPS, name)".
sssd-1.15.1/src/responder/common/cache_req/plugins/cache_req_initgroups_by_name.c:229:
example_checked: Example 3 (cont.): "data" has its value checked in "data ==
NULL".
sssd-1.15.1/src/responder/common/cache_req/plugins/cache_req_netgroup_by_name.c:147:
example_assign: Example 4: Assigning: "data" = return value from
"cache_req_data_name(mem_ctx, CACHE_REQ_NETGROUP_BY_NAME, name)".
sssd-1.15.1/src/responder/common/cache_req/plugins/cache_req_netgroup_by_name.c:148:
example_checked: Example 4 (cont.): "data" has its value checked in "data ==
NULL".
sssd-1.15.1/src/responder/common/cache_req/plugins/cache_req_user_by_filter.c:148:
example_assign: Example 5: Assigning: "data" = return value from
"cache_req_data_name(mem_ctx, CACHE_REQ_USER_BY_FILTER, filter)".
sssd-1.15.1/src/responder/common/cache_req/plugins/cache_req_user_by_filter.c:149:
example_checked: Example 5 (cont.): "data" has its value checked in "data ==
NULL".
sssd-1.15.1/src/responder/pam/pamsrv_cmd.c:1462: var_assigned: Assigning:
"data" = null return value from "cache_req_data_name".
sssd-1.15.1/src/responder/pam/pamsrv_cmd.c:1475: dereference: Dereferencing a
pointer that might be null "data" when calling
"cache_req_data_set_bypass_cache".
sssd-1.15.1/src/responder/common/cache_req/cache_req_data.c:365:5: deref_parm:
Directly dereferencing parameter "data".
# 363| bool bypass_cache)
# 364| {
# 365|-> data->bypass_cache = bypass_cache;
# 366| }
"""
See the full comment at
https://github.com/SSSD/sssd/pull/141#issuecomment-283039533
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#141][comment] PAM: Use cache_req to perform initgroups lookups
URL: https://github.com/SSSD/sssd/pull/141 Title: #141: PAM: Use cache_req to perform initgroups lookups spbnick commented: """ All my comments were addressed, ACK on my points. Thanks, @fidencio! """ See the full comment at https://github.com/SSSD/sssd/pull/141#issuecomment-282267591 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#141][comment] PAM: Use cache_req to perform initgroups lookups
URL: https://github.com/SSSD/sssd/pull/141 Title: #141: PAM: Use cache_req to perform initgroups lookups fidencio commented: """ CI: http://sssd-ci.duckdns.org/logs/job/63/43/summary.html """ See the full comment at https://github.com/SSSD/sssd/pull/141#issuecomment-282157874 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#141][comment] PAM: Use cache_req to perform initgroups lookups
URL: https://github.com/SSSD/sssd/pull/141 Title: #141: PAM: Use cache_req to perform initgroups lookups fidencio commented: """ Big thanks to @sumit-bose who helped me with the tests not passing due to the rebase and for pointing out that a whole piece of code that could also be dropped as part of the "PAM: Use cache_req to perform initgroups lookups" patch. I'm updating this new series here and I'd highly recommend to have this patchset re-tested by the reviewers. """ See the full comment at https://github.com/SSSD/sssd/pull/141#issuecomment-282144466 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#141][comment] PAM: Use cache_req to perform initgroups lookups
URL: https://github.com/SSSD/sssd/pull/141 Title: #141: PAM: Use cache_req to perform initgroups lookups fidencio commented: """ @jhrozek: I'm doing that, but it's going to take some time and I do believe patches will have to be reviewed again (as more changes are being done in order to have the tests passing ...) """ See the full comment at https://github.com/SSSD/sssd/pull/141#issuecomment-281972197 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#141][comment] PAM: Use cache_req to perform initgroups lookups
URL: https://github.com/SSSD/sssd/pull/141 Title: #141: PAM: Use cache_req to perform initgroups lookups jhrozek commented: """ @fidencio , can you rebase this branch atop Sumit's PKINIT patches so we can merge it? """ See the full comment at https://github.com/SSSD/sssd/pull/141#issuecomment-281947395 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#141][comment] PAM: Use cache_req to perform initgroups lookups
URL: https://github.com/SSSD/sssd/pull/141 Title: #141: PAM: Use cache_req to perform initgroups lookups sumit-bose commented: """ I've checked and tested the patches and all worked well. So ACK and if spbnick's comments are addressed the patches can be pushed. While testing UPN/email/Kerberos alias login names I came across a general issue with UPN related cache_req lookup https://fedorahosted.org/sssd/ticket/3313 . This is unrelated to this patch set, I junst want to mention that testing the patches with UPNs might fail. """ See the full comment at https://github.com/SSSD/sssd/pull/141#issuecomment-281092132 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#141][comment] PAM: Use cache_req to perform initgroups lookups
URL: https://github.com/SSSD/sssd/pull/141 Title: #141: PAM: Use cache_req to perform initgroups lookups spbnick commented: """ @fidencio Sorry, I was able to understand very little from the comment, so I can't really suggest a better commit message. """ See the full comment at https://github.com/SSSD/sssd/pull/141#issuecomment-281073850 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#141][comment] PAM: Use cache_req to perform initgroups lookups
URL: https://github.com/SSSD/sssd/pull/141 Title: #141: PAM: Use cache_req to perform initgroups lookups pbrezina commented: """ Try to describe how PAM responder works with cache and where it differes with other responders in few words. If you have time, do it now before next review. You want to do it anyway and typing `git push` is not that hard in the end ;-) """ See the full comment at https://github.com/SSSD/sssd/pull/141#issuecomment-281072688 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#141][comment] PAM: Use cache_req to perform initgroups lookups
URL: https://github.com/SSSD/sssd/pull/141 Title: #141: PAM: Use cache_req to perform initgroups lookups fidencio commented: """ @spbnick: Thanks for the review. I'll submit a new version when someone else does the review as well (yeah, laziness, sorry). About rephrasing the commit message, do you have any suggestion? """ See the full comment at https://github.com/SSSD/sssd/pull/141#issuecomment-281001450 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#141][comment] PAM: Use cache_req to perform initgroups lookups
URL: https://github.com/SSSD/sssd/pull/141 Title: #141: PAM: Use cache_req to perform initgroups lookups spbnick commented: """ I'll try to review this. """ See the full comment at https://github.com/SSSD/sssd/pull/141#issuecomment-279670819 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#141][comment] PAM: Use cache_req to perform initgroups lookups
URL: https://github.com/SSSD/sssd/pull/141 Title: #141: PAM: Use cache_req to perform initgroups lookups fidencio commented: """ Patch set has been rebased on top of git master. """ See the full comment at https://github.com/SSSD/sssd/pull/141#issuecomment-279357985 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#141][comment] PAM: Use cache_req to perform initgroups lookups
URL: https://github.com/SSSD/sssd/pull/141 Title: #141: PAM: Use cache_req to perform initgroups lookups pbrezina commented: """ Ack from me. Can someone else do review as well since I was helping Fabiano with those patches? """ See the full comment at https://github.com/SSSD/sssd/pull/141#issuecomment-277975708 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#141][comment] PAM: Use cache_req to perform initgroups lookups
URL: https://github.com/SSSD/sssd/pull/141 Title: #141: PAM: Use cache_req to perform initgroups lookups fidencio commented: """ CI: http://sssd-ci.duckdns.org/logs/job/61/72/summary.html """ See the full comment at https://github.com/SSSD/sssd/pull/141#issuecomment-277927439 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#141][comment] PAM: Use cache_req to perform initgroups lookups
URL: https://github.com/SSSD/sssd/pull/141 Title: #141: PAM: Use cache_req to perform initgroups lookups fidencio commented: """ @pbrezina, commits are ready to be reviewed. @spbnick, you're not (totally) blocked anymore. """ See the full comment at https://github.com/SSSD/sssd/pull/141#issuecomment-277919987 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
