Skip to content

Conversation

@masaori335
Copy link
Contributor

@masaori335 masaori335 commented Jul 29, 2021

This reverts commit a56638f.

  • Fix a use-after-free reported by clang-analyzer

Co-authored-by: Tomoaki Tanaka tomoatan@yahoo-corp.jp


Address a use-after-free on top of #8182.
https://ci.trafficserver.apache.org/clang-analyzer/github/8182/2021-07-28-135835-24348-1/report-26932e.html#EndPath

@masaori335 masaori335 self-assigned this Jul 29, 2021
@masaori335 masaori335 added this to the 10.0.0 milestone Jul 29, 2021
@masaori335
Copy link
Contributor Author

It's a bit weird that we don't see this clang-analyzer report on 8.1.x (#8180). Do we have any diffs?

connectUp(e->ethread, NO_FD);
} else {
lock.release();
free(e->ethread);
Copy link
Member

Choose a reason for hiding this comment

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

Should change to this->free(e->thread); to be clear on what's being invoked.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It looks like this confused the clang-analyzer. Adding this fixed the report.

@masaori335
Copy link
Contributor Author

masaori335 commented Aug 1, 2021

Please merge without squashing to keep the revert commit from @tomoatan. We have the "Squash & merge" option only. I'll modify the commit message.

Copy link
Contributor

@bneradt bneradt left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@masaori335 masaori335 merged commit 6efb8d8 into apache:master Aug 2, 2021
@masaori335 masaori335 changed the title Fix a use-after-free reported by clang-analyzer Revert "Remove UnixNetVConnection::startEvent - not actually called. (#7596)" + clang-analyzer fix Aug 3, 2021
@zwoop
Copy link
Contributor

zwoop commented Aug 3, 2021

Cherry-picked to v9.0.x branch.
Cherry-picked to v9.1.x branch.

@zwoop zwoop modified the milestones: 10.0.0, 9.1.0 Aug 3, 2021
@bneradt bneradt mentioned this pull request Aug 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants