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

Incorrect value for PTRACE_EVENT_SECCOMP #724

Closed
xd009642 opened this issue Aug 7, 2017 · 9 comments
Closed

Incorrect value for PTRACE_EVENT_SECCOMP #724

xd009642 opened this issue Aug 7, 2017 · 9 comments
Labels

Comments

@xd009642
Copy link
Contributor

xd009642 commented Aug 7, 2017

PTRACE_EVENT_SECCOMP and PTRACE_EVENT_EXIT are both given the value of 6. Instead the seccomp event should have the value 7. Source: https://github.com/torvalds/linux/blob/master/include/uapi/linux/ptrace.h

@Susurrus
Copy link
Contributor

Susurrus commented Aug 8, 2017

Indeed. All constants in src/sys/ptrace.rs should be replaced with their libc equivalents. All the PTRACE_EVENT_* constants are missing, however, so those should be submitted to upstream for all libc/nix supported platforms. A partial PR regarding these constants is in #461. If you'd like to get those constants upstreamed and then take over #461, it'd be greatly appreciated as I'm not certain @chaosagent is working on it anymore.

@Susurrus
Copy link
Contributor

Susurrus commented Aug 8, 2017

Whoops, misspoke on this. All PTRACE_EVENT_* constants have been merged into libc.

@xd009642 Would you be interested in cleaning up the constants in src/sys/ptrace.rs? It'd be mostly what's already done in #461 but with the addition of using PTRACE_EVENT_* constants from libc. Using the libc_enum! macro makes this pretty easy.

@xd009642
Copy link
Contributor Author

xd009642 commented Aug 8, 2017

I am slightly busy at the moment I'll try and fit sometime in this weekend or maybe later this week. Guess it just depends how quickly you'd like this to move along

@Susurrus
Copy link
Contributor

Susurrus commented Aug 9, 2017

@xd009642 I like to move this along as fast you can, whatever that speed is. We just don't have enough people who work on nix directly to take care of many of these types of issues, instead relying on external contributors to scratch their own itch. So if you weren't willing to take this on, I don't know when this would get resolved.

@xd009642
Copy link
Contributor Author

xd009642 commented Aug 9, 2017

That's fair enough, I'll probably start looking at it tonight or tomorrow.

@Susurrus Susurrus added the A-bug label Aug 9, 2017
@xd009642
Copy link
Contributor Author

I've started work on it today, so I'll use this issue to ask questions/float ideas until I've submitted something I'm somewhat happy with if that's okay?

I might not use the libc_enum macro because there look to be some type (signed versus unsigned) discrepancies and things available in some OS's but not others. I'm still working it out so it's early days though. Also, libc removed PTRACE_EVENT_STOP as it is defined in glibc 2.26. I may define the event enum with the macro so i can add the stop event field for completeness

@Susurrus
Copy link
Contributor

I've started work on it today, so I'll use this issue to ask questions/float ideas until I've submitted something I'm somewhat happy with if that's okay?

That's fine, but it's generally easier to discuss code when I can comment easily on specific lines. A PR allows me to do that and is better for reviews. I'd suggest you just open a PR as soon as you have any code to show, but until then discussing it here is fine.

I might not use the libc_enum macro because there look to be some type (signed versus unsigned) discrepancies and things available in some OS's but not others.

Fixing datatypes and using #cfg[] is supported. Search through the source for usages that will show you.

Also, libc removed PTRACE_EVENT_STOP as it is defined in glibc 2.26. I may define the event enum with the macro so i can add the stop event field for completeness.

Yeah, I don't even have glibc on my Fedora 25 box, so we'll need to wait on this one for a bit. I'm pretty certain this isn't used anywhere, however, so I'd be fine leaving it commented out with a FIXME.

@xd009642
Copy link
Contributor Author

Yeah I'll raise the PR when I've got something that's actually worth showing in some form then continue discussion there. And I'll look into the source examples

@xd009642
Copy link
Contributor Author

Just going to close this issue as it was resolved in #749 merged today

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants