utrace_control(XXXSTEP)->is_setting_trap_flag() is not safe

[Roland McGrath]

> Btw, I believe we have another problem. utrace_control(SINGLESTEP) calls
> user_enable_single_step() under utrace->lock. I don't really understand
> the magic in enable_single_step() but is_setting_trap_flag() calls
> access_process_vm(), this doesn't look good under spinlock.

Yes, this has been on my list of concerns for a long time but I always
forget about it.  Perhaps I intentionally let it lie as a test for
reviewers' attention to detail, we'll never know. :-)

> Hmm. Not sure how to fix this...

Well, one thing it can do is just punt them down to UTRACE_REPORT.
It's only a non-guaranteed optimization that these effects happen
directly from utrace_control() without being reinforced by a final
report_quiesce/report_signal callback, after all.

Most or all other machines do not have anything complex or blocky
going on inside those calls at all.  So potentially we could have
arch macros and punt only on x86.  But anyway.

I'd sort of intended to leave this undecided until we get more final
certainty on the locking corners of the utrace API.  For this and
other purposes it would be advantageous to use a mutex rather than a
spin lock for the utrace lock.  But that doesn't mesh with the hairy
RCU guarantees we try to make to avoid callers needing task refs.
So perhaps we'll in the end sort all that out differently and have a
mutex.  Then it would be plausible to hold that mutex while doing
access_process_vm, though it's still not ideal to have "instant"
async utrace ops like set_events and detach block on something
external like arbitrary user page faults.

So perhaps independent of the mutex/spin question (which we don't
need to get into right this minute), we can devise some scheme to
avoid wanting to hold that lock while doing user_enable_single_step.
I haven't lately thought through what exactly is getting
synchronized there that matters.


Thanks,
Roland