Skip to content

Commit f67e605

Browse files
authored
Explicit smart pointer conversion (#954)
* Explicit conversion of smart pointers * Test w/ Valgrind * More suppression
1 parent ee3917f commit f67e605

File tree

29 files changed

+527
-229
lines changed

29 files changed

+527
-229
lines changed

.github/workflows/analysis.yml

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -360,6 +360,32 @@ jobs:
360360
exit 0
361361
fi
362362
363+
valgrind:
364+
if: github.repository_owner == 'aws'
365+
name: Valgrind Memory Check
366+
runs-on: ubuntu-latest
367+
env:
368+
AWS_LC_RS_DISABLE_SLOW_TESTS: 1
369+
steps:
370+
- uses: actions/checkout@v3
371+
with:
372+
submodules: 'recursive'
373+
- uses: dtolnay/rust-toolchain@stable
374+
- name: Install Valgrind
375+
run: sudo apt-get update && sudo apt-get install -y valgrind
376+
- name: Run debug tests under Valgrind
377+
run: |
378+
CARGO_TARGET_X86_64_UNKNOWN_LINUX_GNU_RUNNER="valgrind --error-exitcode=1 --leak-check=full --show-leak-kinds=all --suppressions=$(pwd)/.valgrind/rust-test.supp" \
379+
cargo test -p aws-lc-rs --features unstable --all-targets -- --test-threads=1
380+
- name: Run release tests under Valgrind
381+
run: |
382+
CARGO_TARGET_X86_64_UNKNOWN_LINUX_GNU_RUNNER="valgrind --error-exitcode=1 --leak-check=full --show-leak-kinds=all --suppressions=$(pwd)/.valgrind/rust-test.supp" \
383+
cargo test --release -p aws-lc-rs --features unstable --all-targets -- --test-threads=1
384+
- name: Run FIPS release tests under Valgrind
385+
run: |
386+
CARGO_TARGET_X86_64_UNKNOWN_LINUX_GNU_RUNNER="valgrind --error-exitcode=1 --leak-check=full --show-leak-kinds=all --suppressions=$(pwd)/.valgrind/rust-test.supp" \
387+
cargo test --release -p aws-lc-rs --features fips --all-targets -- --test-threads=1
388+
363389
gnu-stack-check:
364390
if: github.repository_owner == 'aws'
365391
name: Verify (${{ matrix.host }}) GNU-stack section

.valgrind/rust-test.supp

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
# Valgrind suppression file for Rust test harness
2+
3+
{
4+
rust_test_invocations
5+
Memcheck:Leak
6+
match-leak-kinds: possible
7+
...
8+
fun:call_once<fn() -> core::cell::Cell<core::option::Option<std::sync::mpmc::context::Context>>, ()>
9+
...
10+
fun:run_tests<*>
11+
...
12+
}
13+
14+
# AWS-LC ML-DSA Implementation Issues
15+
# TODO: Remove once fixed upstream in AWS-LC
16+
{
17+
aws_lc_ml_dsa_verify_uninit
18+
Memcheck:Cond
19+
fun:*ml_dsa_verify_internal*
20+
}
21+
22+
{
23+
aws_lc_ml_dsa_verify_uninit_value
24+
Memcheck:Value8
25+
fun:*ml_dsa_verify_internal*
26+
}
27+
28+
{
29+
aws_lc_rs_test_possible_leak
30+
Memcheck:Leak
31+
match-leak-kinds: possible
32+
fun:malloc
33+
obj:*/*_test-*
34+
...
35+
fun:__libc_start_main@@GLIBC_*
36+
}
37+
38+
{
39+
rust_thread_new_leak
40+
Memcheck:Leak
41+
match-leak-kinds: possible
42+
fun:malloc
43+
fun:*thread*Thread*new*
44+
...
45+
}

aws-lc-rs/scripts/run-valgrind.sh

Lines changed: 205 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,205 @@
1+
#!/usr/bin/env bash
2+
# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
# SPDX-License-Identifier: Apache-2.0 OR ISC
4+
5+
# # Helper script for running aws-lc-rs tests under Valgrind
6+
#
7+
# Usage:
8+
# ./scripts/run-valgrind.sh [OPTIONS] [TEST_NAME]
9+
#
10+
# Examples:
11+
# ./scripts/run-valgrind.sh # Run all tests
12+
# ./scripts/run-valgrind.sh pqdsa_test # Run specific test
13+
# ./scripts/run-valgrind.sh --no-suppress # Run without suppressions
14+
# ./scripts/run-valgrind.sh --release # Run release build
15+
16+
set -e
17+
18+
# Colors for output
19+
RED='\033[0;31m'
20+
GREEN='\033[0;32m'
21+
YELLOW='\033[1;33m'
22+
BLUE='\033[0;34m'
23+
NC='\033[0m' # No Color
24+
25+
# Default configuration
26+
USE_SUPPRESSIONS=1
27+
BUILD_MODE="debug"
28+
LEAK_CHECK="full"
29+
SHOW_LEAK_KINDS="all"
30+
ERROR_EXITCODE=1
31+
TEST_THREADS=1
32+
FEATURES="unstable"
33+
PACKAGE="aws-lc-rs"
34+
VALGRIND_EXTRA_ARGS=""
35+
GEN_SUPPRESSIONS=0
36+
export AWS_LC_RS_DISABLE_SLOW_TESTS=1
37+
38+
# Parse command line arguments
39+
while [[ $# -gt 0 ]]; do
40+
case $1 in
41+
--no-suppress)
42+
USE_SUPPRESSIONS=0
43+
shift
44+
;;
45+
--gen-suppressions)
46+
GEN_SUPPRESSIONS=1
47+
shift
48+
;;
49+
--release)
50+
BUILD_MODE="release"
51+
shift
52+
;;
53+
--debug)
54+
BUILD_MODE="debug"
55+
shift
56+
;;
57+
--threads)
58+
TEST_THREADS="$2"
59+
shift 2
60+
;;
61+
--features)
62+
FEATURES="$2"
63+
shift 2
64+
;;
65+
--package|-p)
66+
PACKAGE="$2"
67+
shift 2
68+
;;
69+
--help|-h)
70+
echo "Usage: $0 [OPTIONS] [TEST_NAME]"
71+
echo ""
72+
echo "Options:"
73+
echo " --no-suppress Disable Valgrind suppressions (show all warnings)"
74+
echo " --gen-suppressions Generate suppression rules for errors found"
75+
echo " --release Use release build (faster but less debug info)"
76+
echo " --debug Use debug build (default)"
77+
echo " --threads N Number of test threads (default: 1)"
78+
echo " --features FEATS Cargo features to enable (default: unstable)"
79+
echo " --package PKG Package to test (default: aws-lc-rs)"
80+
echo " --help, -h Show this help message"
81+
echo ""
82+
echo "Examples:"
83+
echo " $0 # Run all tests"
84+
echo " $0 pqdsa_test # Run specific test"
85+
echo " $0 --no-suppress # Run without suppressions"
86+
echo " $0 --gen-suppressions # Generate suppression rules"
87+
echo " $0 --release pqdsa_test # Run specific test in release mode"
88+
exit 0
89+
;;
90+
--*)
91+
echo -e "${RED}Error: Unknown option $1${NC}"
92+
exit 1
93+
;;
94+
*)
95+
# Assume it's a test name
96+
TEST_NAME="$1"
97+
shift
98+
;;
99+
esac
100+
done
101+
102+
# Get the repository root directory
103+
REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)"
104+
cd "$REPO_ROOT/aws-lc-rs"
105+
106+
# Check if Valgrind is installed
107+
if ! command -v valgrind &> /dev/null; then
108+
echo -e "${RED}Error: Valgrind is not installed${NC}"
109+
echo "Install it with:"
110+
echo " Ubuntu/Debian: sudo apt-get install valgrind"
111+
echo " macOS: brew install valgrind"
112+
exit 1
113+
fi
114+
115+
# Build Valgrind command
116+
VALGRIND_CMD="valgrind --error-exitcode=${ERROR_EXITCODE} --leak-check=${LEAK_CHECK} --show-leak-kinds=${SHOW_LEAK_KINDS}"
117+
118+
# Add gen-suppressions if enabled
119+
if [ $GEN_SUPPRESSIONS -eq 1 ]; then
120+
VALGRIND_CMD="${VALGRIND_CMD} --gen-suppressions=all"
121+
echo -e "${BLUE}Generating suppression rules for all errors${NC}"
122+
# Disable error exit code when generating suppressions to see all issues
123+
ERROR_EXITCODE=0
124+
fi
125+
126+
# Add suppression file if enabled
127+
if [ $USE_SUPPRESSIONS -eq 1 ]; then
128+
SUPPRESSION_FILE="${REPO_ROOT}/.valgrind/rust-test.supp"
129+
if [ -f "$SUPPRESSION_FILE" ]; then
130+
VALGRIND_CMD="${VALGRIND_CMD} --suppressions=${SUPPRESSION_FILE}"
131+
echo -e "${BLUE}Using suppressions from: ${SUPPRESSION_FILE}${NC}"
132+
else
133+
echo -e "${YELLOW}Warning: Suppression file not found: ${SUPPRESSION_FILE}${NC}"
134+
fi
135+
else
136+
echo -e "${YELLOW}Running WITHOUT suppressions - expect false positives${NC}"
137+
fi
138+
139+
# Add any extra Valgrind arguments
140+
if [ -n "$VALGRIND_EXTRA_ARGS" ]; then
141+
VALGRIND_CMD="${VALGRIND_CMD} ${VALGRIND_EXTRA_ARGS}"
142+
fi
143+
144+
# Build cargo command
145+
CARGO_CMD="cargo test -p ${PACKAGE} --features ${FEATURES}"
146+
147+
if [ "$BUILD_MODE" = "release" ]; then
148+
CARGO_CMD="${CARGO_CMD} --release"
149+
echo -e "${BLUE}Using release build${NC}"
150+
else
151+
echo -e "${BLUE}Using debug build${NC}"
152+
fi
153+
154+
# Add test name if provided
155+
if [ -n "$TEST_NAME" ]; then
156+
CARGO_CMD="${CARGO_CMD} --test ${TEST_NAME}"
157+
echo -e "${BLUE}Running test: ${TEST_NAME}${NC}"
158+
else
159+
echo -e "${BLUE}Running all tests${NC}"
160+
fi
161+
162+
# Add test arguments
163+
CARGO_CMD="${CARGO_CMD} -- --test-threads=${TEST_THREADS}"
164+
165+
# Print configuration
166+
echo -e "${GREEN}=== Valgrind Test Configuration ===${NC}"
167+
echo "Package: ${PACKAGE}"
168+
echo "Features: ${FEATURES}"
169+
echo "Build: ${BUILD_MODE}"
170+
echo "Test threads: ${TEST_THREADS}"
171+
echo "Suppressions: $([ $USE_SUPPRESSIONS -eq 1 ] && echo 'enabled' || echo 'disabled')"
172+
echo "Generate suppressions: $([ $GEN_SUPPRESSIONS -eq 1 ] && echo 'enabled' || echo 'disabled')"
173+
echo ""
174+
175+
# Export environment variables
176+
export CARGO_TARGET_X86_64_UNKNOWN_LINUX_GNU_RUNNER="${VALGRIND_CMD}"
177+
export AWS_LC_RS_DISABLE_SLOW_TESTS=1
178+
179+
echo -e "${GREEN}=== Starting Valgrind Test Run ===${NC}"
180+
echo "Command: ${CARGO_CMD}"
181+
echo ""
182+
183+
# Run the tests
184+
if eval ${CARGO_CMD}; then
185+
echo ""
186+
echo -e "${GREEN}=== Valgrind tests PASSED ===${NC}"
187+
exit 0
188+
else
189+
EXIT_CODE=$?
190+
echo ""
191+
echo -e "${RED}=== Valgrind tests FAILED ===${NC}"
192+
echo ""
193+
echo "Possible causes:"
194+
echo " 1. Memory leak detected (check output above)"
195+
echo " 2. Uninitialized memory usage"
196+
echo " 3. Invalid memory access"
197+
echo ""
198+
echo "Next steps:"
199+
echo " - Review the Valgrind output above"
200+
echo " - Check .valgrind/KNOWN_ISSUES.md for known issues"
201+
echo " - Run with --no-suppress to see all warnings"
202+
echo " - Run with --gen-suppressions to generate suppression rules"
203+
echo " - For false positives in stdlib, add to .valgrind/rust-test.supp"
204+
exit $EXIT_CODE
205+
fi

aws-lc-rs/src/aead/aead_ctx.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -245,12 +245,12 @@ impl AeadCtx {
245245
let mut aead_ctx: LcPtr<EVP_AEAD_CTX> =
246246
LcPtr::new(unsafe { OPENSSL_malloc(size_of::<EVP_AEAD_CTX>()) }.cast())?;
247247

248-
unsafe { EVP_AEAD_CTX_zero(*aead_ctx.as_mut()) };
248+
unsafe { EVP_AEAD_CTX_zero(aead_ctx.as_mut_ptr()) };
249249

250250
if 1 != match direction {
251251
Some(direction) => unsafe {
252252
EVP_AEAD_CTX_init_with_direction(
253-
*aead_ctx.as_mut(),
253+
aead_ctx.as_mut_ptr(),
254254
aead,
255255
key_bytes.as_ptr(),
256256
key_bytes.len(),
@@ -260,7 +260,7 @@ impl AeadCtx {
260260
},
261261
None => unsafe {
262262
EVP_AEAD_CTX_init(
263-
*aead_ctx.as_mut(),
263+
aead_ctx.as_mut_ptr(),
264264
aead,
265265
key_bytes.as_ptr(),
266266
key_bytes.len(),

aws-lc-rs/src/aead/unbound_key.rs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ impl UnboundKey {
105105
let nonce = nonce.as_ref();
106106

107107
if 1 != EVP_AEAD_CTX_open_gather(
108-
*aead_ctx.as_const(),
108+
aead_ctx.as_const_ptr(),
109109
out_plaintext.as_mut_ptr(),
110110
nonce.as_ptr(),
111111
nonce.len(),
@@ -179,7 +179,7 @@ impl UnboundKey {
179179

180180
if 1 != unsafe {
181181
EVP_AEAD_CTX_seal_scatter(
182-
*self.ctx.as_ref().as_const(),
182+
self.ctx.as_ref().as_const_ptr(),
183183
in_out.as_mut_ptr(),
184184
extra_out_and_tag.as_mut_ptr(),
185185
&mut out_tag_len,
@@ -231,7 +231,7 @@ impl UnboundKey {
231231
let mut out_len = MaybeUninit::<usize>::uninit();
232232
if 1 != indicator_check!(unsafe {
233233
EVP_AEAD_CTX_open(
234-
*self.ctx.as_ref().as_const(),
234+
self.ctx.as_ref().as_const_ptr(),
235235
in_out.as_mut_ptr(),
236236
out_len.as_mut_ptr(),
237237
plaintext_len,
@@ -277,7 +277,7 @@ impl UnboundKey {
277277

278278
if 1 != indicator_check!(unsafe {
279279
EVP_AEAD_CTX_open_gather(
280-
*self.ctx.as_ref().as_const(),
280+
self.ctx.as_ref().as_const_ptr(),
281281
in_out.as_mut_ptr(),
282282
null(),
283283
0,
@@ -325,7 +325,7 @@ impl UnboundKey {
325325

326326
if 1 != indicator_check!(unsafe {
327327
EVP_AEAD_CTX_seal(
328-
*self.ctx.as_ref().as_const(),
328+
self.ctx.as_ref().as_const_ptr(),
329329
mut_in_out.as_mut_ptr(),
330330
out_len.as_mut_ptr(),
331331
plaintext_len + alg_tag_len,
@@ -363,7 +363,7 @@ impl UnboundKey {
363363

364364
if 1 != indicator_check!(unsafe {
365365
EVP_AEAD_CTX_seal_scatter(
366-
*self.ctx.as_ref().as_const(),
366+
self.ctx.as_ref().as_const_ptr(),
367367
in_out.as_mut_ptr(),
368368
tag_buffer.as_mut_ptr(),
369369
out_tag_len.as_mut_ptr(),
@@ -410,7 +410,7 @@ impl UnboundKey {
410410

411411
if 1 != indicator_check!(unsafe {
412412
EVP_AEAD_CTX_seal_scatter(
413-
*self.ctx.as_ref().as_const(),
413+
self.ctx.as_ref().as_const_ptr(),
414414
in_out.as_mut_ptr(),
415415
tag.as_mut_ptr(),
416416
out_tag_len.as_mut_ptr(),
@@ -447,7 +447,7 @@ impl UnboundKey {
447447

448448
if 1 != indicator_check!(unsafe {
449449
EVP_AEAD_CTX_seal_scatter(
450-
*self.ctx.as_ref().as_const(),
450+
self.ctx.as_ref().as_const_ptr(),
451451
in_out.as_mut_ptr(),
452452
tag_buffer.as_mut_ptr(),
453453
out_tag_len.as_mut_ptr(),

0 commit comments

Comments
 (0)