On (02/09/14 20:29), Nikolai Kondrashov wrote:
On 09/02/2014 04:38 PM, Lukas Slebodnik wrote:
>On (02/09/14 14:23), Nikolai Kondrashov wrote:
>>On 09/02/2014 02:13 PM, Lukas Slebodnik wrote:
>>>On (02/09/14 13:27), Nikolai Kondrashov wrote:
>>>>On 09/02/2014 01:02 PM, Lukas Slebodnik wrote:
>>>>>We use talloc and valgrind needn't find memory leaks.
>>>>>For this purpose, we have functions in file src/tests/leak_check.c
>>>>
>>>BTW
>>>For tesing purposes, I reverted commit
d528e4960bf233bc002f9676919ede196c91564c
>>
>>Ah, this one is a good source of a failure.
>>
>>>>Thanks Lukas, this seems to work fine with the exception of this one
error
>>>>in refcount-tests:
>>>>
>>>>==2842== Syscall param timer_create(evp) points to uninitialised byte(s)
>>>>==2842== at 0x977FE72: timer_create@(a)GLIBC_2.3.3 (timer_create.c:82)
>>>>==2842== by 0x4E3647B: srunner_run (in /usr/lib64/libcheck.so.0.0.0)
>>>>==2842== by 0x401616: main (refcount-tests.c:233)
>>>>==2842== Address 0xffefff460 is on thread 1's stack
>>>>
>Patch for this issue is on the list:
> "[PATCH] refcount-tests: Do not force to run test in CK_FORK mode"
>
>I updated also readme file and new version is attached.
>
>This patch is blocked by ""[PATCH] refcount-tests: ..."
Thank you Lukas. However, this time Valgrind found some issues with
sysdb-tests on RHEL6 (with refcount-tests fixed):
==13371== Conditional jump or move depends on uninitialised value(s)
==13371== at 0x907C968: ??? (in /lib64/libc-2.12.so)
==13371== by 0x70E6527: sha512_crypt_r (nss_sha512crypt.c:288)
==13371== by 0x70E67F7: s3crypt_sha512 (nss_sha512crypt.c:345)
==13371== by 0x504FF32: sysdb_cache_auth (sysdb_ops.c:3269)
==13371== by 0x40946E: cached_authentication_without_expiration (sysdb-tests.c:1742)
==13371== by 0x4096D2: test_sysdb_cached_authentication_wrong_password
(sysdb-tests.c:1853)
==13371== by 0x4E3387D: srunner_run_all (in /usr/lib64/libcheck.so.0.0.0)
==13371== by 0x405758: main (sysdb-tests.c:6103)
I reproduced them twice:
http://sssd-ci.duckdns.org/logs/job/0/32/rhel6/ci-build-debug/sysdb-tests...
http://sssd-ci.duckdns.org/logs/job/0/33/rhel6/ci-build-debug/sysdb-tests...
The problematic is function stpncpy.
A) works for me on rhel6 i386
B) warnings are on line nss_sha512crypt.c:288, but salt is used also on another
places
src/util/crypto/nss/nss_sha512crypt.c:126: salt = copied_salt = memcpy(tmp +
ALIGN64 - PTR_2_INT(tmp) % ALIGN64, salt, salt_len);
src/util/crypto/nss/nss_sha512crypt.c:156: HASH_Update(ctx, (const unsigned char
*)salt, salt_len);
src/util/crypto/nss/nss_sha512crypt.c:167: HASH_Update(alt_ctx, (const unsigned char
*)salt, salt_len);
src/util/crypto/nss/nss_sha512crypt.c:217: HASH_Update(alt_ctx, (const unsigned
char *)salt, salt_len);
src/util/crypto/nss/nss_sha512crypt.c:244: HASH_Update(ctx, (const unsigned
char *)s_bytes, salt_len);
src/util/crypto/nss/nss_sha512crypt.c:288: cp = stpncpy(cp, salt, salt_len);
C) works with stpncpy equivalent
--- a/src/util/crypto/nss/nss_sha512crypt.c
+++ b/src/util/crypto/nss/nss_sha512crypt.c
@@ -285,7 +285,8 @@ static int sha512_crypt_r(const char *key,
ret = ERANGE;
goto done;
}
- cp = stpncpy(cp, salt, salt_len);
+ cp = strncpy(cp, salt, salt_len);
+ cp += salt_len;
*cp++ = '$';
buflen -= salt_len + 1;
D) works with different version stpncpy in libc which is not in header files.
sh# nm --dynamic --defined-only /lib64/libc.so.6 | grep stpncpy
0000003194484c60 i __stpncpy
00000031944ff860 T __stpncpy_chk
0000003194484c60 i stpncpy
--- a/src/util/crypto/libcrypto/crypto_sha512crypt.c
+++ b/src/util/crypto/libcrypto/crypto_sha512crypt.c
@@ -67,6 +67,8 @@ static inline void b64_from_24bit(char **dest, size_t *len, size_t
n,
#define PTR_2_INT(x) ((x) - ((__typeof__ (x)) NULL))
#define ALIGN64 __alignof__(uint64_t)
+char *__stpncpy_chk(char *dest, const char *src, size_t n);
+
static int sha512_crypt_r(const char *key,
const char *salt,
char *buffer, size_t buflen)
@@ -284,7 +286,7 @@ static int sha512_crypt_r(const char *key,
ret = ERANGE;
goto done;
}
- cp = stpncpy(cp, salt, salt_len);
+ cp = __stpncpy_chk(cp, salt, salt_len);
*cp++ = '$';
buflen -= salt_len + 1;
Summary:
It is either bug in glibc or in valgrind.
I filed a ticket:
https://fedorahosted.org/sssd/ticket/2428
LS