[SSSD] [PATCHES] dyndns: Fix talloc hierarchy of "struct sss_iface_addr"

Jakub Hrozek jhrozek at redhat.com
Fri Aug 22 08:57:07 UTC 2014


On Thu, Aug 21, 2014 at 08:14:50PM +0200, Lukas Slebodnik wrote:
> On (21/08/14 18:58), Jakub Hrozek wrote:
> >On Thu, Aug 21, 2014 at 10:25:08AM +0200, Lukas Slebodnik wrote:
> >> ehlo
> >> 
> >> 0001-dyndns_test-Use-right-socket-length-of-for-IPv4-addr.patch
> >>     not very important patch.
> >> 
> >> 0002-responder-get-domains-tests-fix-checking-of-leaks.patch
> >> 0003-test_dyndns-Use-different-talloc-context-in-wrapped-.patch
> >>     previous two patches are necessary for 4th patch
> >> 
> >> 0004-TESTS-leak_check-functions-shouldn-t-be-called-with-.patch
> >>     Wit this patch it will be possible to cach mem leak in 6th patch
> >> 
> >> 0005-dyndns-Fix-talloc-hierarchy-of-struct-sss_iface_addr.patch
> >>     this simpepatch fix use after free problem in ticket #2405
> >> 
> >> 0006-test_dyndns-sss_iface_addr_list_get-can-return-more-.patch
> >>     test for previous patch. You can try to revert 5th patch
> >>     and you will see memory leak (not all data were moved to appropriate
> >>             talloc context)
> >> 
> >> Leak report for ../sssd/src/tests/cmocka/test_dyndns.c:247:
> >> full talloc report on 'struct dyndns_test_ctx' (total     80 bytes in   3 blocks)
> >>     struct sss_iface_addr          contains     40 bytes in   2 blocks (ref 0) 0x1469c20
> >>         ../sssd/src/providers/dp_dyndns.c:210 contains     16 bytes in   1 blocks (ref 0) 0x1469990
> >> check_leaks_pop(dyndns_test_ctx) == true
> >> ../sssd/src/tests/cmocka/test_dyndns.c:247: error: Failure!
> >> [  FAILED  ] dyndns_test_get_multi_ifaddr
> >> 
> >> 0007-SDAP-free-subrequest-in-sdap_dyndns_update_addrs_don.patch
> >>     subrequest can be freed after fixing talloc hierarchy in 5th patch.
> >> 
> >> How to test.
> >> two IP addresses shoudl be used in ptr update. You can use valgrind to see use
> >> after free error.
> >> 
> >> The other option is to export env variable TALLOC_FREE_FILL=255 and sssd_be
> >> will crash in function resolv_get_string_ptr_address
> >> 
> >> LS
> >
> >> From c1dcfbf48ad07d3ac34f2085272e64666a0b4239 Mon Sep 17 00:00:00 2001
> >> From: Lukas Slebodnik <lslebodn at redhat.com>
> >> Date: Wed, 20 Aug 2014 17:32:04 +0200
> >> Subject: [PATCH 1/7] dyndns_test: Use right socket length of for IPv4 address.
> >> 
> >> man inet_ntop says:
> >>   The caller specifies the number of bytes available in this buffer
> >>   in the argument size.
> >> 
> >>    AF_INET
> >>           src points to a struct in_addr (in network byte order) which is
> >>           converted to an IPv4 network address in the dotted-decimal
> >>           format, "ddd.ddd.ddd.ddd". The buffer dst must be at least
> >>           INET_ADDRSTRLEN bytes long.
> >> ---
> >>  src/tests/cmocka/test_dyndns.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >> 
> >> diff --git a/src/tests/cmocka/test_dyndns.c b/src/tests/cmocka/test_dyndns.c
> >> index b0e8df81bc079efa8ebd8ae45603921261330c9b..b7e45a19db5dd9e85260029fe81e46aa18c1c8ed 100644
> >> --- a/src/tests/cmocka/test_dyndns.c
> >> +++ b/src/tests/cmocka/test_dyndns.c
> >> @@ -194,7 +194,7 @@ void dyndns_test_get_ifaddr(void **state)
> >>  
> >>      assert_non_null(inet_ntop(AF_INET,
> >>                                &((struct sockaddr_in *) addrlist->addr)->sin_addr,
> >> -                              straddr, INET6_ADDRSTRLEN));
> >> +                              straddr, INET_ADDRSTRLEN));
> >
> >Does it make sense to also use INET_ADDRSTRLEN+1 for the straddr length,
> >too?
> >
> maximul length of IPV4 address is 15 "ddd.ddd.ddd.ddd"
> 
> sh-4.3$ grep -RniI INET_ADDRSTRLEN
> netinet/in.h:232:#define INET_ADDRSTRLEN 16
> 
> BTW man inet_ntop says:
>      The caller specifies the number of bytes available in this buffer
>      in the argument size.                    ^^^^^^^^^
> 
>      The buffer dst must be at least INET_ADDRSTRLEN bytes long.
>                             ^^^^^^^^
> In teory, this patch is not needed, because constant INET6_ADDRSTRLEN is longer
> than INET_ADDRSTRLEN

OK, it doesn't hurt either.

master:
    f55d45b931ce6c01e005ae94a69e93abda0d2f1c
    1a783fb0be9a48a0abdfe8b52fce551d530487ce
    0060992d68ba843d4d90b491a1500b6290789a5c
    24000ed5b08499b49595436b8a3b348fcd4012de
    12e7e87ccbae0d5c2f338cd019ca51556cbcd3ae
    bf65fbdd8c3fecd38a66363c3517e7a2679b8186
    e210ed5da220acebb6751db4466fe352de08eaeb
sssd-1-11:
    5446f2a749d3e641b3ffc9feb3240a9b0f4b0598
    79d007fa5776849ab556571faef411b478c6970d
    03cfd27286a77fc991fca7ba68dee36084cfc2d9
    103f2f305ba073f9e9399cac10eefd3685bb291c
    d111a01626bfe1ec468a2f6e01e6a237286481d8
    5b8ed5702b38c2bd39e9c9a44d94ad7dac3040a1
    0789077faa81113e3e6ef46f71bde878d8c58023



More information about the sssd-devel mailing list