[SSSD] [sssd][patch] SYSDB: sysdb_getnetgr returns ENOENT

Pavel Reichl preichl at redhat.com
Thu Aug 28 09:49:38 UTC 2014


On 08/28/2014 10:31 AM, Lukas Slebodnik wrote:
> On (11/07/14 17:28), Pavel Reichl wrote:
>> On Wed, 2014-06-25 at 10:51 -0400, Simo Sorce wrote:
>>> On Tue, 2014-06-24 at 17:28 +0200, Pavel Reichl wrote:
>>>>> Error: PW.NON_CONST_PRINTF_FORMAT_STRING:
>>>>> sssd-1.11.92/src/db/sysdb_ops.c:109: non_const_printf_format_string:
>>>> format string is not a string literal, potential security
>>>> vulnerability if user controlled
>>>>> sssd-1.11.92/src/db/sysdb_ops.c:109: caretline:
>>>>>
>>>>> I prefer to avoid compiler warnings. It will be better to use NULL.
>>>> (lret = ldb_search(ldb, mem_ctx, _result, base, scope, attrs, NULL);)
>>>>
>>>> It's a common practice in SSSD anyway.
>>> Wouldn't it be simpler to use a macro in this case ?
>>> Your new fuinction hjas to go through variadic expansion prematurely,
>>> and adds a lot of code:
>>>
>>> something like:
>>> #define sss_ldbn_seartch(ret, arguments, etc., format, ...) {
>>> do {
>>>     int result = ldb_search(argumets, etc, format, ##__VA_ARGS__);
>>>     ret = result;
>>>     if (res->count == 0) {
>>>         ret = ENOENT;
>>>     }
>>> } while(0);
>>>
>>>
>> Hello,
>>
>> sorry for late reply. I guess your approach will definitely save some
>> lines of code and some CPU cycles.
>>
>> I just must get used to the idea that macros are not considered evil in
>> out environment. :-)
>>
>> Please see attached patch.
>>
>> Regards,
>>
>> Pavel Reichl
>>
> >From d1293b63adcab4298228af225f3af5803759d34b Mon Sep 17 00:00:00 2001
>> From: Pavel Reichl <preichl at redhat.com>
>> Date: Fri, 11 Jul 2014 15:21:59 +0100
>> Subject: [PATCH] SYSDB: SSS_LDB_SEARCH - macro around ldb_search
>>
>> This patch amends previous patch 5153e8b9793dea1e212ca08af0f77ea1d023cbb7.
>>
>> Macro SSS_LDB_SEARCH is used instead of using fuction sss_ldb_search as
>> a wrapper around ldb_search which could lead to premature expansion of
>> variadic parameters.
>>
>> Part of solution for:
>> https://fedorahosted.org/sssd/ticket/1991
>> ---
>> src/db/sysdb.h          | 19 +++++++++++++-----
>> src/db/sysdb_ops.c      | 51 -------------------------------------------------
>> src/db/sysdb_search.c   | 10 +++++-----
>> src/tests/sysdb-tests.c | 38 ++++++++++++++++++------------------
>> 4 files changed, 38 insertions(+), 80 deletions(-)
>>
>> diff --git a/src/db/sysdb.h b/src/db/sysdb.h
>> index 17cd5110c9bdafd8d7b18e621188523ed41e5c8a..a613c84ffa9e4ab6ebdc930a8c4876e69aa3cd13 100644
>> --- a/src/db/sysdb.h
>> +++ b/src/db/sysdb.h
>> @@ -481,11 +481,6 @@ int sysdb_get_netgroup_attr(TALLOC_CTX *mem_ctx,
>>                              const char **attributes,
>>                              struct ldb_result **res);
>>
>> -errno_t sss_ldb_search(struct ldb_context *ldb, TALLOC_CTX *mem_ctx,
>> -                       struct ldb_result **_result, struct ldb_dn *base,
>> -                       enum ldb_scope scope, const char * const *attrs,
>> -                       const char *exp_fmt, ...) SSS_ATTRIBUTE_PRINTF(7, 8);
>> -
>> /* functions that modify the databse
>>   * they have to be called within a transaction
>>   * See sysdb_transaction_send()/_recv() */
>> @@ -864,4 +859,18 @@ errno_t sysdb_search_object_by_sid(TALLOC_CTX *mem_ctx,
>>                                     const char *sid_str,
>>                                     const char **attrs,
>>                                     struct ldb_result **msg);
>> +
>> +#define SSS_LDB_SEARCH(ret, ldb, mem_ctx, _result, base, scope, attrs,    \
>> +                       exp_fmt, ...)                                      \
>> +{                                                                         \
>> +    int lret;                                                             \
>> +                                                                          \
>> +    lret = ldb_search(ldb, mem_ctx, _result, base, scope, attrs, exp_fmt, \
>> +                      ##__VA_ARGS__);                                     \
>> +    ret = sysdb_error_to_errno(lret);                                     \
>> +    if (ret == EOK && (*_result)->count == 0) {                           \
>> +        ret = ENOENT;                                                     \
>> +    }                                                                     \
>> +} while(0)
> Use do { } while(0); for macro, not just { } while(0)
>
> @see macros SYSDB_VERSION_LOWER_ERROR, SYSDB_VERSION_HIGHER_ERROR,
> DEBUG_PAM_DATA ...
>
> Otherwise patch LGTM. If there are no other objections to this version I will
> give ACK.
>
> LS
> _______________________________________________
> sssd-devel mailing list
> sssd-devel at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Thanks, updated patch attached.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-SYSDB-SSS_LDB_SEARCH-macro-around-ldb_search.patch
Type: text/x-patch
Size: 9283 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20140828/852c1a81/attachment.bin>


More information about the sssd-devel mailing list