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

BUG: update the s390 and s390x munge functions #226

Closed
wants to merge 3 commits into from

Conversation

drakenclimber
Copy link
Member

The s390 and s390x munge functions were missing a few syscalls and that was causing failures in the s390x TravisCI run.

This is another step to resolving pull request #215

@coveralls
Copy link

coveralls commented Mar 24, 2020

Coverage Status

Coverage decreased (-0.04%) to 92.701% when pulling 967c984 on drakenclimber:pull/215 into 5abc3b1 on seccomp:master.

@pcmoore
Copy link
Member

pcmoore commented Mar 25, 2020

Do we also need to add the following?

  • semop
  • shmat
  • shmdt
  • shmget
  • shmctl

@pcmoore pcmoore changed the title Update the s390 and s390x munge functions BUG: update the s390 and s390x munge functions Mar 25, 2020
@drakenclimber
Copy link
Member Author

Good question. I'll run them through the s390x VM to see if they're needed. If so, I'll likely add them to the test to ensure they are properly verified in the future.

@drakenclimber
Copy link
Member Author

Do we also need to add the following?

* semop
* shmat
* shmdt
* shmget
* shmctl

Based upon Arnd's commit that was a catalyst for much of this work, I would think they should be munged as well.

But I can't create a real-world test scenario where the munge of the above shm*() syscalls is required. (At least on my s390x machine, that is. Note that I don't have console access and grubby isn't accepting my attempts to change to a newer kernel, so I'm limited to a really old kernel. I've tried hacking /usr/include/asm/unistd.h by hand to influence the behavior, but it hasn't affected any outcomes.)

Strangely, I can get test 37 to fail if the msg*() syscalls aren't munged, but the munging (or not munging) of shm*() doesn't affect the test.

I'm torn. I think you are correct that they should also be munged, but I can't prove it. Thoughts, @pcmoore?

@giuseppe
Copy link
Contributor

please ignore my review, I've clicked by mistake while I was still checking if it is something I broke (and my knowledge is quite limited to make any real review)

@pcmoore
Copy link
Member

pcmoore commented Mar 26, 2020

Doing the munge to a pseudo syscall should be harmless as the libseccomp code will Do The Right Thing and generate the necessary rules to handle both cases, where not doing the munge will cause only the new direct-wired syscall rule to be created.

That said, we should also take a closer look at s390/s390x to make sure that the *_syscall_demux() and *_syscall_mux() are also correct.

@pcmoore
Copy link
Member

pcmoore commented Mar 26, 2020

please ignore my review, I've clicked by mistake while I was still checking if it is something I broke (and my knowledge is quite limited to make any real review)

:)

Thanks for looking, but this was broken long before your changes ;)

The following syscalls were missing from the s390x munge
functions - s390x_syscall_resolve_name_munge() and
s390x_syscall_resolve_num_munge():
	msgctl, msgget, msgrcv, msgsnd, semctl, semget, and
	semtimedop

Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
@drakenclimber
Copy link
Member Author

The various shm*() functions were already in both the s390x and s390 munge/demunge code, so they should be good to go.

semop() is not supported by s390 or s390x.

I did add semtimedop() support to the s390 mux/demux functions as it had not yet been enabled.

The following syscalls were missing from the s390 munge
functions - s390_syscall_resolve_name_munge() and
s390_syscall_resolve_num_munge():
	msgctl, msgget, msgrcv, msgsnd, semctl, semget, and
	semtimedop

Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
This commit adds semtimedop() support to the s390
mux/demux functions - _s390_syscall_demux() and
_s390_syscall_mux().

Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
@pcmoore
Copy link
Member

pcmoore commented Mar 27, 2020

Looks good to me, merged at HEAD 0e3db6a. Thanks!

@pcmoore pcmoore closed this Mar 27, 2020
@drakenclimber drakenclimber deleted the pull/215 branch May 26, 2020 19:31
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.

None yet

4 participants