[SSSD] [PATCH] NSS: Check allocation result

Pavel Březina pbrezina at redhat.com
Thu Oct 31 09:37:13 UTC 2013


On 10/22/2013 12:29 PM, Jakub Hrozek wrote:
> On Mon, Oct 21, 2013 at 05:33:33PM +0200, Jakub Hrozek wrote:
>> On Mon, Oct 21, 2013 at 10:38:19AM -0400, Simo Sorce wrote:
>>> On Mon, 2013-10-21 at 11:03 +0200, Jakub Hrozek wrote:
>>>> Another small bug I found when looking for #1020945
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>> plain text
>>>> document
>>>> attachment
>>>> (0001-NSS-Check-allocation-result.patch)
>>>>
>>>>  From b1d04686f085e25f10dde82f1e19c89278883001 Mon Sep 17 00:00:00 2001
>>>> From: Jakub Hrozek <jhrozek at redhat.com>
>>>> Date: Sun, 20 Oct 2013 19:24:04 +0200
>>>> Subject: [PATCH] NSS: Check allocation result
>>>>
>>>> ---
>>>>   src/responder/nss/nsssrv_cmd.c | 2 ++
>>>>   1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/src/responder/nss/nsssrv_cmd.c
>>>> b/src/responder/nss/nsssrv_cmd.c
>>>> index
>>>> d37b4707cb734375011650632bca6d429042038c..a1938b2fc8d0a88460027d5f0d400f840fade02b 100644
>>>> --- a/src/responder/nss/nsssrv_cmd.c
>>>> +++ b/src/responder/nss/nsssrv_cmd.c
>>>> @@ -320,6 +320,7 @@ static int fill_pwent(struct sss_packet *packet,
>>>>       for (i = 0; i < *count; i++) {
>>>>           talloc_zfree(tmp_ctx);
>>>>           tmp_ctx = talloc_new(NULL);
>>>> +        if (tmp_ctx == NULL) return ENOMEM;
>>>>
>>>>           msg = msgs[i];
>>>>
>>>> @@ -2325,6 +2326,7 @@ static int fill_grent(struct sss_packet *packet,
>>>>       for (i = 0; i < *count; i++) {
>>>>           talloc_zfree(tmp_ctx);
>>>>           tmp_ctx = talloc_new(NULL);
>>>> +        if (tmp_ctx == NULL) return ENOMEM;
>>>>           msg = msgs[i];
>>>>
>>>>           /* new group */
>>>
>>> Sorry to shot down another small fix, but I think what you want to do
>>> here is not talloc_zfree(tmp_ctx) + talloc_new(NULL), but rather use
>>> simply talloc_free_children(tmp_ctx);
>>>
>>> Simo.
>>
>> OK, but that wasn't the point of the patch :-) The point was to check
>> result of allocation, which was missing.
>>
>> I will prepare a new patch.
>
> New patch is attached -- talloc_free_children is used now instead of
> talloc_free() && talloc_new().
>
> The fill_* functions return gracefully instead of erroring out now
> (although I don't think this makes too much of a difference, if sssd
> runs out of memory, it should just care about not crashing and ending the
> request somehow).

Hi,
sorry but nack. We should return ENOMEM in fill_* to stay consistent 
with rest of the code.

These functions already return errno so I really don't get why you chose 
to do it this way :-)

> Also nss_update_initgr_memcache is now checked.




More information about the sssd-devel mailing list