search

aws / s2n-tls

An implementation of the TLS/SSL protocols
https://aws.github.io/s2n-tls/usage-guide/
Apache License 2.0
4.53k stars 707 forks source link

Possible Valgrind false negatives #1533

Open lrstewart opened 4 years ago

lrstewart commented 4 years ago

Problem:

In our self-talk tests, we malloc before we fork and only free in the parent process. Example. Valgrind SHOULD yell at us about this, but it doesn't.

However, when I made an unrelated change to one of the self-talk tests, it started failing due to those mallocs. When I moved the previously-malloced cert objects to the stack, valgrind passed.

I don't know why Valgrind was passing before, and I don't know why my changes caused it to start detecting the leak.

Proposed Solution:

We need to investigate whether Valgrind is actually detecting leaks, especially in our self-talk tests. We could start with a minimal process forking test that intentionally fails to clean up memory.

lrstewart commented 4 years ago

Since we have Valgrind set up to return "9" from processes that fail its checks, we could actually write a unit test to verify that Valgrind catches what it should by writing processes with intentional leaks.

lrstewart commented 4 years ago

We have one valgrind suppression, but it doesn't look relevant to me: https://github.com/awslabs/s2n/blob/master/tests/unit/valgrind.suppressions

jmayclin commented 9 months ago

This issue is related to our "pedantic" valgrind testing. We can now detect these leaks since we have implemented pedantic testing for aws-lc and openssl 1.1.1. Tracking issue for full implementation: #3758