[PATCH 134] mv kernel/ptrace.c kernel/ptrace-utrace.c
mv kernel/ptrace.c kernel/ptrace-utrace.c.
Then I will send the patch which restores the old ptrace.c and
moves kernel/ptrace-common.h into it as you suggested before.
I am not sure what is the best way to do these renames but I hope
this doesn't really matter, utrace-ptrace branch is only for us.
The goal is to make it testable with and without CONFIG_UTRACE.
---
kernel/Makefile|1 +
kernel/ptrace-utrace.c | 1117
kernel/ptrace.c| 1117
3 files changed, 1118 insertions(+), 1117 deletions(-)
create mode 100644 kernel/ptrace-utrace.c
delete mode 100644 kernel/ptrace.c
diff --git a/kernel/Makefile b/kernel/Makefile
index 8f41620..04e9139 100644
--- a/kernel/Makefile
+++ b/kernel/Makefile
@@ -69,6 +69,7 @@ obj-$(CONFIG_RESOURCE_COUNTERS) += res_counter.o
obj-$(CONFIG_STOP_MACHINE) += stop_machine.o
obj-$(CONFIG_KPROBES_SANITY_TEST) += test_kprobes.o
obj-$(CONFIG_UTRACE) += utrace.o
+obj-$(CONFIG_UTRACE) += ptrace-utrace.o
obj-$(CONFIG_AUDIT) += audit.o auditfilter.o audit_watch.o
obj-$(CONFIG_AUDITSYSCALL) += auditsc.o
obj-$(CONFIG_GCOV_KERNEL) += gcov/
diff --git a/kernel/ptrace-utrace.c b/kernel/ptrace-utrace.c
new file mode 100644
index 000..e10410c
--- /dev/null
+++ b/kernel/ptrace-utrace.c
@@ -0,0 +1,1117 @@
+/*
+ * linux/kernel/ptrace.c
+ *
+ * (C) Copyright 1999 Linus Torvalds
+ *
+ * Common interfaces for ptrace() which we do not want
+ * to continually duplicate across every architecture.
+ */
+
+#include linux/capability.h
+#include linux/module.h
+#include linux/sched.h
+#include linux/errno.h
+#include linux/mm.h
+#include linux/highmem.h
+#include linux/pagemap.h
+#include linux/smp_lock.h
+#include linux/ptrace.h
+#include linux/utrace.h
+#include linux/security.h
+#include linux/signal.h
+#include linux/audit.h
+#include linux/pid_namespace.h
+#include linux/syscalls.h
+#include linux/uaccess.h
+#include ptrace-common.h
+
+/*
+ * ptrace a task: make the debugger its new parent and
+ * move it to the ptrace list.
+ *
+ * Must be called with the tasklist lock write-held.
+ */
+void __ptrace_link(struct task_struct *child, struct task_struct *new_parent)
+{
+ BUG_ON(!list_empty(child-ptrace_entry));
+ list_add(child-ptrace_entry, new_parent-ptraced);
+ child-parent = new_parent;
+}
+
+/*
+ * unptrace a task: move it back to its original parent and
+ * remove it from the ptrace list.
+ *
+ * Must be called with the tasklist lock write-held.
+ */
+void __ptrace_unlink(struct task_struct *child)
+{
+ BUG_ON(!child-ptrace);
+
+ child-ptrace = 0;
+ child-parent = child-real_parent;
+ list_del_init(child-ptrace_entry);
+
+ arch_ptrace_untrace(child);
+}
+
+struct ptrace_context {
+ int options;
+
+ int signr;
+ siginfo_t *siginfo;
+
+ int stop_code;
+ unsigned long eventmsg;
+
+ enum utrace_resume_action resume;
+};
+
+#define PT_UTRACED 0x1000
+
+#define PTRACE_O_SYSEMU0x100
+
+#define PTRACE_EVENT_SYSCALL_ENTRY (1 16)
+#define PTRACE_EVENT_SYSCALL_EXIT (2 16)
+#define PTRACE_EVENT_SIGTRAP (3 16)
+#define PTRACE_EVENT_SIGNAL(4 16)
+/* events visible to user-space */
+#define PTRACE_EVENT_MASK 0x
+
+static inline bool ptrace_event_pending(struct ptrace_context *ctx)
+{
+ return ctx-stop_code != 0;
+}
+
+static inline int get_stop_event(struct ptrace_context *ctx)
+{
+ return ctx-stop_code 8;
+}
+
+static inline void set_stop_code(struct ptrace_context *ctx, int event)
+{
+ ctx-stop_code = (event 8) | SIGTRAP;
+}
+
+static inline struct ptrace_context *
+ptrace_context(struct utrace_engine *engine)
+{
+ return engine-data;
+}
+
+static const struct utrace_engine_ops ptrace_utrace_ops; /* forward decl */
+
+static struct utrace_engine *ptrace_lookup_engine(struct task_struct *tracee)
+{
+ return utrace_attach_task(tracee, UTRACE_ATTACH_MATCH_OPS,
+ ptrace_utrace_ops, NULL);
+}
+
+static struct utrace_engine *
+ptrace_reuse_engine(struct task_struct *tracee)
+{
+ struct utrace_engine *engine;
+ struct ptrace_context *ctx;
+ int err = -EPERM;
+
+ engine = ptrace_lookup_engine(tracee);
+ if (IS_ERR(engine))
+ return engine;
+
+ ctx = ptrace_context(engine);
+ if (unlikely(ctx-resume == UTRACE_DETACH)) {
+ /*
+* Try to reuse this self-detaching engine.
+* The only caller which can hit this case is ptrace_attach(),
+* it holds -cred_guard_mutex.
+*/
+ ctx-options = 0;
+ ctx-eventmsg = 0;
+
+ /* make sure we don't get unwanted
Re: [PATCH 0/4] utrace-resume fixes
On 11/18, Roland McGrath wrote: So maybe you will look at this and merge them before I do. Whatever we do, perhaps it makes sense to apply your patch in https://www.redhat.com/archives/utrace-devel/2009-November/msg00109.html first and then do further changes? This way we will have the working utrace-ptrace branch which passes ptrace-tests at least, and we can ask Cai to do more testing. Oleg.
Re: [PATCH 133] stepping, accommodate to utrace-cleanup changes
The former is e.g. PTRACE_SINGLESTEP while an unrelated engine uses UTRACE_EVENT(SYSCALL_ENTRY). The ptrace engine's report_quiesce returns UTRACE_SINGLESTEP. finish_resume_report() calls user_enable_single_step(). That seems fine. Right? In this case ptrace_report_quiesce(event) returns UTRACE_RESUME, note that it does return event ? UTRACE_RESUME : ctx-resume; and event == SYSCALL_ENTRY. This looks correct. With the utrace layer changes we're discussing, we need it to return UTRACE_SINGLESTEP (i.e. ctx-resume) here. AFAICT that never hurts even with today's utrace layer. I see. finish_resume_report() will only do utrace_stop() and then we'll go directly into the syscall unless someone used UTRACE_REPORT. Yes, utrace_stop() will only set TIF_NOTIFY_RESUME and utrace-resume. In the real resume-to-user cases, that's fine because we'll handle that in utrace_resume() or utrace_get_signal() before doing anything else. Not sure I understand. utrace_stop() will not set TIF_NOTIFY_RESUME? We are talking about the case when the tracee stops in SYSCALL_ENTER, and then the tracer does utrace_control(UTRACE_SINGLESTEP) to resume. In this case utrace_control() sets -resume/TIF_NOTIFY_RESUME, yes. I think you do understand fine. Yes, that's what it will do. I was just recognizing that this isn't enough in the syscall-entry case. this seems like the right idea for how to get tracehook_report_syscall_exit called in the cases where it is in old ptrace. Afaics yes. But, what if another engine does utrace_control(UTRACE_INTERRUPT) ? Hmm. Yes, I think this is the one case that is unlike all the others. That is, UTRACE_INTERRUPT at syscall-entry. In all other cases, nothing would care about utrace-resume until we get to get_signal_to_deliver anyway, where the UTRACE_SIGNAL_REPORT pass will trump any pending resume action anyway so you don't care that you lost track of it when UTRACE_INTERRUPT came in. Hmm. So what does UTRACE_INTERRUPT mean here anyway? It means that we should report soon and that signal_pending() should be true until that report. So today that means you can get into the syscall with signal_pending() and depending on the particular call that might cause a normal blocking path not to block and/or a -EINTR/-ERESTART* return, but some syscalls will just complete normally. How about if we say that UTRACE_INTERRUPT at syscall-entry means that the syscall really doesn't happen at all? That is, instead, you force an -ERESTARTNOHAND return and the normal roll-back such that you get your report_signal callback before the syscall is entered. That even seems like a useful utrace API feature, as well as conveniently smoothing over this quirk in the resume action handling. My idea here is that this makes it OK to lose UTRACE_SINGLESTEP here because from the user-mode-centric perspective no instruction has happened yet. The original guy who did UTRACE_SINGLESTEP (and perhaps never cared about syscall tracing) will get a generic report_quiesce or report_signal at which it needs to reassert UTRACE_SINGLESTEP. This merits more thought. For now, let's just leave an XXX comment about a UTRACE_INTERRUPT effectively swallowing the UTRACE_SINGLESTEP that should cause utrace_report_syscall_exit to be called. I think we can revisit this corner after we have merged up all the rest of it. Argh. I meant, from the caller pov, utrace_control(UTRACE_SINGLESTEP) does enable_step() immediately, like it did before utrace-cleanup changes. I see. From the utrace API perspective, I don't think anything changes here. In today's code, the resume action can take effect without a subsequent utrace callback. So that's the same as it ever was, and where this actually happens is not something that a utrace caller should know or can tell. IOW, suppose the tracee is stopped, and ptrace does UTRACE_SINGLESTEP. The tracee resumes, processes -resume, and does enable_step(). From the ptrace pov, it looks as if utrace_control() does enable_step() before utrace_wakeup(). Sure. I meant, every time the tracee stops, it should process -resume after wakeup. Looks like, the patch you sent in another thread (which adds apply_resume_action()) does something like this, right? Right. Thanks, Roland
Re: [PATCH 134] mv kernel/ptrace.c kernel/ptrace-utrace.c
I am not sure what is the best way to do these renames but I hope this doesn't really matter, utrace-ptrace branch is only for us. Sure, whatever you want to try is fine. The goal is to make it testable with and without CONFIG_UTRACE. Ok. We need to get upstream feedback on what they do or don't want like that. Some people have definitely said they don't want to see two implementations in the tree at the same time. But I can never guess what they will say next week. You'll get the joy of hashing that out with them. :-) Thanks, Roland
Re: [PATCH 0/4] utrace-resume fixes
Whatever we do, perhaps it makes sense to apply your patch in https://www.redhat.com/archives/utrace-devel/2009-November/msg00109.html first and then do further changes? Ok. v2.6.32-rc8-245-g3d4f9cf has that. I'll shelve this 4-patch series while we keep discussing (one more reply to come from me as of now). Then you send me a fresh series done however you think is best given that discussion. (So if you want to do more relative to these 4 patches, just resend them as part of the new series.) Thanks, Roland
Re: [PATCH 0/4] utrace-resume fixes
Somehow I can't really understand this patch. I hope more or less
I can see what it does, but the resulting code looks even more
subtle to me.
Well, it was an untested draft and probably needed more comments.
With this patch, apply_resume_action() is always called after
utrace_stop(). Well, except for utrace_report_exit(), but I guess
it could do apply_resume_action() too.
It could, but it just never matters at all. The only thing that can
possibly be meaningful is UTRACE_INTERRUPT, and for that finish_report has
set TIF_SIGPENDING already anyway.
Now it is not clear why utrace_control(SINGLESTEP) sets
TIF_NOTIFY_RESUME, it is not needed after apply_resume_action()
processes -resume. Yes, we have jctl stops, but we can move
this code into utrace_finish_stop().
Yes, it is not really needed. But note that it is actually now possible to
use UTRACE_SINGLESTEP without UTRACE_STOP first--not that it's really
useful, but we don't really have any reason to disallow it. So in that
case it does make a difference. We could just do:
@@ -1183,7 +1183,8 @@ int utrace_control(struct task_struct *target,
clear_engine_wants_stop(engine);
if (action utrace-resume) {
utrace-resume = action;
- set_notify_resume(target);
+ if (reset)
+ set_notify_resume(target);
}
break;
Since TIF_NOTIFY_RESUME will now always be superfluous when stopped.
It is not clear to me why apply_resume_action(UTRACE_INTERRUPT) does
set_tsk_thread_flag(TIF_SIGPENDING). In fact I don't understand why
apply_resume_action() checks UTRACE_INTERRUPT at all. If it is called
after utrace_stop(), action == start_report() which never resets
UTRACE_INTERRUPT.
It's only because this code path is shared with the tail of
finish_resume_report, where it is the only thing processing
UTRACE_INTERRUPT in the real return-to-user cases. i.e., in paths that
don't call finish_report(), TIF_SIGPENDING won't ever have been set by a
callback return value. utrace_control does always set it up front, so it
is superfluous when that's how UTRACE_INTERRUPT got set.
And since we process -resume after stop, it is not clear why we
set -resume = report-resume_action before stop, apply_resume_action()
could use min(report-resume_action, utrace-resume). Yes, yes,
we have the nasty utrace-vfork_stop case, I mean it is not easy to
understand the logic behind all these -resume changes.
Ok. Feel free to clean it up if you think it makes things clearer. To me,
it's just natural to do that MIN operation as soon as you know about it.
utrace_stop() always takes the lock anyway, so it's relatively free. If
the ambient state is stored early, then e.g. utrace_control() won't bother
to set TIF_SIGPENDING or TIF_NOTIFY_RESUME.
I guess I think the logic is simple: apply a new minimum to -resume as
soon as you know about it.
And afaics, this can't help to fix the tracehook_report_syscall_exit()
TIF_SINGLESTEP problems in the multitracing case, UTRACE_INTERRUPT
destroys UTRACE_SINGLESTEP. Ptrace can reassert it later, but this
will be too late, the trace has already passed this tracehook.
See the other thread, but what I said there is let's take this case up in
its own thread later.
I don't understand this skip_notify, utrace_stop() is always called
with skip_notify == true?
Hmm. Yes, this patch was unfinished when I sent it because your patches
were crossing paths with what I was doing. We can skip TIF_NOTIFY_RESUME
when we'll always call apply_resume_action() after utrace_stop() before
user mode (i.e. report_exit doesn't matter).
Since report_exit doesn't matter one way or the other, perhaps it will be
cleaner to roll apply_resume_action() into utrace_stop() in some fashion.
I notice we're now actually using the REPORT() macro only four times, and
one of those is report_exit. So maybe that and finish_report() could
change somehow too to refactor things. I'll leave it to you to rework all
those and finish_* to the most useful organization of subroutines.
I think we're mutually clear now on the idea of when to do user_*_step()
calls (i.e. apply_resume_action).
Thanks,
Roland
