Skip to content

nandpart: fix UAF when freeing mbr - #264

Merged
apritzel merged 1 commit into
linux-sunxi:masterfrom
r4v3n6101:nand-uaf-fix
Nov 4, 2026
Merged

apritzel merged 1 commit into
linux-sunxi:masterfrom
r4v3n6101:nand-uaf-fix

Conversation

@r4v3n6101

@r4v3n6101 r4v3n6101 commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

The cleanup loop frees all mbrs entries, including the one referenced by
mbr. This leaves mbr pointing to freed memory that is subsequently used
to update and write the partition tables.
Skip mbr in the initial cleanup and free it after writing or when the
user cancels.

@apritzel apritzel left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you please extend the commit message a bit, and be it for spelling out "use after free", no need to use cryptic abbreviations, we want this readable and understandable by everybody.
Otherwise the whole code is not very elegant, so your solution is fine and looks correct.

@r4v3n6101

Copy link
Copy Markdown
Contributor Author

Sorry, I should’ve done that from the start. Thanks for reviewing!

@apritzel

apritzel commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

Thanks, that reads very good! Can you now please put this in the actual commit message, not just the PR description? And you would need your Signed-off-by: line, just like in Linux.
Happy to merge it then.

The cleanup loop frees all mbrs entries, including the one referenced by
mbr. This leaves mbr pointing to freed memory that is subsequently used
to update and write the partition tables.
Skip mbr in the initial cleanup and free it after writing or when the
user cancels.

Signed-off-by: r4v3n6101 <raven6107@gmail.com>
@r4v3n6101

Copy link
Copy Markdown
Contributor Author

Made it!

@apritzel
apritzel merged commit 513daa3 into linux-sunxi:master Nov 4, 2026
1 check passed
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.

2 participants