-
Notifications
You must be signed in to change notification settings - Fork 176
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
BUG: fix aliasing UB in the BPF simulator #425
Conversation
Signed-off-by: q66 <[email protected]>
Hi @q66, thank you for the patch! It looks good to me, but we do ask that contributors use their real name in their DCO sign-off. Would you mind updating your sign-off with your name? https://github.com/seccomp/libseccomp/blob/main/CONTRIBUTING.md#sign-your-work |
Then it would be nice to update the CONTRIBUTING.md file to reflect this, because I haven't found where it's mentioned anywhere. Interestingly, the "Developer's Certificate of Origin" part is based on the Linux one, that doesn't require "real names". |
The Linux kernel project recently dropped the real name DCO requirement as far as I know. While I don't mind disclosing my real name, in practice what a "real name" is often tends to be blurry, and in context of developer contributions it makes no sense; if it's about me being reachable, I've been around with the same handle for easily 20 years now; meanwhile, requiring a legal name is potentially discriminatory or maybe even dangerous for certain individuals, so such requirement ranges from merely pointless to possibly harmful. Therefore, I'd really appreciate if you dropped this requirement; not for me, but for anybody who may actually be affected by this. |
That's something we can consider, but that is a much larger issue that the minor fix in this PR. If you are not comfortable using your real name in the sign-off, or if you are opposed to it as a matter of principle, I completely understand, but at this point in time I can't merge your patch as-is. |
With the huge caveat that IANAL, our contributor doc intentionally uses the term "real name" and not "legal name"; the ambiguity of a "real name" is intentional. I don't want anyone to have to deadname (or similar) themselves. |
I think you may have missed it, please re-read the doc and you should find it at the end of the section (which is why I linked it there). |
oh, you're right, my bad: "... then you just add a line to the bottom of your patch description, with your real name" |
Futhermore, everybody has to trust you that "John Smith" is your real name. So names that look like a non-real name (e.g. |
i'm going to close this; due to reasons, i will no longer be using my legal name online, and i don't currently feel comfortable using my real name in random foss contributions either if somebody else wants to pick up the one-liner, go right ahead |
I'll take it later. The change is obviously correct and it's the canonical fix for this type of problem, so there's no issue in putting it in my name. |
sure, thanks :) |
See seccomp#425. Punning sys_data_b between uint32_t* and struct* seccomp_data isn't legal, use memcpy to fix the testsuite with Clang 17. Modern compilers recognise this idiom and optimise it out anyway. Signed-off-by: Sam James <[email protected]>
See #425. Punning sys_data_b between uint32_t* and struct* seccomp_data isn't legal, use memcpy to fix the testsuite with Clang 17. Modern compilers recognise this idiom and optimise it out anyway. Signed-off-by: Sam James <[email protected]> Acked-by: Tom Hromatka <[email protected]> Signed-off-by: Paul Moore <[email protected]>
This restores test suite on Clang 17.