[SSSD] [PATCH] NSS: Check allocation result

Jakub Hrozek jhrozek at redhat.com
Tue Oct 22 10:29:55 UTC 2013


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).

Also nss_update_initgr_memcache is now checked.
-------------- next part --------------
>From b4ffc4165dfc780cc4752df29db632178a316553 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: use talloc_free_children, check allocation result

Several places in the code didn't check for allocation failures.

Use talloc_free_children instead of talloc_free && talloc_new.
---
 src/responder/nss/nsssrv_cmd.c      | 24 ++++++++++++++++++++----
 src/responder/nss/nsssrv_services.c | 12 ++++++++----
 2 files changed, 28 insertions(+), 8 deletions(-)

diff --git a/src/responder/nss/nsssrv_cmd.c b/src/responder/nss/nsssrv_cmd.c
index d37b4707cb734375011650632bca6d429042038c..67d56f882036b1ddf49832a76355d0a563efb8f6 100644
--- a/src/responder/nss/nsssrv_cmd.c
+++ b/src/responder/nss/nsssrv_cmd.c
@@ -317,9 +317,15 @@ static int fill_pwent(struct sss_packet *packet,
     rp = 2*sizeof(uint32_t);
 
     num = 0;
+
+    tmp_ctx = talloc_new(NULL);
+    if (tmp_ctx == NULL) {
+        i = 0;
+        goto done;
+    }
+
     for (i = 0; i < *count; i++) {
-        talloc_zfree(tmp_ctx);
-        tmp_ctx = talloc_new(NULL);
+        talloc_free_children(tmp_ctx);
 
         msg = msgs[i];
 
@@ -2313,6 +2319,12 @@ static int fill_grent(struct sss_packet *packet,
 
     num = 0;
 
+    tmp_ctx = talloc_new(NULL);
+    if (tmp_ctx == NULL) {
+        i = 0;
+        goto done;
+    }
+
     /* first 2 fields (len and reserved), filled up later */
     ret = sss_packet_grow(packet, 2*sizeof(uint32_t));
     if (ret != EOK) {
@@ -2323,8 +2335,8 @@ static int fill_grent(struct sss_packet *packet,
     rsize = 0;
 
     for (i = 0; i < *count; i++) {
-        talloc_zfree(tmp_ctx);
-        tmp_ctx = talloc_new(NULL);
+        talloc_free_children(tmp_ctx);
+
         msg = msgs[i];
 
         /* new group */
@@ -3357,6 +3369,10 @@ void nss_update_initgr_memcache(struct nss_ctx *nctx,
     }
 
     tmp_ctx = talloc_new(NULL);
+    if (tmp_ctx == NULL) {
+        DEBUG(SSSDBG_CRIT_FAILURE, ("Out of memory\n"));
+        goto done;
+    }
 
     ret = sysdb_initgroups(tmp_ctx, dom->sysdb, dom, name, &res);
     if (ret != EOK && ret != ENOENT) {
diff --git a/src/responder/nss/nsssrv_services.c b/src/responder/nss/nsssrv_services.c
index 79caa7d08cdfff25112eedab98a0419f1b1d154e..92e00a723a1141d0a649ff636b2126e820bbc7cc 100644
--- a/src/responder/nss/nsssrv_services.c
+++ b/src/responder/nss/nsssrv_services.c
@@ -625,10 +625,14 @@ fill_service(struct sss_packet *packet,
     rzero = 2 * sizeof(uint32_t);
     rsize = 0;
 
+    tmp_ctx = talloc_new(NULL);
+    if (tmp_ctx == NULL) {
+        num = 0;
+        goto done;
+    }
+
     for (i = 0; i < msg_count; i++) {
-        talloc_zfree(tmp_ctx);
-        tmp_ctx = talloc_new(NULL);
-        if (!tmp_ctx) return ENOMEM;
+        talloc_free_children(tmp_ctx);
 
         msg = msgs[i];
 
@@ -762,7 +766,7 @@ fill_service(struct sss_packet *packet,
 done:
     talloc_free(tmp_ctx);
 
-    if (ret != EOK ||num == 0) {
+    if (ret != EOK || num == 0) {
         /* if num is 0 most probably something went wrong,
          * reset packet and return ENOENT */
         sss_packet_set_size(packet, 0);
-- 
1.8.3.1



More information about the sssd-devel mailing list