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

Q: Is there any way for the NotifReceive function to end? #104

Open
neblen opened this issue Jan 11, 2023 · 6 comments
Open

Q: Is there any way for the NotifReceive function to end? #104

neblen opened this issue Jan 11, 2023 · 6 comments
Labels

Comments

@neblen
Copy link

neblen commented Jan 11, 2023

When no more information is generated in ScmpFd, the NotifReceive function will block and will not return a value. At this time, the process corresponding to ScmpFd has exited, so there is no need to monitor again. Is there any way to end the NotifReceive function?

func NotifReceive(fd ScmpFd) (*ScmpNotifReq, error) {
	return notifReceive(fd)
}
@alban
Copy link
Contributor

alban commented Jan 11, 2023

The Linux API with ioctl SECCOMP_IOCTL_NOTIF_RECV does not seem to have an easy way to interrupt it when the container terminates (when the seccomp filter is no longer attached to any process).

I see this was discussed on the lkml:
https://lists.linuxfoundation.org/pipermail/containers/2020-November/042590.html
But the patch was not merged and as of Linux v6.2-rc3, there does not seem to be any clean solution.

The kernel function seccomp_notify_recv can be interrupted by a signal:
https://github.com/torvalds/linux/blob/v6.2-rc3/kernel/seccomp.c#L1470

ret = down_interruptible(&filter->notif->request);

So the go routine could be tied to a thread (with runtime.LockOSThread) and we could have a Golang ticker interrupting it regularly and check if the container still exists.

I wonder if Golang thread preemption with SIGURG would help us for that somehow.

cc @rata

@pcmoore pcmoore changed the title Is there any way for the NotifReceive function to end? Q: Is there any way for the NotifReceive function to end? Jan 11, 2023
@neblen
Copy link
Author

neblen commented Jan 12, 2023

Hi~ @alban I try to bind notifHandler goroutine with seccomp agent single thread by using runtime.LockOSThread. When the process monitored by notifHandler exited, another goroutine would kill the notifHandler thread.

But If the notifHandler thread is killed, the seccomp agent process will restart and the monitored data will be lost.

So it seems that the notifHandler coroutine needs to exit by itself.

code such as:

func notifHandler(reg *registry.Registry, seccompFile *os.File) {
    runtime.LockOSThread() //bind thread
    ProcessLock.Lock()
    ProcessCache[pid].tid = syscall.Gettid() //get the thread id
    ProcessLock.Unlock()
   ...
}
func CheckProcessStatus() {
	checkTicker := time.Tick(time.Second)
	for {
		select {
		case <-checkTicker:
			ProcessLock.Lock()
			for pid, processFd := range ProcessCache {
				if !alarm.CheckPid(pid) {
					log.Infof("CheckProcessStatus Process %d is dead!", pid)
					kErr := syscall.Kill(processFd.tid, syscall.SIGKILL)
					if kErr != nil {
						log.Errorf("kill tid error: %v", kErr)
					}
					delete(ProcessCache, pid)
				}
			}
			ProcessLock.Unlock()
		}
	}
}

@neblen
Copy link
Author

neblen commented Jan 12, 2023

Hi~ @alban @rata thanks for help and reply~
According to your suggestion, I think of two ways to activate the NotifReceive function.

The first is to send a signal to the kernel to interrupt the NotifReceive function in user mode (it is difficult to implement at present, and the related libseccomp-golang api is not yet supported).

The second is to imitate The kernel writes data to ScmpFd to activate the NotifReceive function (I don't know if this is feasible).

@alban
Copy link
Contributor

alban commented Jan 12, 2023

I think you have to use syscall.tgkill to send a signal to a single thread.

@neblen
Copy link
Author

neblen commented Jan 12, 2023

I think you have to use syscall.tgkill to send a signal to a single thread.

Hi~ @alban I use syscall.Tgkill(processFd.tgid, processFd.tid, syscall.SIGTERM) or kErr := syscall.Tgkill(processFd.tgid, processFd.tid, syscall.SIGKILL) . These will cause the seccomp-agent process to be killed. The Seccomp agent container restart.

test code:

func notifHandler(reg *registry.Registry, seccompFile *os.File) {
    runtime.LockOSThread() //bind thread
    ProcessLock.Lock()
    ProcessCache[pid].tid = syscall.Gettid()  //get the thread id
    ProcessCache[pid].tgid = syscall.Getpid() //tgid is the pid of the thread 
    ProcessCache[pid].pid = syscall.Getpid() 
    ProcessLock.Unlock()
   ...
}
func CheckProcessStatus() {
	checkTicker := time.Tick(time.Second)
	for {
		select {
		case <-checkTicker:
			ProcessLock.Lock()
			for pid, processFd := range ProcessCache {
				if !alarm.CheckPid(pid) {
					log.Infof("CheckProcessStatus Process %d is dead!", pid)
					kErr := syscall.Tgkill(processFd.tgid, processFd.tid, syscall.SIGKILL)
                                        // kErr := syscall.Tgkill(processFd.tgid, processFd.tid, syscall.SIGTERM)
					if kErr != nil {
						log.Errorf("kill tid error: %v", kErr)
					}
					delete(ProcessCache, pid)
				}
			}
			ProcessLock.Unlock()
		}
	}
}

@rata
Copy link
Contributor

rata commented Jan 12, 2023

I guess is only killed if it doesn't handle the signal (and the signal it is sent default action is to kill the process), right?

Anyways, I think what I mentioned here might be a better option: kinvolk/seccompagent#30 (comment). If we poll on the fd, we should detect either when a notification is received or when the process died. See this upstream commit (also mentioned in the linked issue): torvalds/linux@99cdb8b

We will probably need some plumbing here and it will not be a small change, at least semantically. So I'd like to see what @pcmoore @drakenclimber @kolyshkin think about this.

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

4 participants