memleak in secp256k1_ec_pubkey_serialize

[bit-wasp]

memleak in secp256k1_ec_pubkey_serialize

[flgh]

My 2 tests are still working with your branch @ a5a8ed4 , so ok for me.

[afk11]

Err you're quite right. I'll look into this today and run tests with valgrind during travis runs. Thanks for opening this issue!

[afk11]

Can you try with this branch? https://github.com/afk11/secp256k1-php/tree/0.1.3-valgrind I've reviewed and made some modifications.

I also experimented with PHP's test suite running valgrind, but it didn't report any detected leaks. Partly may be because I can't pass extra flags to valgrind, but could you share your script with me? here, or email [email protected] (0xC740DE0BE7F08642)

[flgh]

I also ran with TESTS="-m" and didn't spot the leak.
Unfortunately the script calls some private php libs but I could reproduce the leak in secp256k1_ec_privkey_tweak_mul with:

$ctx = secp256k1_context_create(SECP256K1_CONTEXT_SIGN);

$key = pack("H*","1ab42cc412b618bdea3a599e3c9bae199ebf030895b039e9db1e30dafb12b727");
$twe = pack("H*","8bd3c9effcc25e5de72b329041f72a0067c1ea058b583b1b0d19308b06f189ba");
$result = secp256k1_ec_privkey_tweak_mul($ctx, $key, $twe);
echo $result . PHP_EOL;
echo unpack("H*", $key)[1] . PHP_EOL;

secp256k1_context_destroy($ctx);

and using:

po@g860:~/tests$ ZEND_DONT_UNLOAD_MODULES=1 USE_ZEND_ALLOC=0 valgrind --leak-check=full --show-reachable=yes --track-origins=yes /usr/bin/php ./testec2.php
==26130== Memcheck, a memory error detector
==26130== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==26130== Using Valgrind-3.12.0.SVN and LibVEX; rerun with -h for copyright info
==26130== Command: /usr/bin/php ./testec2.php
==26130==
1
70bdfa3c5f8c19ab21e3b8235677a9761a6dc2133411fcf5eba013d6afc8b62e
==26130==
==26130== HEAP SUMMARY:
==26130==     in use at exit: 114,009 bytes in 349 blocks
==26130==   total heap usage: 32,501 allocs, 32,152 frees, 3,879,884 bytes allocated
==26130==
...
==26130== 32 bytes in 1 blocks are definitely lost in loss record 23 of 51
==26130==    at 0x4C2BBAF: malloc (vg_replace_malloc.c:299)
==26130==    by 0x378D68: __zend_malloc (in /usr/bin/php7.2)
==26130==    by 0x12CE757C: zif_secp256k1_ec_privkey_tweak_mul (secp256k1.c:1027)
==26130==    by 0x450F4A: execute_ex (in /usr/bin/php7.2)
==26130==    by 0x456ABD: zend_execute (in /usr/bin/php7.2)
==26130==    by 0x3A4A62: zend_execute_scripts (in /usr/bin/php7.2)
==26130==    by 0x33E9A7: php_execute_script (in /usr/bin/php7.2)
==26130==    by 0x458E8E: ??? (in /usr/bin/php7.2)
==26130==    by 0x1F2FC1: ??? (in /usr/bin/php7.2)
==26130==    by 0x69922E0: (below main) (libc-start.c:291)
==26130==
...
==26130== LEAK SUMMARY:
==26130==    definitely lost: 32 bytes in 1 blocks
==26130==    indirectly lost: 0 bytes in 0 blocks
==26130==      possibly lost: 0 bytes in 0 blocks
==26130==    still reachable: 113,977 bytes in 348 blocks
==26130==         suppressed: 0 bytes in 0 blocks
==26130==
==26130== For counts of detected and suppressed errors, rerun with: -v
==26130== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)

[flgh]

Just tested with the 0.1.3-valgrind branch (both the original secp256k1_ec_pubkey_serialize/secp256k1_ec_privkey_tweak_add script and the secp256k1_ec_privkey_tweak_mul script) same commandline and valgrind looks happy.

[afk11]

@flgh thanks, great to hear. I've added another commit to avoid using the heap in some situations, which might be a better habit to get into. I'll open a PR with it now, but could you test and see if things look ok?

[afk11]

Awesome, I've released v0.1.4 and v0.2.2 with the fix! Thanks a lot for reporting this!