[SSSD] [PATCH] NSS: Fix service enumeration

Jakub Hrozek jhrozek at redhat.com
Tue Oct 29 16:59:12 UTC 2013


On Tue, Oct 29, 2013 at 05:14:17PM +0100, Lukas Slebodnik wrote:
> On (29/10/13 16:44), Jakub Hrozek wrote:
> >On Tue, Oct 29, 2013 at 03:24:18PM +0100, Lukas Slebodnik wrote:
> >> On (29/10/13 10:05), Jakub Hrozek wrote:
> >> >The code wrote into the middle of the packet to a space that was already
> >> >reserved and allocated but then still advanced the pointer to the
> >> >buffer.
> >> >
> >> >https://fedorahosted.org/sssd/ticket/2124
> >> 
> >> >From d533bb267ffb1447503f6af671a6d2d2db9ddd10 Mon Sep 17 00:00:00 2001
> >> >From: Jakub Hrozek <jhrozek at redhat.com>
> >> >Date: Tue, 29 Oct 2013 07:10:30 +0100
> >> >Subject: [PATCH] NSS: Fix service enumeration
> >> >
> >> >The code wrote into the middle of the packet to a space that was already
> >> >reserved and allocated but then still advanced the pointer to the buffer.
> >> >
> >> >https://fedorahosted.org/sssd/ticket/2124
> >> >---
> >> > src/responder/nss/nsssrv_services.c | 2 +-
> >> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >> >
> >> >diff --git a/src/responder/nss/nsssrv_services.c b/src/responder/nss/nsssrv_services.c
> >> >index 79caa7d08cdfff25112eedab98a0419f1b1d154e..ff588592c814dc43e30f9e57ddab230efff15df2 100644
> >> >--- a/src/responder/nss/nsssrv_services.c
> >> >+++ b/src/responder/nss/nsssrv_services.c
> >> >@@ -752,7 +752,7 @@ fill_service(struct sss_packet *packet,
> >> >             written_aliases++;
> >> >             talloc_zfree(tmpstr);
> >> >         }
> >> >-        SAFEALIGN_SET_UINT32(&body[aptr], written_aliases, &rsize);
> >> >+        SAFEALIGN_SET_UINT32(&body[aptr], written_aliases, NULL);
> >> > 
> >> >         num++;
> >> >     }
> >> >-- 
> >> >1.8.3.1
> >> >
> >> 
> >> Nice catch.
> >> We have already reserved place for variable written_aliases.
> >> 
> >> Here is code:
> >> -------------
> >> ret = sss_packet_grow(packet, 2 * sizeof(uint16_t)
> >>                        + sizeof(uint32_t)  // reserve place for variable written_aliases
> >>                        + cased_name.len
> >>                        + cased_proto.len);
> >> sss_packet_get_body(packet, &body, &blen);
> >> 
> >> /* Store the port number */
> >> SAFEALIGN_SET_UINT32(&body[rzero + rsize], (uint32_t)htons(port), &rsize);
> >> 
> >>  ... snip ...
> >> 
> >> /* We'll store the alias count here */
> >> aptr = rzero+rsize;   <<< remember offset for variable written_aliases
> >> rsize += sizeof(uint32_t);  <<< increment rsize
> >> 
> >> So it does not make sense to increment rsize twice and it was
> >> a reason why we could access behind allocated array.
> >> 
> >> Please add comment before line, where variable is written to packet,
> >> because it needn't be clear why rsize is not incremented.
> >> 
> >> I know that this patch will be backported to older branches.
> >> In this case, do we want to use new macro SAFEALIGN_SETMEM_UINT32?
> >
> >We can use it in master, but I would strongly prefer a separate patch so
> >that it's easily visible what changed.
> >
> >I added the comment.
> 
> >From ab86dc24e9d5b5594e790cf4def23121b27d7f0e Mon Sep 17 00:00:00 2001
> >From: Jakub Hrozek <jhrozek at redhat.com>
> >Date: Tue, 29 Oct 2013 07:10:30 +0100
> >Subject: [PATCH 1/2] NSS: Fix service enumeration
> >
> I tested patch with master an sssd-1-9 branch.
> 
> ACK
> 
> 
> >From e137eaf703c1e5ca89d6523d3aa51e5e7a3448a1 Mon Sep 17 00:00:00 2001
> >From: Jakub Hrozek <jhrozek at redhat.com>
> >Date: Tue, 29 Oct 2013 16:41:13 +0100
> >Subject: [PATCH 2/2] NSS: Use new safealign macros in NSS responder
> >
> ACK
> 
> LS

Thank you for the review, both patches were pushed to master and
patch #1 was pushed to sssd-1-11 



More information about the sssd-devel mailing list