Linuxlator enable ptrace

[kib]

Summary

Quick, dirty and definitely problematic enable of Linux ptrace support.

The main issue with linux ptrace, was that fork1 is called with RFSTOPPED and TDB_STOPATFORK. fork1 will block waiting for the thread do stop, but is not started so it hangs forever.

This is true not only for linux_fork* but for any other thread forked with RFSTOPPED & TDB_STOPATFORK.

In this patch I have moved the wait to a separate function fork_finish. It is not called in fork1 if RFSTOPPED is set. Calls to fork_finish are added after the thread is scheduled in the linux_fork * functions.

The only other change to enable ptrace is adding TDB_BORN in linux_clone_proc on new thread created.

Obvious problems:

racct_proc_fork_done is moved in this function. I have no idea what this is and if should stay in fork1 before the return.
There are plenty of other callsites for fork1 that should also have fork_finish called. Some of them like the kthread* hopefully can't have TDB_STOPATFORK but some might have.

The code like this works, i.e. truss & gdb are fully functional on Linux binaries and I have not experienced issues.

Posting the code like this, as suggested by kib in the freebsd-emulation thread. The difference with initial patch there, is that I have moved the common code the fork_finish function.

[kib]

Please generate large diff context when you upload diff into phab, e.g. for svn it would be svn diff -x -U999999, for git diff -U999999.

You are adding _FreeBSD_ ptrace support to target linuxolator process, not the Linux emulation ptrace, right ? In other words, the patch allows to attach to the Linux process by a FreeBSD debugger, but not by a Linux debugger.

You are right that RFSTOPPED is used by kernel in more places than only linuxolator. Since you moved PRELE() call into fork_finish(), all places must call the function otherwise process hold count leaks. racct should be moved altogether, so this part is correct.

[yanko.yankulov_gmail.com]

In D15823#335057, @kib wrote:

Please generate large diff context when you upload diff into phab, e.g. for svn it would be svn diff -x -U999999, for git diff -U999999.

Noted. I even read it in the wiki page but totally ignored it

You are adding _FreeBSD_ ptrace support to target linuxolator process, not the Linux emulation ptrace, right ? In other words, the patch allows to attach to the Linux process by a FreeBSD debugger, but not by a Linux debugger.

Yes. I have not looked in what will be needed to emulate Linux ptrace support, but it seems that it will need this sorted out beforehand anyway.

You are right that RFSTOPPED is used by kernel in more places than only linuxolator. Since you moved PRELE() call into fork_finish(), all places must call the function otherwise process hold count leaks. racct should be moved altogether, so this part is correct.

Totally missed the PRELE. Shame on me :)

I checked the rest of the RFSTOPPED use points and applying the change seems trivial. So if the general idea is acceptable I can apply the changes to all the places and upload a new patch.

Just one more question. I can't figure out why we can't reparent the new proc in do_fork instead of waiting for its thread to do it? Any hint will be appreciated.

Thanks

[kib]

In D15823#335084, @yanko.yankulov_gmail.com wrote:

I checked the rest of the RFSTOPPED use points and applying the change seems trivial. So if the general idea is acceptable I can apply the changes to all the places and upload a new patch.

Right now I think that this is working approach.

Just one more question. I can't figure out why we can't reparent the new proc in do_fork instead of waiting for its thread to do it? Any hint will be appreciated.

I am not sure about your question. Can you point to the exact piece of code which you want to move into do_fork ? If you mean proc_reparent() call from fork_return(), I put it there to have all code dealing with the attaching new child to the debugger, in single place. ptracestop() must be called from the stopping thread context, and proc_reparent() as part of the attachment code is naturally located nearby. But I may be mis-interpreting your question, and also I do not see why you are asking.

[yanko.yankulov_gmail.com]

Just one more question. I can't figure out why we can't reparent the new proc in do_fork instead of waiting for its thread to do it? Any hint will be appreciated.

I am not sure about your question. Can you point to the exact piece of code which you want to move into do_fork ? If you mean proc_reparent() call from fork_return(), I put it there to have all code dealing with the attaching new child to the debugger, in single place. ptracestop() must be called from the stopping thread context, and proc_reparent() as part of the attachment code is naturally located nearby. But I may be mis-interpreting your question, and also I do not see why you are asking.

Yes, that the question. I was trying to figure out why we need the sleep in the fork path at all. Because if we can get rid of the sleep, there is no need to change anything else. So my reasoning was like this - it is either required by the ptrace interface/promises i.e. parent process will not continue execution before the child is attached, or it is part of the implementation of the interface.

I assumed it is not a ptrace requirement because there is no guarantees for the parent/child execution in non-ptraced mode, and went on to see why it might be needed. One thing in the current code is that we can't allow the parent process to exit before the child was reparented as we will not attach to the child.

So my question is if we reparent the child in do_fork wouldn't it be possible and desirable to drop the wait altogether?

[yanko.yankulov_gmail.com]

In D15823#335123, @yanko.yankulov_gmail.com wrote:

Just one more question. I can't figure out why we can't reparent the new proc in do_fork instead of waiting for its thread to do it? Any hint will be appreciated.

I am not sure about your question. Can you point to the exact piece of code which you want to move into do_fork ? If you mean proc_reparent() call from fork_return(), I put it there to have all code dealing with the attaching new child to the debugger, in single place. ptracestop() must be called from the stopping thread context, and proc_reparent() as part of the attachment code is naturally located nearby. But I may be mis-interpreting your question, and also I do not see why you are asking.

Yes, that the question. I was trying to figure out why we need the sleep in the fork path at all. Because if we can get rid of the sleep, there is no need to change anything else. So my reasoning was like this - it is either required by the ptrace interface/promises i.e. parent process will not continue execution before the child is attached, or it is part of the implementation of the interface.

I assumed it is not a ptrace requirement because there is no guarantees for the parent/child execution in non-ptraced mode, and went on to see why it might be needed. One thing in the current code is that we can't allow the parent process to exit before the child was reparented as we will not attach to the child.

So my question is if we reparent the child in do_fork wouldn't it be possible and desirable to drop the wait altogether?

I went on and implemented first version of removing the wait to try it out. https://reviews.freebsd.org/D15857
No issues so far. Looking forward any comments

[kib]

I believe that was superseded by D15857, which was committed quite some time ago.