[SSSD] [PATCH] NSS: Handle ENOENT when doing initgroups by UPN

Pavel Reichl preichl at redhat.com
Wed Mar 11 15:30:52 UTC 2015


On 03/11/2015 03:46 PM, Jakub Hrozek wrote:


> @@ -4062,29 +4062,39 @@ static int nss_cmd_initgroups_search(struct nss_dom_ctx *dctx)
>   
>           if (cmdctx->name_is_upn) {
>               ret = sysdb_search_user_by_upn(cmdctx, dom, name, user_attrs, &msg);
> -            if (ret != EOK && ret != ENOENT) {
> +            if (ret == EOK) {
> +                sysdb_name = ldb_msg_find_attr_as_string(msg, SYSDB_NAME, NULL);
> +                if (sysdb_name == NULL) {
> +                    DEBUG(SSSDBG_OP_FAILURE,
> +                        "Sysdb entry does not have a name.\n");
> +                    return EINVAL;
> +                }
> +
> +                ret = sysdb_initgroups(cmdctx, dom, sysdb_name, &dctx->res);
> +                if (ret == EOK && DOM_HAS_VIEWS(dom)) {
> +                    for (c = 0; c < dctx->res->count; c++) {
> +                        ret = sysdb_add_overrides_to_object(dom, dctx->res->msgs[c],
> +                                                            NULL, NULL);
> +                        if (ret != EOK) {
> +                            DEBUG(SSSDBG_OP_FAILURE,
> +                                "sysdb_add_overrides_to_object failed.\n");
> +                            return ret;
> +                        }
> +                    }
> +                }
> +            } else if (ret != ENOENT) {
>                   DEBUG(SSSDBG_OP_FAILURE, "sysdb_search_user_by_upn failed.\n");
>                   return ret;
> -            }
> -
> -            sysdb_name = ldb_msg_find_attr_as_string(msg, SYSDB_NAME, NULL);
> -            if (sysdb_name == NULL) {
> -                DEBUG(SSSDBG_OP_FAILURE,
> -                      "Sysdb entry does not have a name.\n");
> -                return EINVAL;
> -            }
> -
> -            ret = sysdb_initgroups(cmdctx, dom, sysdb_name, &dctx->res);
> -            if (ret == EOK && DOM_HAS_VIEWS(dom)) {
> -                for (c = 0; c < dctx->res->count; c++) {
> -                    ret = sysdb_add_overrides_to_object(dom, dctx->res->msgs[c],
> -                                                        NULL, NULL);
> -                    if (ret != EOK) {
> -                        DEBUG(SSSDBG_OP_FAILURE,
> -                              "sysdb_add_overrides_to_object failed.\n");
> -                        return ret;
> -                    }
> +            } else {
> +                dctx->res = talloc_zero(cmdctx, struct ldb_result);
> +                if (dctx->res == NULL) {
> +                    DEBUG(SSSDBG_OP_FAILURE, "talloc_zero failed.\n");
> +                    return ENOMEM;
>                   }
> +
> +                dctx->res->count = 0;
> +                dctx->res->msgs = NULL;
> +                ret = EOK;
>               }
>           } else {
>               ret = sysdb_initgroups_with_views(cmdctx, dom, name, &dctx->res);
> -- 2.1.0
Thanks for the patches.
All patches LGTM.

I've tested that tests fail if the forth patch is not applied. I also 
wrote simple program to reproduce the segfault and after applying the 
4th patch I'm no longer able to reproduce the bug.

I have only one style nitpick - would it make more sense to test 
explicitly for 'ret == ENOENT' and handle the other cases in else branch?

If you don't agree then ignore then comment and push the patches (ACK to 
all).

Thanks!

ci passed:

http://sssd-ci.duckdns.org/logs/job/9/46/summary.html







More information about the sssd-devel mailing list