Skip to content

update linked_list_allocator to 0.10.4 #57

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

Merged
merged 1 commit into from
Nov 3, 2022
Merged

update linked_list_allocator to 0.10.4 #57

merged 1 commit into from
Nov 3, 2022

Conversation

rursprung
Copy link
Contributor

versions <= 0.10.1 were affected by CVE-2022-36086.
note that with 0.10.0 a breaking API change was done which changed the signature of init. this can however be avoided simply by casting back (to avoid a breaking API change in alloc-cortex-m by changing the API here as well - if wished, this should be done as a separate step).

i've also tried to instead switch to the init_from_slice API (introduced in 0.9.1), however I've failed at getting this to compile due to lifetimes (i'm sure it's somehow possible and i just missed the obvious...), but that'd anyway have been a breaking change for alloc-cortex-m and, if done, should be done in a separate step (though it'd definitely clean the API up and make it nicer!).

@rursprung rursprung requested a review from a team as a code owner October 29, 2022 15:57
versions <= 0.10.1 were affected by [CVE-2022-36086][].
note that with 0.10.0 a breaking API change was done which changed the
signature of `init`. this can however be avoided simply by casting back
(to avoid a breaking API change in `alloc-cortex-m` by changing the API
here as well - if wished, this should be done as a separate step).

i've also tried to instead switch to the `init_from_slice` API
(introduced in 0.9.1), however I've failed at getting this to compile
due to lifetimes (i'm sure it's somehow possible and i just missed the
obvious...), but that'd anyway have been a breaking change for
`alloc-cortex-m` and, if done, should be done in a separate step (though
it'd definitely clean the API up and make it nicer!).

[CVE-2022-36086]: GHSA-xg8p-34w2-j49j
Copy link
Member

@newAM newAM left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me, thanks for opening this PR! I'll wait for a maintainer who knows more about this crate for merging, since this is a good cause for a new release.


I also opened a PR for this CVE in rustsec/advisory-db#1448, for visibility with cargo audit.

@rursprung
Copy link
Contributor Author

thanks!

please note that dependabot reports this (that's how i found it: i pushed a small alloc-cortex-m based project to GitHub and immediately got the report) - if you haven't enabled it yet for the repository (or even the whole rust-embedded org) then you should strongly consider doing so as it both tries to do version updates when available (of course only works if it isn't a breaking change) and reports security issues like these. then you don't explicitly need it in cargo audit (though that's of course also important for all the projects not hosted on GitHub!)

@adamgreig
Copy link
Member

Thanks! I think it makes sense to release this fix by itself for now, what happens longer-term is probably more up in the air, especially with some level of global alloc hopefully reaching stable soon, the new critical-section crate that's probably a better choice for the mutex, and a lot of recent talk about alternatives to linked-list-allocator probably being better for this use-case.

bors r+

@bors
Copy link
Contributor

bors bot commented Nov 3, 2022

@bors bors bot merged commit 5b6f3ae into rust-embedded:master Nov 3, 2022
@adamgreig adamgreig mentioned this pull request Nov 3, 2022
@rursprung rursprung deleted the update-to-linked_list_allocator_0.10.4 branch November 3, 2022 07:22
bors bot added a commit that referenced this pull request Nov 6, 2022
58: Prepare v0.4.3 r=newAM a=adamgreig

cc #57

Co-authored-by: Adam Greig <adam@adamgreig.com>
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

Successfully merging this pull request may close these issues.

3 participants