Skip to content
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

{bp-15160} binfmt/exec: initialize binary_s to empty to avoid invaild access #15176

Merged
merged 2 commits into from
Dec 13, 2024

Conversation

jerpelea
Copy link
Contributor

Summary

binfmt/exec: initialize binary_s to empty to avoid invaild access

Included
#15124

Impact

RELEASE

Testing

CI

@github-actions github-actions bot added Area: BINFMT Size: S The size of the change in this PR is small labels Dec 13, 2024
@nuttxpr
Copy link

nuttxpr commented Dec 13, 2024

[Experimental Bot, please feedback here]

No. This PR summary lacks crucial information. While it links to the PR itself, the requirements ask for a summary within the PR description, not just a title and link. The requirements ask for:

  • Why is the change necessary? What problem does it solve? "Avoid invalid access" is a symptom, not a root cause. Explain what was being accessed invalidly and under what circumstances.
  • What functional part of the code changed? Be more specific than binfmt/exec. Which function(s), data structures, etc., were modified?
  • How does the initialization prevent the invalid access?

The impact section is also insufficient. "RELEASE" doesn't explain anything. While it might mean this is a bug fix for a release branch, it doesn't address any of the specific impact questions. Each "NO" or "YES" should be followed by a justification, even if it's a brief "N/A". For example, Impact on user: NO - This is an internal bug fix.

Finally, "CI" for testing is inadequate. The requirements explicitly ask for details about the build host and target platform(s), as well as actual test logs demonstrating the problem before the change and the correct behavior after the change. Just saying "CI" doesn't prove anything; the CI logs themselves should be referenced or included (if short).

@xiaoxiang781216 xiaoxiang781216 merged commit 200015d into apache:releases/12.8 Dec 13, 2024
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: BINFMT Size: S The size of the change in this PR is small
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants