[SSSD] [PATCHES] dyndns: Fix talloc hierarchy of "struct sss_iface_addr"
Lukas Slebodnik
lslebodn at redhat.com
Thu Aug 21 18:14:50 UTC 2014
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
LS
More information about the sssd-devel
mailing list