Re: [PATCH 12/13] tracing/uprobes: Add more fetch functions
Hi Steve,
On Mon, 4 Nov 2013 12:17:06 -0500, Steven Rostedt wrote:
> On Mon, 4 Nov 2013 17:44:31 +0100
> Oleg Nesterov wrote:
>
>> On 11/04, Namhyung Kim wrote:
>> >
>> > On Thu, 31 Oct 2013 19:22:18 +0100, Oleg Nesterov wrote:
>> > > On 10/29, Namhyung Kim wrote:
>> > >>
>> > >> +static void __user *get_user_vaddr(unsigned long addr, struct
>> > >> trace_uprobe *tu)
>> > >> +{
>> > >> + unsigned long pgoff = addr >> PAGE_SHIFT;
>> > >> + struct vm_area_struct *vma;
>> > >> + struct address_space *mapping;
>> > >> + unsigned long vaddr = 0;
>> > >> +
>> > >> + if (tu == NULL) {
>> > >> + /* A NULL tu means that we already got the vaddr */
>> > >> + return (void __force __user *) addr;
>> > >> + }
>> > >> +
>> > >> + mapping = tu->inode->i_mapping;
>> > >> +
>> > >> + mutex_lock(&mapping->i_mmap_mutex);
>> > >> + vma_interval_tree_foreach(vma, &mapping->i_mmap, pgoff, pgoff) {
>> > >> + if (vma->vm_mm != current->mm)
>> > >> + continue;
>> > >> + if (!(vma->vm_flags & VM_READ))
>> > >> + continue;
>> > >> +
>> > >> + vaddr = offset_to_vaddr(vma, addr);
>> > >> + break;
>> > >> + }
>> > >> + mutex_unlock(&mapping->i_mmap_mutex);
>> > >> +
>> > >> + WARN_ON_ONCE(vaddr == 0);
>> > >
>> > > Hmm. But unless I missed something this "addr" passed as an argument can
>> > > be wrong? And if nothing else this or another thread can unmap the vma?
>> >
>> > You mean WARN_ON_ONCE here is superfluous? I admit that it should
>> > protect concurrent vma [un]mappings. Please see my reply in other
>> > thread for a new approach.
>>
>> Whatever we do this address can be unmapped. For example, just because of
>> @invalid_address passed to trace_uprobe.c.
>>
>> We do not really care, copy_from_user() should fail. But we should not
>> WARN() in this case.
>>
>
> I agree, the WARN_ON_ONCE() above looks like it's uncalled for.
> WARN()ings should only be used when an anomaly in the kernel logic is
> detected. Can this trigger on bad input from user space, or something
> else that userspace does? (a race with unmapping memory?). If so, error
> out to the user process, but do not call any of the WARN() functions.
Will do. Thanks for the explanation.
Thanks,
Namhyung
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 12/13] tracing/uprobes: Add more fetch functions
On Mon, 4 Nov 2013 17:44:31 +0100, Oleg Nesterov wrote:
> On 11/04, Namhyung Kim wrote:
>>
>> On Thu, 31 Oct 2013 19:22:18 +0100, Oleg Nesterov wrote:
>> > On 10/29, Namhyung Kim wrote:
>> >>
>> >> +static void __user *get_user_vaddr(unsigned long addr, struct
>> >> trace_uprobe *tu)
>> >> +{
>> >> + unsigned long pgoff = addr >> PAGE_SHIFT;
>> >> + struct vm_area_struct *vma;
>> >> + struct address_space *mapping;
>> >> + unsigned long vaddr = 0;
>> >> +
>> >> + if (tu == NULL) {
>> >> + /* A NULL tu means that we already got the vaddr */
>> >> + return (void __force __user *) addr;
>> >> + }
>> >> +
>> >> + mapping = tu->inode->i_mapping;
>> >> +
>> >> + mutex_lock(&mapping->i_mmap_mutex);
>> >> + vma_interval_tree_foreach(vma, &mapping->i_mmap, pgoff, pgoff) {
>> >> + if (vma->vm_mm != current->mm)
>> >> + continue;
>> >> + if (!(vma->vm_flags & VM_READ))
>> >> + continue;
>> >> +
>> >> + vaddr = offset_to_vaddr(vma, addr);
>> >> + break;
>> >> + }
>> >> + mutex_unlock(&mapping->i_mmap_mutex);
>> >> +
>> >> + WARN_ON_ONCE(vaddr == 0);
>> >
>> > Hmm. But unless I missed something this "addr" passed as an argument can
>> > be wrong? And if nothing else this or another thread can unmap the vma?
>>
>> You mean WARN_ON_ONCE here is superfluous? I admit that it should
>> protect concurrent vma [un]mappings. Please see my reply in other
>> thread for a new approach.
>
> Whatever we do this address can be unmapped. For example, just because of
> @invalid_address passed to trace_uprobe.c.
>
> We do not really care, copy_from_user() should fail. But we should not
> WARN() in this case.
Okay, I see. Will remove it in the next spin.
Thanks,
Namhyung
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 12/13] tracing/uprobes: Add more fetch functions
On Mon, 4 Nov 2013 17:44:31 +0100
Oleg Nesterov wrote:
> On 11/04, Namhyung Kim wrote:
> >
> > On Thu, 31 Oct 2013 19:22:18 +0100, Oleg Nesterov wrote:
> > > On 10/29, Namhyung Kim wrote:
> > >>
> > >> +static void __user *get_user_vaddr(unsigned long addr, struct
> > >> trace_uprobe *tu)
> > >> +{
> > >> +unsigned long pgoff = addr >> PAGE_SHIFT;
> > >> +struct vm_area_struct *vma;
> > >> +struct address_space *mapping;
> > >> +unsigned long vaddr = 0;
> > >> +
> > >> +if (tu == NULL) {
> > >> +/* A NULL tu means that we already got the vaddr */
> > >> +return (void __force __user *) addr;
> > >> +}
> > >> +
> > >> +mapping = tu->inode->i_mapping;
> > >> +
> > >> +mutex_lock(&mapping->i_mmap_mutex);
> > >> +vma_interval_tree_foreach(vma, &mapping->i_mmap, pgoff, pgoff) {
> > >> +if (vma->vm_mm != current->mm)
> > >> +continue;
> > >> +if (!(vma->vm_flags & VM_READ))
> > >> +continue;
> > >> +
> > >> +vaddr = offset_to_vaddr(vma, addr);
> > >> +break;
> > >> +}
> > >> +mutex_unlock(&mapping->i_mmap_mutex);
> > >> +
> > >> +WARN_ON_ONCE(vaddr == 0);
> > >
> > > Hmm. But unless I missed something this "addr" passed as an argument can
> > > be wrong? And if nothing else this or another thread can unmap the vma?
> >
> > You mean WARN_ON_ONCE here is superfluous? I admit that it should
> > protect concurrent vma [un]mappings. Please see my reply in other
> > thread for a new approach.
>
> Whatever we do this address can be unmapped. For example, just because of
> @invalid_address passed to trace_uprobe.c.
>
> We do not really care, copy_from_user() should fail. But we should not
> WARN() in this case.
>
I agree, the WARN_ON_ONCE() above looks like it's uncalled for.
WARN()ings should only be used when an anomaly in the kernel logic is
detected. Can this trigger on bad input from user space, or something
else that userspace does? (a race with unmapping memory?). If so, error
out to the user process, but do not call any of the WARN() functions.
-- Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 12/13] tracing/uprobes: Add more fetch functions
On 11/04, Namhyung Kim wrote:
>
> On Thu, 31 Oct 2013 19:22:18 +0100, Oleg Nesterov wrote:
> > On 10/29, Namhyung Kim wrote:
> >>
> >> +static void __user *get_user_vaddr(unsigned long addr, struct
> >> trace_uprobe *tu)
> >> +{
> >> + unsigned long pgoff = addr >> PAGE_SHIFT;
> >> + struct vm_area_struct *vma;
> >> + struct address_space *mapping;
> >> + unsigned long vaddr = 0;
> >> +
> >> + if (tu == NULL) {
> >> + /* A NULL tu means that we already got the vaddr */
> >> + return (void __force __user *) addr;
> >> + }
> >> +
> >> + mapping = tu->inode->i_mapping;
> >> +
> >> + mutex_lock(&mapping->i_mmap_mutex);
> >> + vma_interval_tree_foreach(vma, &mapping->i_mmap, pgoff, pgoff) {
> >> + if (vma->vm_mm != current->mm)
> >> + continue;
> >> + if (!(vma->vm_flags & VM_READ))
> >> + continue;
> >> +
> >> + vaddr = offset_to_vaddr(vma, addr);
> >> + break;
> >> + }
> >> + mutex_unlock(&mapping->i_mmap_mutex);
> >> +
> >> + WARN_ON_ONCE(vaddr == 0);
> >
> > Hmm. But unless I missed something this "addr" passed as an argument can
> > be wrong? And if nothing else this or another thread can unmap the vma?
>
> You mean WARN_ON_ONCE here is superfluous? I admit that it should
> protect concurrent vma [un]mappings. Please see my reply in other
> thread for a new approach.
Whatever we do this address can be unmapped. For example, just because of
@invalid_address passed to trace_uprobe.c.
We do not really care, copy_from_user() should fail. But we should not
WARN() in this case.
Oleg.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 12/13] tracing/uprobes: Add more fetch functions
On Thu, 31 Oct 2013 19:22:18 +0100, Oleg Nesterov wrote:
> On 10/29, Namhyung Kim wrote:
>>
>> +static void __user *get_user_vaddr(unsigned long addr, struct trace_uprobe
>> *tu)
>> +{
>> +unsigned long pgoff = addr >> PAGE_SHIFT;
>> +struct vm_area_struct *vma;
>> +struct address_space *mapping;
>> +unsigned long vaddr = 0;
>> +
>> +if (tu == NULL) {
>> +/* A NULL tu means that we already got the vaddr */
>> +return (void __force __user *) addr;
>> +}
>> +
>> +mapping = tu->inode->i_mapping;
>> +
>> +mutex_lock(&mapping->i_mmap_mutex);
>> +vma_interval_tree_foreach(vma, &mapping->i_mmap, pgoff, pgoff) {
>> +if (vma->vm_mm != current->mm)
>> +continue;
>> +if (!(vma->vm_flags & VM_READ))
>> +continue;
>> +
>> +vaddr = offset_to_vaddr(vma, addr);
>> +break;
>> +}
>> +mutex_unlock(&mapping->i_mmap_mutex);
>> +
>> +WARN_ON_ONCE(vaddr == 0);
>
> Hmm. But unless I missed something this "addr" passed as an argument can
> be wrong? And if nothing else this or another thread can unmap the vma?
You mean WARN_ON_ONCE here is superfluous? I admit that it should
protect concurrent vma [un]mappings. Please see my reply in other
thread for a new approach.
Thanks,
Namhyung
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 12/13] tracing/uprobes: Add more fetch functions
On 10/29, Namhyung Kim wrote:
>
> +static unsigned long get_user_stack_nth(struct pt_regs *regs, unsigned int n)
> +{
> + struct vm_area_struct *vma;
> + unsigned long addr = user_stack_pointer(regs);
> + bool valid = false;
> + unsigned long ret = 0;
> +
> + down_read(¤t->mm->mmap_sem);
> + vma = find_vma(current->mm, addr);
> + if (vma && vma->vm_start <= addr) {
> + if (within_user_stack(vma, addr, n))
> + valid = true;
> + }
> + up_read(¤t->mm->mmap_sem);
> +
> + addr = adjust_stack_addr(addr, n);
> +
> + if (valid && copy_from_user(&ret, (void __force __user *)addr,
> + sizeof(ret)) == 0)
> + return ret;
> + return 0;
> +}
Namhyung, I am just curious, why do we need find_vma/within_user_stack?
copy_from_user() should fail or expand the stack. Yes, we can actually
look into the wrong vma, but do we really care?
> +static void __user *get_user_vaddr(unsigned long addr, struct trace_uprobe
> *tu)
> +{
> + unsigned long pgoff = addr >> PAGE_SHIFT;
> + struct vm_area_struct *vma;
> + struct address_space *mapping;
> + unsigned long vaddr = 0;
> +
> + if (tu == NULL) {
> + /* A NULL tu means that we already got the vaddr */
> + return (void __force __user *) addr;
> + }
> +
> + mapping = tu->inode->i_mapping;
> +
> + mutex_lock(&mapping->i_mmap_mutex);
> + vma_interval_tree_foreach(vma, &mapping->i_mmap, pgoff, pgoff) {
> + if (vma->vm_mm != current->mm)
> + continue;
> + if (!(vma->vm_flags & VM_READ))
> + continue;
> +
> + vaddr = offset_to_vaddr(vma, addr);
> + break;
> + }
> + mutex_unlock(&mapping->i_mmap_mutex);
> +
> + WARN_ON_ONCE(vaddr == 0);
> + return (void __force __user *) vaddr;
So. If I understand correctly, @addr cat only read the memory mmaped
to the probed binary, and we need to translate the address... And in
general we can't read the data from bss.
Right?
I'll probably ask another question about this later...
> +static __kprobes void FETCH_FUNC_NAME(memory, string)(struct pt_regs *regs,
> + void *addr, void *dest, void *priv)
> +{
> + long ret;
> + u32 rloc = *(u32 *)dest;
> + int maxlen = get_rloc_len(rloc);
> + u8 *dst = get_rloc_data(dest);
> + void __user *vaddr = get_user_vaddr((unsigned long)addr, priv);
> + void __user *src = vaddr;
> +
> + if (!maxlen)
> + return;
> +
> + do {
> + ret = copy_from_user(dst, src, sizeof(*dst));
> + dst++;
> + src++;
> + } while (dst[-1] && ret == 0 && (src - vaddr) < maxlen);
Can't we use strncpy_from_user() ?
> +static __kprobes void FETCH_FUNC_NAME(memory, string_size)(struct pt_regs
> *regs,
> +void *addr, void *dest, void *priv)
> +{
> + int ret, len = 0;
> + u8 c;
> + void __user *vaddr = get_user_vaddr((unsigned long)addr, priv);
> +
> + do {
> + ret = __copy_from_user_inatomic(&c, vaddr + len, 1);
Hmm. I guess I need to actually apply this series ;)
Why inatomic? it seems that this is for uprobes, no? And perhaps
strnlen_user() should work just fine?
Oleg.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 12/13] tracing/uprobes: Add more fetch functions
On 10/29, Namhyung Kim wrote:
>
> +static void __user *get_user_vaddr(unsigned long addr, struct trace_uprobe
> *tu)
> +{
> + unsigned long pgoff = addr >> PAGE_SHIFT;
> + struct vm_area_struct *vma;
> + struct address_space *mapping;
> + unsigned long vaddr = 0;
> +
> + if (tu == NULL) {
> + /* A NULL tu means that we already got the vaddr */
> + return (void __force __user *) addr;
> + }
> +
> + mapping = tu->inode->i_mapping;
> +
> + mutex_lock(&mapping->i_mmap_mutex);
> + vma_interval_tree_foreach(vma, &mapping->i_mmap, pgoff, pgoff) {
> + if (vma->vm_mm != current->mm)
> + continue;
> + if (!(vma->vm_flags & VM_READ))
> + continue;
> +
> + vaddr = offset_to_vaddr(vma, addr);
> + break;
> + }
> + mutex_unlock(&mapping->i_mmap_mutex);
> +
> + WARN_ON_ONCE(vaddr == 0);
Hmm. But unless I missed something this "addr" passed as an argument can
be wrong? And if nothing else this or another thread can unmap the vma?
Oleg.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 12/13] tracing/uprobes: Add more fetch functions
From: Namhyung Kim
Implement uprobe-specific stack and memory fetch functions and add
them to the uprobes_fetch_type_table. Other fetch fucntions will be
shared with kprobes.
Original-patch-by: Hyeoncheol Lee
Reviewed-by: Masami Hiramatsu
Cc: Srikar Dronamraju
Cc: Oleg Nesterov
Cc: zhangwei(Jovi)
Cc: Arnaldo Carvalho de Melo
Signed-off-by: Namhyung Kim
---
kernel/trace/trace_probe.c | 9 ++-
kernel/trace/trace_probe.h | 1 +
kernel/trace/trace_uprobe.c | 188 +++-
3 files changed, 192 insertions(+), 6 deletions(-)
diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index eaee44d5d9d1..70cd3bfde5a6 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -101,6 +101,10 @@ struct deref_fetch_param {
fetch_func_tfetch_size;
};
+/*
+ * For uprobes, it'll get a vaddr from first call_fetch() so pass NULL
+ * as a priv on the second dprm->fetch() not to translate it to vaddr again.
+ */
#define DEFINE_FETCH_deref(type) \
__kprobes void FETCH_FUNC_NAME(deref, type)(struct pt_regs *regs, \
void *data, void *dest, void *priv) \
@@ -110,13 +114,14 @@ __kprobes void FETCH_FUNC_NAME(deref, type)(struct
pt_regs *regs, \
call_fetch(&dprm->orig, regs, &addr, priv); \
if (addr) { \
addr += dprm->offset; \
- dprm->fetch(regs, (void *)addr, dest, priv);\
+ dprm->fetch(regs, (void *)addr, dest, NULL);\
} else \
*(type *)dest = 0; \
}
DEFINE_BASIC_FETCH_FUNCS(deref)
DEFINE_FETCH_deref(string)
+/* Same as above */
__kprobes void FETCH_FUNC_NAME(deref, string_size)(struct pt_regs *regs,
void *data, void *dest, void *priv)
{
@@ -126,7 +131,7 @@ __kprobes void FETCH_FUNC_NAME(deref, string_size)(struct
pt_regs *regs,
call_fetch(&dprm->orig, regs, &addr, priv);
if (addr && dprm->fetch_size) {
addr += dprm->offset;
- dprm->fetch_size(regs, (void *)addr, dest, priv);
+ dprm->fetch_size(regs, (void *)addr, dest, NULL);
} else
*(string_size *)dest = 0;
}
diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
index fc7edf3749ef..b1e7d722c354 100644
--- a/kernel/trace/trace_probe.h
+++ b/kernel/trace/trace_probe.h
@@ -263,6 +263,7 @@ ASSIGN_FETCH_FUNC(bitfield, ftype), \
#define NR_FETCH_TYPES 10
extern const struct fetch_type kprobes_fetch_type_table[];
+extern const struct fetch_type uprobes_fetch_type_table[];
static inline __kprobes void call_fetch(struct fetch_param *fprm,
struct pt_regs *regs, void *dest, void *priv)
diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index b41e11621aed..b7664f6eb356 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -530,6 +530,186 @@ static const struct file_operations uprobe_profile_ops = {
.release= seq_release,
};
+#ifdef CONFIG_STACK_GROWSUP
+static unsigned long adjust_stack_addr(unsigned long addr, unsigned n)
+{
+ return addr - (n * sizeof(long));
+}
+
+static bool within_user_stack(struct vm_area_struct *vma, unsigned long addr,
+ unsigned int n)
+{
+ return vma->vm_start <= adjust_stack_addr(addr, n);
+}
+#else
+static unsigned long adjust_stack_addr(unsigned long addr, unsigned n)
+{
+ return addr + (n * sizeof(long));
+}
+
+static bool within_user_stack(struct vm_area_struct *vma, unsigned long addr,
+ unsigned int n)
+{
+ return vma->vm_end >= adjust_stack_addr(addr, n);
+}
+#endif
+
+static unsigned long get_user_stack_nth(struct pt_regs *regs, unsigned int n)
+{
+ struct vm_area_struct *vma;
+ unsigned long addr = user_stack_pointer(regs);
+ bool valid = false;
+ unsigned long ret = 0;
+
+ down_read(¤t->mm->mmap_sem);
+ vma = find_vma(current->mm, addr);
+ if (vma && vma->vm_start <= addr) {
+ if (within_user_stack(vma, addr, n))
+ valid = true;
+ }
+ up_read(¤t->mm->mmap_sem);
+
+ addr = adjust_stack_addr(addr, n);
+
+ if (valid && copy_from_user(&ret, (void __force __user *)addr,
+ sizeof(ret)) == 0)
+ return ret;
+ return 0;
+}
+
+static unsigned long offset_to_vaddr(struct vm_area_struct *vma,
+unsigned long offset)
+{
+ return vma->vm_start + offset - ((loff_t)vma->vm_pgoff << PAGE_SHIFT);
+}
+
+static void __user *get_user_vaddr(unsign
[PATCH 12/13] tracing/uprobes: Add more fetch functions
From: Namhyung Kim
Implement uprobe-specific stack and memory fetch functions and add
them to the uprobes_fetch_type_table. Other fetch fucntions will be
shared with kprobes.
Original-patch-by: Hyeoncheol Lee
Reviewed-by: Masami Hiramatsu
Cc: Srikar Dronamraju
Cc: Oleg Nesterov
Cc: zhangwei(Jovi)
Cc: Arnaldo Carvalho de Melo
Signed-off-by: Namhyung Kim
---
kernel/trace/trace_probe.c | 9 ++-
kernel/trace/trace_probe.h | 1 +
kernel/trace/trace_uprobe.c | 188 +++-
3 files changed, 192 insertions(+), 6 deletions(-)
diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index eaee44d5d9d1..70cd3bfde5a6 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -101,6 +101,10 @@ struct deref_fetch_param {
fetch_func_tfetch_size;
};
+/*
+ * For uprobes, it'll get a vaddr from first call_fetch() so pass NULL
+ * as a priv on the second dprm->fetch() not to translate it to vaddr again.
+ */
#define DEFINE_FETCH_deref(type) \
__kprobes void FETCH_FUNC_NAME(deref, type)(struct pt_regs *regs, \
void *data, void *dest, void *priv) \
@@ -110,13 +114,14 @@ __kprobes void FETCH_FUNC_NAME(deref, type)(struct
pt_regs *regs, \
call_fetch(&dprm->orig, regs, &addr, priv); \
if (addr) { \
addr += dprm->offset; \
- dprm->fetch(regs, (void *)addr, dest, priv);\
+ dprm->fetch(regs, (void *)addr, dest, NULL);\
} else \
*(type *)dest = 0; \
}
DEFINE_BASIC_FETCH_FUNCS(deref)
DEFINE_FETCH_deref(string)
+/* Same as above */
__kprobes void FETCH_FUNC_NAME(deref, string_size)(struct pt_regs *regs,
void *data, void *dest, void *priv)
{
@@ -126,7 +131,7 @@ __kprobes void FETCH_FUNC_NAME(deref, string_size)(struct
pt_regs *regs,
call_fetch(&dprm->orig, regs, &addr, priv);
if (addr && dprm->fetch_size) {
addr += dprm->offset;
- dprm->fetch_size(regs, (void *)addr, dest, priv);
+ dprm->fetch_size(regs, (void *)addr, dest, NULL);
} else
*(string_size *)dest = 0;
}
diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
index fc7edf3749ef..b1e7d722c354 100644
--- a/kernel/trace/trace_probe.h
+++ b/kernel/trace/trace_probe.h
@@ -263,6 +263,7 @@ ASSIGN_FETCH_FUNC(bitfield, ftype), \
#define NR_FETCH_TYPES 10
extern const struct fetch_type kprobes_fetch_type_table[];
+extern const struct fetch_type uprobes_fetch_type_table[];
static inline __kprobes void call_fetch(struct fetch_param *fprm,
struct pt_regs *regs, void *dest, void *priv)
diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index fc5f8aa62156..1b778bbf5c70 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -530,6 +530,186 @@ static const struct file_operations uprobe_profile_ops = {
.release= seq_release,
};
+#ifdef CONFIG_STACK_GROWSUP
+static unsigned long adjust_stack_addr(unsigned long addr, unsigned n)
+{
+ return addr - (n * sizeof(long));
+}
+
+static bool within_user_stack(struct vm_area_struct *vma, unsigned long addr,
+ unsigned int n)
+{
+ return vma->vm_start <= adjust_stack_addr(addr, n);
+}
+#else
+static unsigned long adjust_stack_addr(unsigned long addr, unsigned n)
+{
+ return addr + (n * sizeof(long));
+}
+
+static bool within_user_stack(struct vm_area_struct *vma, unsigned long addr,
+ unsigned int n)
+{
+ return vma->vm_end >= adjust_stack_addr(addr, n);
+}
+#endif
+
+static unsigned long get_user_stack_nth(struct pt_regs *regs, unsigned int n)
+{
+ struct vm_area_struct *vma;
+ unsigned long addr = user_stack_pointer(regs);
+ bool valid = false;
+ unsigned long ret = 0;
+
+ down_read(¤t->mm->mmap_sem);
+ vma = find_vma(current->mm, addr);
+ if (vma && vma->vm_start <= addr) {
+ if (within_user_stack(vma, addr, n))
+ valid = true;
+ }
+ up_read(¤t->mm->mmap_sem);
+
+ addr = adjust_stack_addr(addr, n);
+
+ if (valid && copy_from_user(&ret, (void __force __user *)addr,
+ sizeof(ret)) == 0)
+ return ret;
+ return 0;
+}
+
+static unsigned long offset_to_vaddr(struct vm_area_struct *vma,
+unsigned long offset)
+{
+ return vma->vm_start + offset - ((loff_t)vma->vm_pgoff << PAGE_SHIFT);
+}
+
+static void __user *get_user_vaddr(unsign
Re: [PATCH 12/13] tracing/uprobes: Add more fetch functions
Hi Masami,
On Tue, 27 Aug 2013 21:03:32 +0900, Masami Hiramatsu wrote:
> (2013/08/27 17:48), Namhyung Kim wrote:
>> From: Namhyung Kim
>>
>> Implement uprobe-specific stack and memory fetch functions and add
>> them to the uprobes_fetch_type_table. Other fetch fucntions will be
>> shared with kprobes.
[SNIP]
>> +
>> +static unsigned long get_user_stack_nth(struct pt_regs *regs, unsigned int
>> n)
>> +{
>> +struct vm_area_struct *vma;
>> +unsigned long addr = GET_USP(regs);
>
> I recommend you to use user_stack_pointer() rather than GET_USP here.
> Since some arch seem not to have correct GET_USP implementation (e.g. arm),
> kernel build will be failed.
Okay, I'll change to the user_stack_pointer().
>
> And at least trace_probe.c part is good for me.
>
> Reviewed-by: Masami Hiramatsu
Thanks for your review!
Namhyung
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 12/13] tracing/uprobes: Add more fetch functions
(2013/08/27 17:48), Namhyung Kim wrote:
> From: Namhyung Kim
>
> Implement uprobe-specific stack and memory fetch functions and add
> them to the uprobes_fetch_type_table. Other fetch fucntions will be
> shared with kprobes.
>
> Original-patch-by: Hyeoncheol Lee
> Cc: Masami Hiramatsu
> Cc: Srikar Dronamraju
> Cc: Oleg Nesterov
> Cc: zhangwei(Jovi)
> Cc: Arnaldo Carvalho de Melo
> Signed-off-by: Namhyung Kim
> ---
> kernel/trace/trace_probe.c | 9 ++-
> kernel/trace/trace_probe.h | 1 +
> kernel/trace/trace_uprobe.c | 188
> +++-
> 3 files changed, 192 insertions(+), 6 deletions(-)
>
> diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
> index eaee44d5d9d1..70cd3bfde5a6 100644
> --- a/kernel/trace/trace_probe.c
> +++ b/kernel/trace/trace_probe.c
> @@ -101,6 +101,10 @@ struct deref_fetch_param {
> fetch_func_tfetch_size;
> };
>
> +/*
> + * For uprobes, it'll get a vaddr from first call_fetch() so pass NULL
> + * as a priv on the second dprm->fetch() not to translate it to vaddr again.
> + */
> #define DEFINE_FETCH_deref(type) \
> __kprobes void FETCH_FUNC_NAME(deref, type)(struct pt_regs *regs,\
> void *data, void *dest, void *priv) \
> @@ -110,13 +114,14 @@ __kprobes void FETCH_FUNC_NAME(deref, type)(struct
> pt_regs *regs, \
> call_fetch(&dprm->orig, regs, &addr, priv); \
> if (addr) { \
> addr += dprm->offset; \
> - dprm->fetch(regs, (void *)addr, dest, priv);\
> + dprm->fetch(regs, (void *)addr, dest, NULL);\
> } else \
> *(type *)dest = 0; \
> }
> DEFINE_BASIC_FETCH_FUNCS(deref)
> DEFINE_FETCH_deref(string)
>
> +/* Same as above */
> __kprobes void FETCH_FUNC_NAME(deref, string_size)(struct pt_regs *regs,
> void *data, void *dest, void *priv)
> {
> @@ -126,7 +131,7 @@ __kprobes void FETCH_FUNC_NAME(deref, string_size)(struct
> pt_regs *regs,
> call_fetch(&dprm->orig, regs, &addr, priv);
> if (addr && dprm->fetch_size) {
> addr += dprm->offset;
> - dprm->fetch_size(regs, (void *)addr, dest, priv);
> + dprm->fetch_size(regs, (void *)addr, dest, NULL);
> } else
> *(string_size *)dest = 0;
> }
> diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
> index fc7edf3749ef..b1e7d722c354 100644
> --- a/kernel/trace/trace_probe.h
> +++ b/kernel/trace/trace_probe.h
> @@ -263,6 +263,7 @@ ASSIGN_FETCH_FUNC(bitfield, ftype),
> \
> #define NR_FETCH_TYPES 10
>
> extern const struct fetch_type kprobes_fetch_type_table[];
> +extern const struct fetch_type uprobes_fetch_type_table[];
>
> static inline __kprobes void call_fetch(struct fetch_param *fprm,
>struct pt_regs *regs, void *dest, void *priv)
> diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
> index fc5f8aa62156..89d4b86abbe1 100644
> --- a/kernel/trace/trace_uprobe.c
> +++ b/kernel/trace/trace_uprobe.c
> @@ -530,6 +530,186 @@ static const struct file_operations uprobe_profile_ops
> = {
> .release= seq_release,
> };
>
> +#ifdef CONFIG_STACK_GROWSUP
> +static unsigned long adjust_stack_addr(unsigned long addr, unsigned n)
> +{
> + return addr - (n * sizeof(long));
> +}
> +
> +static bool within_user_stack(struct vm_area_struct *vma, unsigned long addr,
> + unsigned int n)
> +{
> + return vma->vm_start <= adjust_stack_addr(addr, n);
> +}
> +#else
> +static unsigned long adjust_stack_addr(unsigned long addr, unsigned n)
> +{
> + return addr + (n * sizeof(long));
> +}
> +
> +static bool within_user_stack(struct vm_area_struct *vma, unsigned long addr,
> + unsigned int n)
> +{
> + return vma->vm_end >= adjust_stack_addr(addr, n);
> +}
> +#endif
> +
> +static unsigned long get_user_stack_nth(struct pt_regs *regs, unsigned int n)
> +{
> + struct vm_area_struct *vma;
> + unsigned long addr = GET_USP(regs);
I recommend you to use user_stack_pointer() rather than GET_USP here.
Since some arch seem not to have correct GET_USP implementation (e.g. arm),
kernel build will be failed.
And at least trace_probe.c part is good for me.
Reviewed-by: Masami Hiramatsu
Thank you,
--
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu...@hitachi.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vg
[PATCH 12/13] tracing/uprobes: Add more fetch functions
From: Namhyung Kim
Implement uprobe-specific stack and memory fetch functions and add
them to the uprobes_fetch_type_table. Other fetch fucntions will be
shared with kprobes.
Original-patch-by: Hyeoncheol Lee
Cc: Masami Hiramatsu
Cc: Srikar Dronamraju
Cc: Oleg Nesterov
Cc: zhangwei(Jovi)
Cc: Arnaldo Carvalho de Melo
Signed-off-by: Namhyung Kim
---
kernel/trace/trace_probe.c | 9 ++-
kernel/trace/trace_probe.h | 1 +
kernel/trace/trace_uprobe.c | 188 +++-
3 files changed, 192 insertions(+), 6 deletions(-)
diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index eaee44d5d9d1..70cd3bfde5a6 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -101,6 +101,10 @@ struct deref_fetch_param {
fetch_func_tfetch_size;
};
+/*
+ * For uprobes, it'll get a vaddr from first call_fetch() so pass NULL
+ * as a priv on the second dprm->fetch() not to translate it to vaddr again.
+ */
#define DEFINE_FETCH_deref(type) \
__kprobes void FETCH_FUNC_NAME(deref, type)(struct pt_regs *regs, \
void *data, void *dest, void *priv) \
@@ -110,13 +114,14 @@ __kprobes void FETCH_FUNC_NAME(deref, type)(struct
pt_regs *regs, \
call_fetch(&dprm->orig, regs, &addr, priv); \
if (addr) { \
addr += dprm->offset; \
- dprm->fetch(regs, (void *)addr, dest, priv);\
+ dprm->fetch(regs, (void *)addr, dest, NULL);\
} else \
*(type *)dest = 0; \
}
DEFINE_BASIC_FETCH_FUNCS(deref)
DEFINE_FETCH_deref(string)
+/* Same as above */
__kprobes void FETCH_FUNC_NAME(deref, string_size)(struct pt_regs *regs,
void *data, void *dest, void *priv)
{
@@ -126,7 +131,7 @@ __kprobes void FETCH_FUNC_NAME(deref, string_size)(struct
pt_regs *regs,
call_fetch(&dprm->orig, regs, &addr, priv);
if (addr && dprm->fetch_size) {
addr += dprm->offset;
- dprm->fetch_size(regs, (void *)addr, dest, priv);
+ dprm->fetch_size(regs, (void *)addr, dest, NULL);
} else
*(string_size *)dest = 0;
}
diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
index fc7edf3749ef..b1e7d722c354 100644
--- a/kernel/trace/trace_probe.h
+++ b/kernel/trace/trace_probe.h
@@ -263,6 +263,7 @@ ASSIGN_FETCH_FUNC(bitfield, ftype), \
#define NR_FETCH_TYPES 10
extern const struct fetch_type kprobes_fetch_type_table[];
+extern const struct fetch_type uprobes_fetch_type_table[];
static inline __kprobes void call_fetch(struct fetch_param *fprm,
struct pt_regs *regs, void *dest, void *priv)
diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index fc5f8aa62156..89d4b86abbe1 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -530,6 +530,186 @@ static const struct file_operations uprobe_profile_ops = {
.release= seq_release,
};
+#ifdef CONFIG_STACK_GROWSUP
+static unsigned long adjust_stack_addr(unsigned long addr, unsigned n)
+{
+ return addr - (n * sizeof(long));
+}
+
+static bool within_user_stack(struct vm_area_struct *vma, unsigned long addr,
+ unsigned int n)
+{
+ return vma->vm_start <= adjust_stack_addr(addr, n);
+}
+#else
+static unsigned long adjust_stack_addr(unsigned long addr, unsigned n)
+{
+ return addr + (n * sizeof(long));
+}
+
+static bool within_user_stack(struct vm_area_struct *vma, unsigned long addr,
+ unsigned int n)
+{
+ return vma->vm_end >= adjust_stack_addr(addr, n);
+}
+#endif
+
+static unsigned long get_user_stack_nth(struct pt_regs *regs, unsigned int n)
+{
+ struct vm_area_struct *vma;
+ unsigned long addr = GET_USP(regs);
+ bool valid = false;
+ unsigned long ret = 0;
+
+ down_read(¤t->mm->mmap_sem);
+ vma = find_vma(current->mm, addr);
+ if (vma && vma->vm_start <= addr) {
+ if (within_user_stack(vma, addr, n))
+ valid = true;
+ }
+ up_read(¤t->mm->mmap_sem);
+
+ addr = adjust_stack_addr(addr, n);
+
+ if (valid && copy_from_user(&ret, (void __force __user *)addr,
+ sizeof(ret)) == 0)
+ return ret;
+ return 0;
+}
+
+static unsigned long offset_to_vaddr(struct vm_area_struct *vma,
+unsigned long offset)
+{
+ return vma->vm_start + offset - ((loff_t)vma->vm_pgoff << PAGE_SHIFT);
+}
+
+static void __user *get_user_vaddr(unsigned long addr, struct
[PATCH 12/13] tracing/uprobes: Add more fetch functions
From: Namhyung Kim
Implement uprobe-specific stack and memory fetch functions and add
them to the uprobes_fetch_type_table. Other fetch fucntions will be
shared with kprobes.
Original-patch-by: Hyeoncheol Lee
Cc: Masami Hiramatsu
Cc: Srikar Dronamraju
Cc: Oleg Nesterov
Cc: zhangwei(Jovi)
Cc: Arnaldo Carvalho de Melo
Signed-off-by: Namhyung Kim
---
kernel/trace/trace_probe.c | 9 ++-
kernel/trace/trace_probe.h | 1 +
kernel/trace/trace_uprobe.c | 188 +++-
3 files changed, 192 insertions(+), 6 deletions(-)
diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index eaee44d5d9d1..70cd3bfde5a6 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -101,6 +101,10 @@ struct deref_fetch_param {
fetch_func_tfetch_size;
};
+/*
+ * For uprobes, it'll get a vaddr from first call_fetch() so pass NULL
+ * as a priv on the second dprm->fetch() not to translate it to vaddr again.
+ */
#define DEFINE_FETCH_deref(type) \
__kprobes void FETCH_FUNC_NAME(deref, type)(struct pt_regs *regs, \
void *data, void *dest, void *priv) \
@@ -110,13 +114,14 @@ __kprobes void FETCH_FUNC_NAME(deref, type)(struct
pt_regs *regs, \
call_fetch(&dprm->orig, regs, &addr, priv); \
if (addr) { \
addr += dprm->offset; \
- dprm->fetch(regs, (void *)addr, dest, priv);\
+ dprm->fetch(regs, (void *)addr, dest, NULL);\
} else \
*(type *)dest = 0; \
}
DEFINE_BASIC_FETCH_FUNCS(deref)
DEFINE_FETCH_deref(string)
+/* Same as above */
__kprobes void FETCH_FUNC_NAME(deref, string_size)(struct pt_regs *regs,
void *data, void *dest, void *priv)
{
@@ -126,7 +131,7 @@ __kprobes void FETCH_FUNC_NAME(deref, string_size)(struct
pt_regs *regs,
call_fetch(&dprm->orig, regs, &addr, priv);
if (addr && dprm->fetch_size) {
addr += dprm->offset;
- dprm->fetch_size(regs, (void *)addr, dest, priv);
+ dprm->fetch_size(regs, (void *)addr, dest, NULL);
} else
*(string_size *)dest = 0;
}
diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
index fc7edf3749ef..b1e7d722c354 100644
--- a/kernel/trace/trace_probe.h
+++ b/kernel/trace/trace_probe.h
@@ -263,6 +263,7 @@ ASSIGN_FETCH_FUNC(bitfield, ftype), \
#define NR_FETCH_TYPES 10
extern const struct fetch_type kprobes_fetch_type_table[];
+extern const struct fetch_type uprobes_fetch_type_table[];
static inline __kprobes void call_fetch(struct fetch_param *fprm,
struct pt_regs *regs, void *dest, void *priv)
diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index 2c1c648e75bb..80b3f7d667f6 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -510,6 +510,186 @@ static const struct file_operations uprobe_profile_ops = {
.release= seq_release,
};
+#ifdef CONFIG_STACK_GROWSUP
+static unsigned long adjust_stack_addr(unsigned long addr, unsigned n)
+{
+ return addr - (n * sizeof(long));
+}
+
+static bool within_user_stack(struct vm_area_struct *vma, unsigned long addr,
+ unsigned int n)
+{
+ return vma->vm_start <= adjust_stack_addr(addr, n);
+}
+#else
+static unsigned long adjust_stack_addr(unsigned long addr, unsigned n)
+{
+ return addr + (n * sizeof(long));
+}
+
+static bool within_user_stack(struct vm_area_struct *vma, unsigned long addr,
+ unsigned int n)
+{
+ return vma->vm_end >= adjust_stack_addr(addr, n);
+}
+#endif
+
+static unsigned long get_user_stack_nth(struct pt_regs *regs, unsigned int n)
+{
+ struct vm_area_struct *vma;
+ unsigned long addr = GET_USP(regs);
+ bool valid = false;
+ unsigned long ret = 0;
+
+ down_read(¤t->mm->mmap_sem);
+ vma = find_vma(current->mm, addr);
+ if (vma && vma->vm_start <= addr) {
+ if (within_user_stack(vma, addr, n))
+ valid = true;
+ }
+ up_read(¤t->mm->mmap_sem);
+
+ addr = adjust_stack_addr(addr, n);
+
+ if (valid && copy_from_user(&ret, (void __force __user *)addr,
+ sizeof(ret)) == 0)
+ return ret;
+ return 0;
+}
+
+static unsigned long offset_to_vaddr(struct vm_area_struct *vma,
+unsigned long offset)
+{
+ return vma->vm_start + offset - ((loff_t)vma->vm_pgoff << PAGE_SHIFT);
+}
+
+static void __user *get_user_vaddr(unsigned long addr, struct
