[SSSD] [PATCH] NSS: Check allocation result

Jakub Hrozek jhrozek at redhat.com
Thu Oct 31 18:51:20 UTC 2013


On Thu, Oct 31, 2013 at 10:37:13AM +0100, Pavel Březina wrote:
> 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 :-)

That's exactly the opposite of what Sumit requested earlier. Please make
up your mind :)



More information about the sssd-devel mailing list