Skip to content

Conversation

@MegaManSec
Copy link

@MegaManSec MegaManSec commented Sep 15, 2025

Merging filter collections didn’t carry over notify_used; if the source had SCMP_ACT_NOTIFY rules, the merged filter could skip requesting SECCOMP_FILTER_FLAG_NEW_LISTENER, yielding no listener FD. This ORs the flag from source to destination to keep seccomp notification working.

This bug was discovered with ZeroPath.

@pcmoore pcmoore changed the title propagate notify_used when merging filter collections BUG: propagate notify_used when merging filter collections Oct 7, 2025
@pcmoore pcmoore added this to the v2.6.1 milestone Oct 7, 2025
@coveralls
Copy link

coveralls commented Oct 7, 2025

Coverage Status

coverage: 89.024% (-0.02%) from 89.046%
when pulling 6358d0e on MegaManSec:pass
into 2bc7189 on seccomp:main.

@pcmoore
Copy link
Member

pcmoore commented Oct 8, 2025

Hi @MegaManSec, thanks for catching this and sending a fix!

I would like to request to small changes to the PR/commit:

  • Could you copy the problem and fix description at the top of this PR into the commit description? This preserves the explanation of the change in the git log so it easier for us to understand the logic behind the change in the future.
  • Instead of using a GH "noreply" email in your sign-off, please use a proper email.

Thanks!

@MegaManSec
Copy link
Author

  1. Done!
  2. I would really prefer not to. This is to reduce the amount of spam I receive.

@pcmoore
Copy link
Member

pcmoore commented Oct 8, 2025

  1. Done!

Thanks!

  1. I would really prefer not to. This is to reduce the amount of spam I receive.

I see you've listed an email on the contact page of your website, is the problem you don't want to list it in the PR, or you don't want it in the git log at all? We really want to have an email address in the git log, it's helpful for a number of things.

@MegaManSec
Copy link
Author

MegaManSec commented Oct 8, 2025

I see you've listed an email on the contact page of your website, is the problem you don't want to list it in the PR, or you don't want it in the git log at all? We really want to have an email address in the git log, it's helpful for a number of things.

I've found that when I add my email address in any git log which is uploaded to github, i start to receive some type of spam on that email address in a matter of days; using the noreply github one allows it to be obvious where the PR is coming from (megamansec), while ensuring I don't receive that spam. Using the noreply github one also links to commit to my github account; which otherwise wouldn't be linked, as i remember.

@pcmoore
Copy link
Member

pcmoore commented Oct 9, 2025

Unfortunately, I think we really want an email address in the git log as I want the contact info to remain useful outside of GH.

@MegaManSec
Copy link
Author

Can I simply write another email address, but not sign with it? By signing with it, and for it to be associated with my github account, I need to do one of the following:

  1. Use my (github-exclusive) email address as the signer,
  2. Associate (register) a new email address on my github account, which means I will have two email addresses registered to my account, which defeats the purpose of point number one.

Since Github does not associate commits with a github account by the actual committer, but rather the Author and/or the committer of the commit, if this makes it difficult for me.

Thanks!

@pcmoore
Copy link
Member

pcmoore commented Oct 9, 2025

Can I simply write another email address, but not sign with it?

Perhaps some clarification would help here ... when I talk about a sign-off, I'm talking about the metadata at the end of the patch that satisfies the DCO requirement and looks like this:

Signed-off-by: name <email>

... it's entirely independent from anything GH does and from anything you may do with commit signing. So, yes, as long as you have a Signed-off-by: ... line with a valid email we're going to be happy :)

Hopefully that helps?

Merging filter collections didn’t carry over notify_used; if the
source had SCMP_ACT_NOTIFY rules, the merged filter could skip requesting
SECCOMP_FILTER_FLAG_NEW_LISTENER, yielding no listener FD. This ORs the flag
from source to destination to keep seccomp notification working.

Signed-off-by: Joshua Rogers <[email protected]>
@MegaManSec
Copy link
Author

I'm talking about the metadata at the end of the patch that satisfies the DCO requirement and looks like this

No, that doesn't work, because the DCO bot does not like that the committer email address differs from the sign-off:

Commit sha: 6358d0e, Author: Joshua Rogers, Committer: Joshua Rogers; Expected "Joshua Rogers [email protected]", but got "Joshua Rogers [email protected]".

@pcmoore
Copy link
Member

pcmoore commented Oct 9, 2025

Don't worry about the DCO bot, we can override as needed. I just manually approved the rest of the CI, let's make sure it passes cleanly, but from what I saw in the PR I don't expect any problems.

@MegaManSec
Copy link
Author

Thank you!

@MegaManSec

This comment was marked as spam.

@MegaManSec
Copy link
Author

Any chance to get this merged? I would like to delete my fork

@pcmoore
Copy link
Member

pcmoore commented Dec 31, 2025

I'm going to "hide" this comment as it looks like SPAM (?); if you think that's a misunderstanding please let me know.

@MegaManSec
Copy link
Author

I think it's fine to hide it, however it was deliberately posted.

Previously, I pushed back against publishing a commit with a real email because I didn't want to receive spam.

I see you've listed an email on the contact page of your website, is the problem you don't want to list it in the PR, or you don't want it in the git log at all? We really want to have an email address in the git log, it's helpful for a number of things.

In the hidden comment, a spammer had emailed the address joshua-github-joshuahu@.... This email address has only ever been used as the Author of the MegaManSec/JoshuaHu repository. The point was to show that this type of spam being sent to committers' email addresses are common enough.

@pcmoore pcmoore self-requested a review December 31, 2025 14:41
Copy link
Member

@pcmoore pcmoore left a comment

Choose a reason for hiding this comment

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

I would suggest we add a "db:" prefix when merging, e.g. "db: propagate notify_used when merging filter collections", but otherwise this looks good to me.

@drakenclimber ?

Acked-by: Paul Moore <[email protected]>

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.

3 participants