Skip to content

opt_dff: don't remove cells until all have been visited to prevent UAF #5165

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 3 commits into from
Jun 20, 2025

Conversation

georgerennie
Copy link
Collaborator

@georgerennie georgerennie commented Jun 4, 2025

What are the reasons/motivation for this change?

Fixes #5164.

Explain how this is achieved.

Instead of removing and emiting changed cells as they are found no longer needed, cache them and remove/emit afterwards to prevent invalidating the Cell*s in ModWalker.

If applicable, please suggest to reviewers how they can test the change.

Added a testcase from #5164 that fails with an ASAN build before this change.

@georgerennie
Copy link
Collaborator Author

This fix doesn't actually work yet as it turns out ff.emit() can also remove cells. Will finish a fix in the morning.

@georgerennie
Copy link
Collaborator Author

Ok I've fixed the further issue with emitting cells so this is ready for review.

@KrystalDelusion KrystalDelusion added the merge-soon Merge: PR will be merged at the end of the next work day unless concerns are raised label Jun 20, 2025
@georgerennie georgerennie merged commit 170933e into YosysHQ:main Jun 20, 2025
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge-soon Merge: PR will be merged at the end of the next work day unless concerns are raised
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use after free in opt_dff
2 participants