Fix an invalid memory access [release-7.1] by jzhou77 · Pull Request #10951 · apple/foundationdb

jzhou77 · 2023-10-02T17:02:50Z

cherrypick #10950 #10763 #10960

Fixed two use-after-free bugs.

Code-Reviewer Section

The general pull request guidelines can be found here.

Please check each of the following things and check all boxes before accepting a PR.

The PR has a description, explaining both the problem and the solution.
The description mentions which forms of testing were done and the testing seems reasonable.
Every function/class/actor that was touched is reasonably well documented.

For Release-Branches

If this PR is made against a release-branch, please also check the following:

This change/bugfix is a cherry-pick from the next younger branch (younger release-branch or main if this is the youngest branch)
There is a good reason why this PR needs to go into a release branch and this reason is documented (either in the description above or in a linked GitHub issue)

foundationdb-ci · 2023-10-02T17:43:15Z

Result of foundationdb-pr-cluster-tests on Linux CentOS 7

Commit ID: bedad6e
Duration 0:40:13
Result: ❌ FAILED
Error: Error while executing command: ninja -v -C build_output -j ${NPROC} all packages strip_targets. Reason: exit status 1
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)
Cluster Test Logs zip file of the test logs (available for 30 days)

foundationdb-ci · 2023-10-02T17:53:48Z

Result of foundationdb-pr-clang on Linux CentOS 7

Commit ID: bedad6e
Duration 0:50:48
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2023-10-02T18:14:18Z

Result of foundationdb-pr on Linux CentOS 7

Commit ID: bedad6e
Duration 1:11:18
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2023-10-02T21:03:54Z

Result of foundationdb-pr-clang on Linux CentOS 7

Commit ID: 969f371
Duration 0:56:12
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2023-10-02T21:23:20Z

Result of foundationdb-pr on Linux CentOS 7

Commit ID: 969f371
Duration 1:15:40
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

Found by Valgrind

foundationdb-ci · 2023-10-02T21:46:27Z

Result of foundationdb-pr-macos-m1 on macOS Ventura 13.x

Commit ID: 536c6ac
Duration 0:15:20
Result: ❌ FAILED
Error: Error while executing command: ssh -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -i ${HOME}/.ssh_key ec2-user@${MAC_EC2_HOST} /opt/homebrew/bin/bash --login -c ./build_pr_macos.sh. Reason: exit status 1
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2023-10-02T21:52:18Z

Result of foundationdb-pr-macos on macOS Ventura 13.x

Commit ID: 536c6ac
Duration 0:21:13
Result: ❌ FAILED
Error: Error while executing command: ssh -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -i ${HOME}/.ssh_key ec2-user@${MAC_EC2_HOST} /usr/local/bin/bash --login -c ./build_pr_macos.sh. Reason: exit status 1
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2023-10-02T22:12:15Z

Result of foundationdb-pr-cluster-tests on Linux CentOS 7

Commit ID: 969f371
Duration 2:04:30
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)
Cluster Test Logs zip file of the test logs (available for 30 days)

foundationdb-ci · 2023-10-02T22:28:25Z

Result of foundationdb-pr-clang on Linux CentOS 7

Commit ID: 536c6ac
Duration 0:57:15
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2023-10-02T22:46:17Z

Result of foundationdb-pr on Linux CentOS 7

Commit ID: 536c6ac
Duration 1:15:10
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2023-10-02T23:34:22Z

Result of foundationdb-pr-cluster-tests on Linux CentOS 7

Commit ID: 536c6ac
Duration 2:03:17
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)
Cluster Test Logs zip file of the test logs (available for 30 days)

foundationdb-ci · 2023-10-03T19:50:56Z

Result of foundationdb-pr-macos-m1 on macOS Ventura 13.x

Commit ID: a693f25
Duration 0:15:52
Result: ❌ FAILED
Error: Error while executing command: ssh -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -i ${HOME}/.ssh_key ec2-user@${MAC_EC2_HOST} /opt/homebrew/bin/bash --login -c ./build_pr_macos.sh. Reason: exit status 1
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2023-10-03T19:57:07Z

Result of foundationdb-pr-macos on macOS Ventura 13.x

Commit ID: a693f25
Duration 0:22:04
Result: ❌ FAILED
Error: Error while executing command: ssh -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -i ${HOME}/.ssh_key ec2-user@${MAC_EC2_HOST} /usr/local/bin/bash --login -c ./build_pr_macos.sh. Reason: exit status 1
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

sbodagala · 2023-10-03T20:02:45Z

+			char* valueEnd = nullptr;
+			std::string s = value.toString();
+			int ivalue = strtol(s.c_str(), &valueEnd, 10);
+			if (*valueEnd || (ivalue == 0 && s != "0"))


Probably this change has been reviewed already, but according to "strtol" documentation "(ivalue == 0)" itself indicates no valid conversion could be found so why further checking for (s != "0")?

"0" is a valid string itself

Makes sense.

Not sure what happened to my other comment here: why are we not checking for "(ivalue == LONG_MAX)" (or LONG_MIN)? And, why "ivalue" is not of type "long int"?

Good point. I fixed return type and added checking.

foundationdb-ci · 2023-10-03T20:33:24Z

Result of foundationdb-pr-clang on Linux CentOS 7

Commit ID: a693f25
Duration 0:58:20
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2023-10-03T20:44:35Z

Result of foundationdb-pr on Linux CentOS 7

Commit ID: a693f25
Duration 1:09:31
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2023-10-03T21:38:20Z

Result of foundationdb-pr-cluster-tests on Linux CentOS 7

Commit ID: a693f25
Duration 2:02:22
Result: ❌ FAILED
Error: Error while executing command: if $fail_test; then exit 1; fi. Reason: exit status 1
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)
Cluster Test Logs zip file of the test logs (available for 30 days)

Added an overflow test as well.

foundationdb-ci · 2023-10-03T23:23:27Z

Result of foundationdb-pr-macos-m1 on macOS Ventura 13.x

Commit ID: bae47e0
Duration 0:16:06
Result: ❌ FAILED
Error: Error while executing command: ssh -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -i ${HOME}/.ssh_key ec2-user@${MAC_EC2_HOST} /opt/homebrew/bin/bash --login -c ./build_pr_macos.sh. Reason: exit status 1
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2023-10-03T23:28:54Z

Result of foundationdb-pr-macos on macOS Ventura 13.x

Commit ID: bae47e0
Duration 0:21:33
Result: ❌ FAILED
Error: Error while executing command: ssh -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -i ${HOME}/.ssh_key ec2-user@${MAC_EC2_HOST} /usr/local/bin/bash --login -c ./build_pr_macos.sh. Reason: exit status 1
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2023-10-04T00:01:34Z

Result of foundationdb-pr-clang on Linux CentOS 7

Commit ID: bae47e0
Duration 0:54:09
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2023-10-04T00:17:40Z

Result of foundationdb-pr on Linux CentOS 7

Commit ID: bae47e0
Duration 1:10:15
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2023-10-04T00:55:55Z

Result of foundationdb-pr-cluster-tests on Linux CentOS 7

Commit ID: bae47e0
Duration 1:48:32
Result: ❌ FAILED
Error: Error while executing command: if $fail_test; then exit 1; fi. Reason: exit status 1
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)
Cluster Test Logs zip file of the test logs (available for 30 days)

jzhou77 force-pushed the release-7.1 branch from bedad6e to 969f371 Compare October 2, 2023 20:07

Fix an invalid memory access

536c6ac

Found by Valgrind

jzhou77 force-pushed the release-7.1 branch from 969f371 to 536c6ac Compare October 2, 2023 21:30

Fix guess region from s3 URL

a693f25

jzhou77 requested review from flowguru, sbodagala and xis19 October 3, 2023 19:36

flowguru previously approved these changes Oct 3, 2023

View reviewed changes

sbodagala reviewed Oct 3, 2023

View reviewed changes

Fix strtol return type and error checking

bae47e0

Added an overflow test as well.

jzhou77 dismissed flowguru’s stale review via bae47e0 October 3, 2023 23:07

jzhou77 requested review from flowguru and sbodagala October 3, 2023 23:08

flowguru approved these changes Oct 4, 2023

View reviewed changes

sbodagala approved these changes Oct 4, 2023

View reviewed changes

jzhou77 merged commit b7d1cb4 into apple:release-7.1 Oct 4, 2023

Conversation

jzhou77 commented Oct 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code-Reviewer Section

For Release-Branches

Uh oh!

foundationdb-ci commented Oct 2, 2023

Result of foundationdb-pr-cluster-tests on Linux CentOS 7

Uh oh!

foundationdb-ci commented Oct 2, 2023

Result of foundationdb-pr-clang on Linux CentOS 7

Uh oh!

foundationdb-ci commented Oct 2, 2023

Result of foundationdb-pr on Linux CentOS 7

Uh oh!

foundationdb-ci commented Oct 2, 2023

Result of foundationdb-pr-clang on Linux CentOS 7

Uh oh!

foundationdb-ci commented Oct 2, 2023

Result of foundationdb-pr on Linux CentOS 7

Uh oh!

foundationdb-ci commented Oct 2, 2023

Result of foundationdb-pr-macos-m1 on macOS Ventura 13.x

Uh oh!

foundationdb-ci commented Oct 2, 2023

Result of foundationdb-pr-macos on macOS Ventura 13.x

Uh oh!

foundationdb-ci commented Oct 2, 2023

Result of foundationdb-pr-cluster-tests on Linux CentOS 7

Uh oh!

foundationdb-ci commented Oct 2, 2023

Result of foundationdb-pr-clang on Linux CentOS 7

Uh oh!

foundationdb-ci commented Oct 2, 2023

Result of foundationdb-pr on Linux CentOS 7

Uh oh!

foundationdb-ci commented Oct 2, 2023

Result of foundationdb-pr-cluster-tests on Linux CentOS 7

Uh oh!

foundationdb-ci commented Oct 3, 2023

Result of foundationdb-pr-macos-m1 on macOS Ventura 13.x

Uh oh!

foundationdb-ci commented Oct 3, 2023

Result of foundationdb-pr-macos on macOS Ventura 13.x

Uh oh!

sbodagala Oct 3, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

flowguru Oct 3, 2023

Choose a reason for hiding this comment

Uh oh!

sbodagala Oct 3, 2023

Choose a reason for hiding this comment

Uh oh!

jzhou77 Oct 3, 2023

Choose a reason for hiding this comment

Uh oh!

foundationdb-ci commented Oct 3, 2023

Result of foundationdb-pr-clang on Linux CentOS 7

Uh oh!

foundationdb-ci commented Oct 3, 2023

Result of foundationdb-pr on Linux CentOS 7

Uh oh!

foundationdb-ci commented Oct 3, 2023

Result of foundationdb-pr-cluster-tests on Linux CentOS 7

Uh oh!

foundationdb-ci commented Oct 3, 2023

Result of foundationdb-pr-macos-m1 on macOS Ventura 13.x

Uh oh!

foundationdb-ci commented Oct 3, 2023

Result of foundationdb-pr-macos on macOS Ventura 13.x

Uh oh!

foundationdb-ci commented Oct 4, 2023

Result of foundationdb-pr-clang on Linux CentOS 7

Uh oh!

foundationdb-ci commented Oct 4, 2023

Result of foundationdb-pr on Linux CentOS 7

Uh oh!

foundationdb-ci commented Oct 4, 2023

jzhou77 commented Oct 2, 2023 •

edited

Loading

sbodagala Oct 3, 2023 •

edited

Loading