[SSSD] [PATCHES] GPO: fail if there is problem with storing gpo into sysdb

Lukas Slebodnik lslebodn at redhat.com
Mon Oct 13 07:58:56 UTC 2014


On (12/10/14 17:21), Jakub Hrozek wrote:
>On Sat, Oct 11, 2014 at 11:49:54PM +0200, Lukas Slebodnik wrote:
>> ehlo,
>> 
>> some variable could be used unitialized.
>> 
>> Simple patches are attached.
>> 
>> LS
>
>> From f2a7faefba6e6191f324c4c33609ccb7570835e5 Mon Sep 17 00:00:00 2001
>> From: Lukas Slebodnik <lslebodn at redhat.com>
>> Date: Sat, 11 Oct 2014 12:55:05 +0200
>> Subject: [PATCH 7/9] GPO: fail if there is problem with storing gpo into sysdb
>> 
>> ---
>>  src/providers/ad/ad_gpo.c | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>> 
>> diff --git a/src/providers/ad/ad_gpo.c b/src/providers/ad/ad_gpo.c
>> index 9f52aedf8d10d6a870846f311b5347896fba31bd..6fdcbe3fe65b31d56123ddea95a5e46282cfb599 100644
>> --- a/src/providers/ad/ad_gpo.c
>> +++ b/src/providers/ad/ad_gpo.c
>> @@ -1147,6 +1147,11 @@ ad_gpo_store_policy_settings(struct sss_domain_info *domain,
>>                  ret = sysdb_gpo_store_gpo_result_setting(domain,
>>                                                           allow_key,
>>                                                           allow_value);
>> +                DEBUG(SSSDBG_CRIT_FAILURE,
>> +                      "sysdb_gpo_store_gpo_result_setting failed for key:'%s' "
>> +                      "value:'%s' [%d][%s]\n", allow_key, allow_value,
>> +                      ret, sss_strerror(ret));
>> +                goto done;
>
>I think the debug and goto should be wrapped in an if statement here.
>
>>              }
>>          }
>>  
>> @@ -1166,6 +1171,11 @@ ad_gpo_store_policy_settings(struct sss_domain_info *domain,
>>                  ret = sysdb_gpo_store_gpo_result_setting(domain,
>>                                                           deny_key,
>>                                                           deny_value);
>> +                DEBUG(SSSDBG_CRIT_FAILURE,
>> +                      "sysdb_gpo_store_gpo_result_setting failed for key:'%s' "
>> +                      "value:'%s' [%d][%s]\n", deny_key, deny_value,
>> +                      ret, sss_strerror(ret));
>> +                goto done;
>
>Same here
>
Copy paste problem.
Fixed version is attached.

LS
-------------- next part --------------
>From 124dd7cf4fba5f0038fbdd0d78237e4d861bffa3 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik <lslebodn at redhat.com>
Date: Sat, 11 Oct 2014 12:55:05 +0200
Subject: [PATCH 1/3] GPO: fail if there is problem with storing gpo into sysdb

---
 src/providers/ad/ad_gpo.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/src/providers/ad/ad_gpo.c b/src/providers/ad/ad_gpo.c
index 9f52aedf8d10d6a870846f311b5347896fba31bd..d581db8fc8529644a9e2471e57e45100423e8624 100644
--- a/src/providers/ad/ad_gpo.c
+++ b/src/providers/ad/ad_gpo.c
@@ -1147,6 +1147,13 @@ ad_gpo_store_policy_settings(struct sss_domain_info *domain,
                 ret = sysdb_gpo_store_gpo_result_setting(domain,
                                                          allow_key,
                                                          allow_value);
+                if (ret != EOK) {
+                    DEBUG(SSSDBG_CRIT_FAILURE,
+                          "sysdb_gpo_store_gpo_result_setting failed for key:"
+                          "'%s' value:'%s' [%d][%s]\n", allow_key, allow_value,
+                          ret, sss_strerror(ret));
+                    goto done;
+                }
             }
         }
 
@@ -1166,6 +1173,13 @@ ad_gpo_store_policy_settings(struct sss_domain_info *domain,
                 ret = sysdb_gpo_store_gpo_result_setting(domain,
                                                          deny_key,
                                                          deny_value);
+                if (ret != EOK) {
+                    DEBUG(SSSDBG_CRIT_FAILURE,
+                          "sysdb_gpo_store_gpo_result_setting failed for key:"
+                          "'%s' value:'%s' [%d][%s]\n", deny_key, deny_value,
+                          ret, sss_strerror(ret));
+                    goto done;
+                }
             }
         }
     }
-- 
2.1.0

-------------- next part --------------
>From 9c8939c14815b4266d3ad46f3386df8535eeda14 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik <lslebodn at redhat.com>
Date: Sat, 11 Oct 2014 13:10:18 +0200
Subject: [PATCH 2/3] GPO: Fail if we cannot retrieve gpo from cache.

The variable value could be used uninitialized.
---
 src/providers/ad/ad_gpo.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/src/providers/ad/ad_gpo.c b/src/providers/ad/ad_gpo.c
index d581db8fc8529644a9e2471e57e45100423e8624..2d8ba82da10498cb5ff7000613fa5ba8cbae1686 100644
--- a/src/providers/ad/ad_gpo.c
+++ b/src/providers/ad/ad_gpo.c
@@ -1358,6 +1358,12 @@ parse_policy_setting_value(TALLOC_CTX *mem_ctx,
     char **sids_list = NULL;
 
     ret = sysdb_gpo_get_gpo_result_setting(mem_ctx, domain, key, &value);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_OP_FAILURE,
+              "Cannot retrieve settings from sysdb for key: '%s' [%d][%s].\n",
+              key, ret, sss_strerror(ret));
+        goto done;
+    }
 
     if (value == NULL) {
         DEBUG(SSSDBG_TRACE_FUNC,
-- 
2.1.0

-------------- next part --------------
>From 83260f699146085ef5a2d341d35eed1099d6a755 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik <lslebodn at redhat.com>
Date: Sat, 11 Oct 2014 21:38:32 +0200
Subject: [PATCH 3/3] GPO: Do not use output argument if function failed

deny_size and allow_size were initialized in function
parse_policy_setting_value (output argument).
If function parse_policy_setting_value fails then output arguments
will not initialized and will be used uninitialized in debug macro.
---
 src/providers/ad/ad_gpo.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/providers/ad/ad_gpo.c b/src/providers/ad/ad_gpo.c
index 2d8ba82da10498cb5ff7000613fa5ba8cbae1686..3f5df75c5a9de53eac11ffcf785e929cf9b3165e 100644
--- a/src/providers/ad/ad_gpo.c
+++ b/src/providers/ad/ad_gpo.c
@@ -1431,8 +1431,8 @@ ad_gpo_perform_hbac_processing(TALLOC_CTX *mem_ctx,
                                      &allow_sids, &allow_size);
     if (ret != EOK) {
         DEBUG(SSSDBG_OP_FAILURE,
-              "parse_policy_setting_value failed; allow_size %d: %d\n",
-              allow_size, ret);
+              "parse_policy_setting_value failed for key %s: [%d](%s)\n",
+              allow_key, ret, sss_strerror(ret));
         ret = EINVAL;
         goto done;
     }
@@ -1441,8 +1441,8 @@ ad_gpo_perform_hbac_processing(TALLOC_CTX *mem_ctx,
                                      &deny_sids, &deny_size);
     if (ret != EOK) {
         DEBUG(SSSDBG_OP_FAILURE,
-              "parse_policy_setting_value failed; deny_size %d: %d\n",
-              deny_size, ret);
+              "parse_policy_setting_value failed for key %s: [%d](%s)\n",
+              deny_key, ret, sss_strerror(ret));
         ret = EINVAL;
         goto done;
     }
-- 
2.1.0



More information about the sssd-devel mailing list