[SSSD] [PATCH] NSS: Fix service enumeration

Lukas Slebodnik lslebodn at redhat.com
Tue Oct 29 16:14:17 UTC 2013


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



More information about the sssd-devel mailing list