seccomp / libseccomp

The main libseccomp repository
GNU Lesser General Public License v2.1
791 stars 170 forks source link

BUG: fix aliasing UB in the BPF simulator #425

Closed q66 closed 2 months ago

q66 commented 8 months ago

This restores test suite on Clang 17.

pcmoore commented 8 months ago

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

jvoisin commented 8 months ago

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?

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".

q66 commented 8 months ago

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.

pcmoore commented 8 months ago

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.

pcmoore commented 8 months ago

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.

pcmoore commented 8 months ago

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?

Then it would be nice to update the CONTRIBUTING.md file to reflect this, because I haven't found where it's mentioned anywhere.

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).

jvoisin commented 8 months ago

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"

rusty-snake commented 8 months ago

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;

Futhermore, everybody has to trust you that "John Smith" is your real name. So names that look like a non-real name (e.g. X Æ A-12) will be scrutinized while everything that looks like a real name will get accepted.

drakenclimber commented 4 months ago

The technical aspect of this change looks fine to me. I also agree with @pcmoore's comments above about names and its legal aspects. If you have issues with this or questions, @q66, feel free to ask here or send @pcmoore or me a DM. Thanks.

q66 commented 2 months ago

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

thesamesam commented 2 months ago

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.

q66 commented 2 months ago

sure, thanks :)