Skip to content

Memory safety issue: TryFrom&[u8]> impls in device_path have incorrect lifetime annotations #1281

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
nicholasbishop opened this issue Aug 1, 2024 · 2 comments

Comments

@nicholasbishop
Copy link
Member

nicholasbishop commented Aug 1, 2024

These impls were added in uefi-0.29.0, I missed a safety issue when reviewing the code.

The current impls look like this:

impl<'a> TryFrom<&[u8]> for &'a DevicePath { ... }

But they should look like this:

impl<'a> TryFrom<&'a [u8]> for &'a DevicePath { ... }

This wasn't caught by the compiler because internally these impls use unsafe pointer-based code. The missing lifetime means that the &[u8] buffer can be free'd while the &DevicePath still exists, which is UB.

The fix is straightforward, I will put up a PR. I think we should also do a 0.29.1 release since we're not quite ready for a 0.30.0 release yet. EDIT: actually, this is a semver-incompatible change, so it should be a 0.30.0 release. We can branch this off of commit 4e4e190.

cc @andre-braga

@nicholasbishop
Copy link
Member Author

nicholasbishop commented Aug 1, 2024

Fix is up: #1282

After that's merged, I will create a version-0.30 branch off of commit 4e4e190, then open a PR to cherry-pick the fix into that branch. Then I'll put up a release PR for the branch. (This will be the first time I've run the release workflow on a branch other than main, but it should be set up to work on version- branches already.)

@nicholasbishop
Copy link
Member Author

I've created the version-0.30 branch with the cherry-pick, and opened a release PR: #1285

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant