Here are some fixes for ksym_tracer.
[PATCH 1/8] ksym_tracer: Extract trace entry from struct trace_ksym
[PATCH 2/8] ksym_tracer: Rewrite ksym_trace_filter_read()
[PATCH 3/8] ksym_tracer: Fix validation of access type
[PATCH 4/8] ksym_tracer: Fix validation of length of access type
[PATCH 5/8] ksym_tracer: NIL-terminate user input filter
[PATCH 6/8] ksym_tracer: Report error when failed to re-register hbp
[PATCH 7/8] ksym_tracer: Fix memory leak
[PATCH 8/8] ksym_tracer: Fix the output of stat tracing
---
kernel/trace/trace_ksym.c | 161 ++++++++++++++++++++++-----------------------
1 files changed, 78 insertions(+), 83 deletions(-)
struct trace_ksym is used as an entry in hbp list, and is also
used as trace_entry stored in ring buffer.
This is not necessary and is a waste of memory in ring buffer.
There is also a bug that dereferencing field->ksym_hbp in
ksym_trace_output() can be invalid.
Signed-off-by: Li Zefan <[email protected]>
---
kernel/trace/trace.h | 13 ++++---------
kernel/trace/trace_ksym.c | 26 ++++++++++++++++++--------
2 files changed, 22 insertions(+), 17 deletions(-)
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 61b4e94..6868902 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -215,17 +215,12 @@ struct syscall_trace_exit {
#define KSYM_SELFTEST_ENTRY "ksym_selftest_dummy"
extern int process_new_ksym_entry(char *ksymname, int op, unsigned long addr);
-struct trace_ksym {
+struct ksym_trace_entry {
struct trace_entry ent;
- struct hw_breakpoint *ksym_hbp;
- unsigned long ksym_addr;
unsigned long ip;
-#ifdef CONFIG_PROFILE_KSYM_TRACER
- unsigned long counter;
-#endif
- struct hlist_node ksym_hlist;
+ unsigned char type;
char ksym_name[KSYM_NAME_LEN];
- char p_name[TASK_COMM_LEN];
+ char cmd[TASK_COMM_LEN];
};
/*
@@ -343,7 +338,7 @@ extern void __ftrace_bad_type(void);
TRACE_SYSCALL_ENTER); \
IF_ASSIGN(var, ent, struct syscall_trace_exit, \
TRACE_SYSCALL_EXIT); \
- IF_ASSIGN(var, ent, struct trace_ksym, TRACE_KSYM); \
+ IF_ASSIGN(var, ent, struct ksym_trace_entry, TRACE_KSYM);\
__ftrace_bad_type(); \
} while (0)
diff --git a/kernel/trace/trace_ksym.c b/kernel/trace/trace_ksym.c
index eef97e7..085ff05 100644
--- a/kernel/trace/trace_ksym.c
+++ b/kernel/trace/trace_ksym.c
@@ -37,6 +37,15 @@
#define KSYM_TRACER_OP_LEN 3 /* rw- */
#define KSYM_FILTER_ENTRY_LEN (KSYM_NAME_LEN + KSYM_TRACER_OP_LEN + 1)
+struct trace_ksym {
+ struct hw_breakpoint *ksym_hbp;
+ unsigned long ksym_addr;
+#ifdef CONFIG_PROFILE_KSYM_TRACER
+ unsigned long counter;
+#endif
+ struct hlist_node ksym_hlist;
+};
+
static struct trace_array *ksym_trace_array;
static unsigned int ksym_filter_entry_count;
@@ -71,7 +80,7 @@ void ksym_hbp_handler(struct hw_breakpoint *hbp, struct pt_regs *regs)
{
struct ring_buffer_event *event;
struct trace_array *tr;
- struct trace_ksym *entry;
+ struct ksym_trace_entry *entry;
int pc;
if (!ksym_tracing_enabled)
@@ -85,11 +94,12 @@ void ksym_hbp_handler(struct hw_breakpoint *hbp, struct pt_regs *regs)
if (!event)
return;
- entry = ring_buffer_event_data(event);
+ entry = ring_buffer_event_data(event);
+ entry->ip = instruction_pointer(regs);
+ entry->type = hbp->info.type;
strlcpy(entry->ksym_name, hbp->info.name, KSYM_SYMBOL_LEN);
- entry->ksym_hbp = hbp;
- entry->ip = instruction_pointer(regs);
- strlcpy(entry->p_name, current->comm, TASK_COMM_LEN);
+ strlcpy(entry->cmd, current->comm, TASK_COMM_LEN);
+
#ifdef CONFIG_PROFILE_KSYM_TRACER
ksym_collect_stats(hbp->info.address);
#endif /* CONFIG_PROFILE_KSYM_TRACER */
@@ -380,7 +390,7 @@ static enum print_line_t ksym_trace_output(struct trace_iterator *iter)
{
struct trace_entry *entry = iter->ent;
struct trace_seq *s = &iter->seq;
- struct trace_ksym *field;
+ struct ksym_trace_entry *field;
char str[KSYM_SYMBOL_LEN];
int ret;
@@ -389,12 +399,12 @@ static enum print_line_t ksym_trace_output(struct trace_iterator *iter)
trace_assign_type(field, entry);
- ret = trace_seq_printf(s, "%-15s %-5d %-3d %-20s ", field->p_name,
+ ret = trace_seq_printf(s, "%-15s %-5d %-3d %-20s ", field->cmd,
entry->pid, iter->cpu, field->ksym_name);
if (!ret)
return TRACE_TYPE_PARTIAL_LINE;
- switch (field->ksym_hbp->info.type) {
+ switch (field->type) {
case HW_BREAKPOINT_WRITE:
ret = trace_seq_printf(s, " W ");
break;
--
1.5.4.rc3
Reading ksym_trace_filter gave me some arbitrary characters,
when it should show nothing. It's because buf is not initialized
when there's no filter.
Also reduce stack usage by about 512 bytes.
Signed-off-by: Li Zefan <[email protected]>
---
kernel/trace/trace_ksym.c | 29 ++++++++++++++++++-----------
1 files changed, 18 insertions(+), 11 deletions(-)
diff --git a/kernel/trace/trace_ksym.c b/kernel/trace/trace_ksym.c
index 085ff05..b6710d3 100644
--- a/kernel/trace/trace_ksym.c
+++ b/kernel/trace/trace_ksym.c
@@ -35,7 +35,6 @@
#define KSYM_TRACER_MAX HBP_NUM
#define KSYM_TRACER_OP_LEN 3 /* rw- */
-#define KSYM_FILTER_ENTRY_LEN (KSYM_NAME_LEN + KSYM_TRACER_OP_LEN + 1)
struct trace_ksym {
struct hw_breakpoint *ksym_hbp;
@@ -230,25 +229,33 @@ static ssize_t ksym_trace_filter_read(struct file *filp, char __user *ubuf,
{
struct trace_ksym *entry;
struct hlist_node *node;
- char buf[KSYM_FILTER_ENTRY_LEN * KSYM_TRACER_MAX];
- ssize_t ret, cnt = 0;
+ struct trace_seq *s;
+ ssize_t cnt = 0;
+ int ret;
+
+ s = kmalloc(sizeof(*s), GFP_KERNEL);
+ if (!s)
+ return -ENOMEM;
+ trace_seq_init(s);
mutex_lock(&ksym_tracer_mutex);
hlist_for_each_entry(entry, node, &ksym_filter_head, ksym_hlist) {
- cnt += snprintf(&buf[cnt], KSYM_FILTER_ENTRY_LEN - cnt, "%s:",
- entry->ksym_hbp->info.name);
+ ret = trace_seq_printf(s, "%s:", entry->ksym_hbp->info.name);
if (entry->ksym_hbp->info.type == HW_BREAKPOINT_WRITE)
- cnt += snprintf(&buf[cnt], KSYM_FILTER_ENTRY_LEN - cnt,
- "-w-\n");
+ ret = trace_seq_puts(s, "-w-\n");
else if (entry->ksym_hbp->info.type == HW_BREAKPOINT_RW)
- cnt += snprintf(&buf[cnt], KSYM_FILTER_ENTRY_LEN - cnt,
- "rw-\n");
+ ret = trace_seq_puts(s, "rw-\n");
+ WARN_ON_ONCE(!ret);
}
- ret = simple_read_from_buffer(ubuf, count, ppos, buf, strlen(buf));
+
+ cnt = simple_read_from_buffer(ubuf, count, ppos, s->buffer, s->len);
+
mutex_unlock(&ksym_tracer_mutex);
- return ret;
+ kfree(s);
+
+ return cnt;
}
static ssize_t ksym_trace_filter_write(struct file *file,
--
1.5.4.rc3
# echo 'pid_max:rw-' > ksym_trace_filter
# cat ksym_trace_filter
pid_max:rw-
# echo 'pid_max:ww-' > ksym_trace_filter
(should return -EINVAL)
# cat ksym_trace_filter
(but it ended up removing filter entry)
Signed-off-by: Li Zefan <[email protected]>
---
kernel/trace/trace_ksym.c | 32 ++++++++++++++------------------
1 files changed, 14 insertions(+), 18 deletions(-)
diff --git a/kernel/trace/trace_ksym.c b/kernel/trace/trace_ksym.c
index b6710d3..9556009 100644
--- a/kernel/trace/trace_ksym.c
+++ b/kernel/trace/trace_ksym.c
@@ -114,24 +114,22 @@ void ksym_hbp_handler(struct hw_breakpoint *hbp, struct pt_regs *regs)
* --x : Set Execution Break points (Not available yet)
*
*/
-static int ksym_trace_get_access_type(char *access_str)
+static int ksym_trace_get_access_type(char *str)
{
- int pos, access = 0;
+ int access = 0;
- for (pos = 0; pos < KSYM_TRACER_OP_LEN; pos++) {
- switch (access_str[pos]) {
- case 'r':
- access += (pos == 0) ? 4 : -1;
- break;
- case 'w':
- access += (pos == 1) ? 2 : -1;
- break;
- case '-':
- break;
- default:
- return -EINVAL;
- }
- }
+ if (str[0] == 'r')
+ access += 4;
+ else if (str[0] != '-')
+ return -EINVAL;
+
+ if (str[1] == 'w')
+ access += 2;
+ else if (str[1] != '-')
+ return -EINVAL;
+
+ if (str[2] != '-')
+ return -EINVAL;
switch (access) {
case 6:
@@ -140,8 +138,6 @@ static int ksym_trace_get_access_type(char *access_str)
case 2:
access = HW_BREAKPOINT_WRITE;
break;
- case 0:
- access = 0;
}
return access;
--
1.5.4.rc3
Don't take newline into account, otherwise:
# echo 'pid_max:-w-' > ksym_trace_filter
# echo -n 'pid_max:rw-' > ksym_trace_filter
bash: echo: write error: Invalid argument
Signed-off-by: Li Zefan <[email protected]>
---
kernel/trace/trace_ksym.c | 14 +++++++-------
1 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/kernel/trace/trace_ksym.c b/kernel/trace/trace_ksym.c
index 9556009..72fcb46 100644
--- a/kernel/trace/trace_ksym.c
+++ b/kernel/trace/trace_ksym.c
@@ -158,21 +158,21 @@ static int ksym_trace_get_access_type(char *str)
static int parse_ksym_trace_str(char *input_string, char **ksymname,
unsigned long *addr)
{
- char *delimiter = ":";
int ret;
- ret = -EINVAL;
- *ksymname = strsep(&input_string, delimiter);
+ strstrip(input_string);
+
+ *ksymname = strsep(&input_string, ":");
*addr = kallsyms_lookup_name(*ksymname);
/* Check for malformed request: (2), (1) and (5) */
if ((!input_string) ||
- (strlen(input_string) != (KSYM_TRACER_OP_LEN + 1)) ||
- (*addr == 0))
- goto return_code;
+ (strlen(input_string) != KSYM_TRACER_OP_LEN) ||
+ (*addr == 0))
+ return -EINVAL;;
+
ret = ksym_trace_get_access_type(input_string);
-return_code:
return ret;
}
--
1.5.4.rc3
Make sure the user input string is NULL-terminated.
Signed-off-by: Li Zefan <[email protected]>
---
kernel/trace/trace_ksym.c | 7 ++-----
1 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/kernel/trace/trace_ksym.c b/kernel/trace/trace_ksym.c
index 72fcb46..8cbed5a 100644
--- a/kernel/trace/trace_ksym.c
+++ b/kernel/trace/trace_ksym.c
@@ -264,11 +264,7 @@ static ssize_t ksym_trace_filter_write(struct file *file,
unsigned long ksym_addr = 0;
int ret, op, changed = 0;
- /* Ignore echo "" > ksym_trace_filter */
- if (count == 0)
- return 0;
-
- input_string = kzalloc(count, GFP_KERNEL);
+ input_string = kzalloc(count + 1, GFP_KERNEL);
if (!input_string)
return -ENOMEM;
@@ -276,6 +272,7 @@ static ssize_t ksym_trace_filter_write(struct file *file,
kfree(input_string);
return -EFAULT;
}
+ input_string[count] = '\0';
ret = op = parse_ksym_trace_str(input_string, &ksymname, &ksym_addr);
if (ret < 0) {
--
1.5.4.rc3
When access type is changed, the hw break point will be
unregistered and then be registered again with new access
type. But the registration may fail, in this case, -errno
should be returned.
Signed-off-by: Li Zefan <[email protected]>
---
kernel/trace/trace_ksym.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/trace/trace_ksym.c b/kernel/trace/trace_ksym.c
index 8cbed5a..891e3b8 100644
--- a/kernel/trace/trace_ksym.c
+++ b/kernel/trace/trace_ksym.c
@@ -302,13 +302,13 @@ static ssize_t ksym_trace_filter_write(struct file *file,
ret = count;
goto unlock_ret_path;
}
- }
+ } else
+ ret = count;
ksym_filter_entry_count--;
hlist_del_rcu(&(entry->ksym_hlist));
synchronize_rcu();
kfree(entry->ksym_hbp);
kfree(entry);
- ret = count;
goto err_ret;
} else {
/* Check for malformed request: (4) */
--
1.5.4.rc3
- When remove a filter, we leak entry->ksym_hbp->info.name.
- With CONFIG_FTRAC_SELFTEST enabled, we leak ->info.name:
# echo ksym_tracer > current_tracer
# echo 'ksym_selftest_dummy:rw-' > ksym_trace_filter
# echo nop > current_tracer
Signed-off-by: Li Zefan <[email protected]>
---
kernel/trace/trace_ksym.c | 61 ++++++++++++++++++++-------------------------
1 files changed, 27 insertions(+), 34 deletions(-)
diff --git a/kernel/trace/trace_ksym.c b/kernel/trace/trace_ksym.c
index 891e3b8..7d349d3 100644
--- a/kernel/trace/trace_ksym.c
+++ b/kernel/trace/trace_ksym.c
@@ -179,7 +179,7 @@ static int parse_ksym_trace_str(char *input_string, char **ksymname,
int process_new_ksym_entry(char *ksymname, int op, unsigned long addr)
{
struct trace_ksym *entry;
- int ret;
+ int ret = -ENOMEM;
if (ksym_filter_entry_count >= KSYM_TRACER_MAX) {
printk(KERN_ERR "ksym_tracer: Maximum limit:(%d) reached. No"
@@ -193,12 +193,13 @@ int process_new_ksym_entry(char *ksymname, int op, unsigned long addr)
return -ENOMEM;
entry->ksym_hbp = kzalloc(sizeof(struct hw_breakpoint), GFP_KERNEL);
- if (!entry->ksym_hbp) {
- kfree(entry);
- return -ENOMEM;
- }
+ if (!entry->ksym_hbp)
+ goto err;
+
+ entry->ksym_hbp->info.name = kstrdup(ksymname, GFP_KERNEL);
+ if (!entry->ksym_hbp->info.name)
+ goto err;
- entry->ksym_hbp->info.name = ksymname;
entry->ksym_hbp->info.type = op;
entry->ksym_addr = entry->ksym_hbp->info.address = addr;
#ifdef CONFIG_X86
@@ -210,14 +211,18 @@ int process_new_ksym_entry(char *ksymname, int op, unsigned long addr)
if (ret < 0) {
printk(KERN_INFO "ksym_tracer request failed. Try again"
" later!!\n");
- kfree(entry->ksym_hbp);
- kfree(entry);
- return -EAGAIN;
+ ret = -EAGAIN;
+ goto err;
}
hlist_add_head_rcu(&(entry->ksym_hlist), &ksym_filter_head);
ksym_filter_entry_count++;
-
return 0;
+err:
+ if (entry->ksym_hbp)
+ kfree(entry->ksym_hbp->info.name);
+ kfree(entry->ksym_hbp);
+ kfree(entry);
+ return ret;
}
static ssize_t ksym_trace_filter_read(struct file *filp, char __user *ubuf,
@@ -289,7 +294,7 @@ static ssize_t ksym_trace_filter_write(struct file *file,
if (entry->ksym_hbp->info.type != op)
changed = 1;
else
- goto err_ret;
+ goto out;
break;
}
}
@@ -298,34 +303,29 @@ static ssize_t ksym_trace_filter_write(struct file *file,
entry->ksym_hbp->info.type = op;
if (op > 0) {
ret = register_kernel_hw_breakpoint(entry->ksym_hbp);
- if (ret == 0) {
- ret = count;
- goto unlock_ret_path;
- }
- } else
- ret = count;
+ if (ret == 0)
+ goto out;
+ }
ksym_filter_entry_count--;
hlist_del_rcu(&(entry->ksym_hlist));
synchronize_rcu();
+ kfree(entry->ksym_hbp->info.name);
kfree(entry->ksym_hbp);
kfree(entry);
- goto err_ret;
+ goto out;
} else {
/* Check for malformed request: (4) */
if (op == 0)
- goto err_ret;
+ goto out;
ret = process_new_ksym_entry(ksymname, op, ksym_addr);
- if (ret)
- goto err_ret;
}
- ret = count;
- goto unlock_ret_path;
+out:
+ mutex_unlock(&ksym_tracer_mutex);
-err_ret:
kfree(input_string);
-unlock_ret_path:
- mutex_unlock(&ksym_tracer_mutex);
+ if (!ret)
+ ret = count;
return ret;
}
@@ -349,14 +349,7 @@ static void ksym_trace_reset(struct trace_array *tr)
ksym_filter_entry_count--;
hlist_del_rcu(&(entry->ksym_hlist));
synchronize_rcu();
- /* Free the 'input_string' only if reset
- * after startup self-test
- */
-#ifdef CONFIG_FTRACE_SELFTEST
- if (strncmp(entry->ksym_hbp->info.name, KSYM_SELFTEST_ENTRY,
- strlen(KSYM_SELFTEST_ENTRY)) != 0)
-#endif /* CONFIG_FTRACE_SELFTEST*/
- kfree(entry->ksym_hbp->info.name);
+ kfree(entry->ksym_hbp->info.name);
kfree(entry->ksym_hbp);
kfree(entry);
}
--
1.5.4.rc3
- make ksym_tracer_stat_start() return head->first instead of
&head->first
- make the output properly aligned
Before:
Access type Symbol Counter
NA <NA> 0
RW pid_max 0
After:
Access Type Symbol Counter
----------- ------ -------
RW pid_max 0
Signed-off-by: Li Zefan <[email protected]>
---
kernel/trace/trace_ksym.c | 20 +++++++++++---------
1 files changed, 11 insertions(+), 9 deletions(-)
diff --git a/kernel/trace/trace_ksym.c b/kernel/trace/trace_ksym.c
index 7d349d3..1256a6e 100644
--- a/kernel/trace/trace_ksym.c
+++ b/kernel/trace/trace_ksym.c
@@ -453,8 +453,10 @@ device_initcall(init_ksym_trace);
#ifdef CONFIG_PROFILE_KSYM_TRACER
static int ksym_tracer_stat_headers(struct seq_file *m)
{
- seq_printf(m, " Access type ");
- seq_printf(m, " Symbol Counter \n");
+ seq_puts(m, " Access Type ");
+ seq_puts(m, " Symbol Counter\n");
+ seq_puts(m, " ----------- ");
+ seq_puts(m, " ------ -------\n");
return 0;
}
@@ -472,27 +474,27 @@ static int ksym_tracer_stat_show(struct seq_file *m, void *v)
switch (access_type) {
case HW_BREAKPOINT_WRITE:
- seq_printf(m, " W ");
+ seq_puts(m, " W ");
break;
case HW_BREAKPOINT_RW:
- seq_printf(m, " RW ");
+ seq_puts(m, " RW ");
break;
default:
- seq_printf(m, " NA ");
+ seq_puts(m, " NA ");
}
if (lookup_symbol_name(entry->ksym_addr, fn_name) >= 0)
- seq_printf(m, " %s ", fn_name);
+ seq_printf(m, " %-36s", fn_name);
else
- seq_printf(m, " <NA> ");
+ seq_printf(m, " %-36s", "<NA>");
+ seq_printf(m, " %15lu\n", entry->counter);
- seq_printf(m, "%15lu\n", entry->counter);
return 0;
}
static void *ksym_tracer_stat_start(struct tracer_stat *trace)
{
- return &(ksym_filter_head.first);
+ return ksym_filter_head.first;
}
static void *
--
1.5.4.rc3
On Tue, Jul 07, 2009 at 01:52:36PM +0800, Li Zefan wrote:
> struct trace_ksym is used as an entry in hbp list, and is also
> used as trace_entry stored in ring buffer.
>
> This is not necessary and is a waste of memory in ring buffer.
>
> There is also a bug that dereferencing field->ksym_hbp in
> ksym_trace_output() can be invalid.
>
> Signed-off-by: Li Zefan <[email protected]>
> ---
> kernel/trace/trace.h | 13 ++++---------
> kernel/trace/trace_ksym.c | 26 ++++++++++++++++++--------
> 2 files changed, 22 insertions(+), 17 deletions(-)
>
> diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
> index 61b4e94..6868902 100644
> --- a/kernel/trace/trace.h
> +++ b/kernel/trace/trace.h
> @@ -215,17 +215,12 @@ struct syscall_trace_exit {
> #define KSYM_SELFTEST_ENTRY "ksym_selftest_dummy"
> extern int process_new_ksym_entry(char *ksymname, int op, unsigned long addr);
>
> -struct trace_ksym {
> +struct ksym_trace_entry {
> struct trace_entry ent;
> - struct hw_breakpoint *ksym_hbp;
> - unsigned long ksym_addr;
> unsigned long ip;
> -#ifdef CONFIG_PROFILE_KSYM_TRACER
> - unsigned long counter;
> -#endif
> - struct hlist_node ksym_hlist;
> + unsigned char type;
> char ksym_name[KSYM_NAME_LEN];
> - char p_name[TASK_COMM_LEN];
> + char cmd[TASK_COMM_LEN];
> };
>
> /*
> @@ -343,7 +338,7 @@ extern void __ftrace_bad_type(void);
> TRACE_SYSCALL_ENTER); \
> IF_ASSIGN(var, ent, struct syscall_trace_exit, \
> TRACE_SYSCALL_EXIT); \
> - IF_ASSIGN(var, ent, struct trace_ksym, TRACE_KSYM); \
> + IF_ASSIGN(var, ent, struct ksym_trace_entry, TRACE_KSYM);\
> __ftrace_bad_type(); \
> } while (0)
>
> diff --git a/kernel/trace/trace_ksym.c b/kernel/trace/trace_ksym.c
> index eef97e7..085ff05 100644
> --- a/kernel/trace/trace_ksym.c
> +++ b/kernel/trace/trace_ksym.c
> @@ -37,6 +37,15 @@
> #define KSYM_TRACER_OP_LEN 3 /* rw- */
> #define KSYM_FILTER_ENTRY_LEN (KSYM_NAME_LEN + KSYM_TRACER_OP_LEN + 1)
>
> +struct trace_ksym {
> + struct hw_breakpoint *ksym_hbp;
> + unsigned long ksym_addr;
> +#ifdef CONFIG_PROFILE_KSYM_TRACER
> + unsigned long counter;
> +#endif
> + struct hlist_node ksym_hlist;
> +};
> +
> static struct trace_array *ksym_trace_array;
>
> static unsigned int ksym_filter_entry_count;
> @@ -71,7 +80,7 @@ void ksym_hbp_handler(struct hw_breakpoint *hbp, struct pt_regs *regs)
> {
> struct ring_buffer_event *event;
> struct trace_array *tr;
> - struct trace_ksym *entry;
> + struct ksym_trace_entry *entry;
> int pc;
>
> if (!ksym_tracing_enabled)
> @@ -85,11 +94,12 @@ void ksym_hbp_handler(struct hw_breakpoint *hbp, struct pt_regs *regs)
> if (!event)
> return;
>
> - entry = ring_buffer_event_data(event);
> + entry = ring_buffer_event_data(event);
> + entry->ip = instruction_pointer(regs);
> + entry->type = hbp->info.type;
> strlcpy(entry->ksym_name, hbp->info.name, KSYM_SYMBOL_LEN);
> - entry->ksym_hbp = hbp;
> - entry->ip = instruction_pointer(regs);
> - strlcpy(entry->p_name, current->comm, TASK_COMM_LEN);
> + strlcpy(entry->cmd, current->comm, TASK_COMM_LEN);
Ah, I did not realized that the cmdline was that roughly saved.
It would be more ring-buffer-friendly to use tracing_record_cmdline.
> +
> #ifdef CONFIG_PROFILE_KSYM_TRACER
> ksym_collect_stats(hbp->info.address);
> #endif /* CONFIG_PROFILE_KSYM_TRACER */
> @@ -380,7 +390,7 @@ static enum print_line_t ksym_trace_output(struct trace_iterator *iter)
> {
> struct trace_entry *entry = iter->ent;
> struct trace_seq *s = &iter->seq;
> - struct trace_ksym *field;
> + struct ksym_trace_entry *field;
> char str[KSYM_SYMBOL_LEN];
> int ret;
>
> @@ -389,12 +399,12 @@ static enum print_line_t ksym_trace_output(struct trace_iterator *iter)
>
> trace_assign_type(field, entry);
>
> - ret = trace_seq_printf(s, "%-15s %-5d %-3d %-20s ", field->p_name,
> + ret = trace_seq_printf(s, "%-15s %-5d %-3d %-20s ", field->cmd,
> entry->pid, iter->cpu, field->ksym_name);
> if (!ret)
> return TRACE_TYPE_PARTIAL_LINE;
>
> - switch (field->ksym_hbp->info.type) {
> + switch (field->type) {
> case HW_BREAKPOINT_WRITE:
> ret = trace_seq_printf(s, " W ");
> break;
Thanks!
Acked-by: Frederic Weisbecker <[email protected]>
Prasad, any objection?
On Tue, Jul 07, 2009 at 01:52:52PM +0800, Li Zefan wrote:
> Reading ksym_trace_filter gave me some arbitrary characters,
> when it should show nothing. It's because buf is not initialized
> when there's no filter.
>
> Also reduce stack usage by about 512 bytes.
>
> Signed-off-by: Li Zefan <[email protected]>
> ---
> kernel/trace/trace_ksym.c | 29 ++++++++++++++++++-----------
> 1 files changed, 18 insertions(+), 11 deletions(-)
>
> diff --git a/kernel/trace/trace_ksym.c b/kernel/trace/trace_ksym.c
> index 085ff05..b6710d3 100644
> --- a/kernel/trace/trace_ksym.c
> +++ b/kernel/trace/trace_ksym.c
> @@ -35,7 +35,6 @@
> #define KSYM_TRACER_MAX HBP_NUM
>
> #define KSYM_TRACER_OP_LEN 3 /* rw- */
> -#define KSYM_FILTER_ENTRY_LEN (KSYM_NAME_LEN + KSYM_TRACER_OP_LEN + 1)
>
> struct trace_ksym {
> struct hw_breakpoint *ksym_hbp;
> @@ -230,25 +229,33 @@ static ssize_t ksym_trace_filter_read(struct file *filp, char __user *ubuf,
> {
> struct trace_ksym *entry;
> struct hlist_node *node;
> - char buf[KSYM_FILTER_ENTRY_LEN * KSYM_TRACER_MAX];
> - ssize_t ret, cnt = 0;
> + struct trace_seq *s;
Hehe, this trace_seq type is very convenient :)
> + ssize_t cnt = 0;
> + int ret;
> +
> + s = kmalloc(sizeof(*s), GFP_KERNEL);
> + if (!s)
> + return -ENOMEM;
> + trace_seq_init(s);
>
> mutex_lock(&ksym_tracer_mutex);
>
> hlist_for_each_entry(entry, node, &ksym_filter_head, ksym_hlist) {
> - cnt += snprintf(&buf[cnt], KSYM_FILTER_ENTRY_LEN - cnt, "%s:",
> - entry->ksym_hbp->info.name);
> + ret = trace_seq_printf(s, "%s:", entry->ksym_hbp->info.name);
> if (entry->ksym_hbp->info.type == HW_BREAKPOINT_WRITE)
> - cnt += snprintf(&buf[cnt], KSYM_FILTER_ENTRY_LEN - cnt,
> - "-w-\n");
> + ret = trace_seq_puts(s, "-w-\n");
> else if (entry->ksym_hbp->info.type == HW_BREAKPOINT_RW)
> - cnt += snprintf(&buf[cnt], KSYM_FILTER_ENTRY_LEN - cnt,
> - "rw-\n");
> + ret = trace_seq_puts(s, "rw-\n");
> + WARN_ON_ONCE(!ret);
> }
> - ret = simple_read_from_buffer(ubuf, count, ppos, buf, strlen(buf));
> +
> + cnt = simple_read_from_buffer(ubuf, count, ppos, s->buffer, s->len);
> +
> mutex_unlock(&ksym_tracer_mutex);
>
> - return ret;
> + kfree(s);
> +
> + return cnt;
> }
>
> static ssize_t ksym_trace_filter_write(struct file *file,
Looks good too.
Ack.
On Tue, Jul 07, 2009 at 01:52:17PM +0800, Li Zefan wrote:
> Here are some fixes for ksym_tracer.
>
> [PATCH 1/8] ksym_tracer: Extract trace entry from struct trace_ksym
> [PATCH 2/8] ksym_tracer: Rewrite ksym_trace_filter_read()
> [PATCH 3/8] ksym_tracer: Fix validation of access type
> [PATCH 4/8] ksym_tracer: Fix validation of length of access type
> [PATCH 5/8] ksym_tracer: NIL-terminate user input filter
> [PATCH 6/8] ksym_tracer: Report error when failed to re-register hbp
> [PATCH 7/8] ksym_tracer: Fix memory leak
> [PATCH 8/8] ksym_tracer: Fix the output of stat tracing
> ---
> kernel/trace/trace_ksym.c | 161 ++++++++++++++++++++++-----------------------
> 1 files changed, 78 insertions(+), 83 deletions(-)
>
For the whole series:
Acked-by: Frederic Weisbecker <[email protected]>
Thanks a lot!
Prasad, any objection?
>> - entry = ring_buffer_event_data(event);
>> + entry = ring_buffer_event_data(event);
>> + entry->ip = instruction_pointer(regs);
>> + entry->type = hbp->info.type;
>> strlcpy(entry->ksym_name, hbp->info.name, KSYM_SYMBOL_LEN);
>> - entry->ksym_hbp = hbp;
>> - entry->ip = instruction_pointer(regs);
>> - strlcpy(entry->p_name, current->comm, TASK_COMM_LEN);
>> + strlcpy(entry->cmd, current->comm, TASK_COMM_LEN);
>
>
> Ah, I did not realized that the cmdline was that roughly saved.
> It would be more ring-buffer-friendly to use tracing_record_cmdline.
>
Yeah, I'll send an incremental patch later.
>
* Frederic Weisbecker <[email protected]> wrote:
> On Tue, Jul 07, 2009 at 01:52:17PM +0800, Li Zefan wrote:
> > Here are some fixes for ksym_tracer.
> >
> > [PATCH 1/8] ksym_tracer: Extract trace entry from struct trace_ksym
> > [PATCH 2/8] ksym_tracer: Rewrite ksym_trace_filter_read()
> > [PATCH 3/8] ksym_tracer: Fix validation of access type
> > [PATCH 4/8] ksym_tracer: Fix validation of length of access type
> > [PATCH 5/8] ksym_tracer: NIL-terminate user input filter
> > [PATCH 6/8] ksym_tracer: Report error when failed to re-register hbp
> > [PATCH 7/8] ksym_tracer: Fix memory leak
> > [PATCH 8/8] ksym_tracer: Fix the output of stat tracing
> > ---
> > kernel/trace/trace_ksym.c | 161 ++++++++++++++++++++++-----------------------
> > 1 files changed, 78 insertions(+), 83 deletions(-)
> >
>
> For the whole series:
>
> Acked-by: Frederic Weisbecker <[email protected]>
>
> Thanks a lot!
>
> Prasad, any objection?
Thanks guys, i've applied it to tip:tracing/hw-breakpoints. Not yet
merged into tracing/core - we also want to see how the perfcounters
angle plays out before committing this for v2.6.32.
Ingo
On Tue, Jul 07, 2009 at 01:52:52PM +0800, Li Zefan wrote:
> Reading ksym_trace_filter gave me some arbitrary characters,
> when it should show nothing. It's because buf is not initialized
> when there's no filter.
>
I started noticing this behaviour recently.
A simple
char buf[KSYM_FILTER_ENTRY_LEN * KSYM_TRACER_MAX] = "\0";
solves the problem.
> Also reduce stack usage by about 512 bytes.
>
The stack usage would be much lesser on other architectures with fewer
breakpoint registers (like PPC64), and should really be an issue.
> Signed-off-by: Li Zefan <[email protected]>
> ---
> kernel/trace/trace_ksym.c | 29 ++++++++++++++++++-----------
> 1 files changed, 18 insertions(+), 11 deletions(-)
>
> diff --git a/kernel/trace/trace_ksym.c b/kernel/trace/trace_ksym.c
> index 085ff05..b6710d3 100644
> --- a/kernel/trace/trace_ksym.c
> +++ b/kernel/trace/trace_ksym.c
> @@ -35,7 +35,6 @@
> #define KSYM_TRACER_MAX HBP_NUM
>
> #define KSYM_TRACER_OP_LEN 3 /* rw- */
> -#define KSYM_FILTER_ENTRY_LEN (KSYM_NAME_LEN + KSYM_TRACER_OP_LEN + 1)
>
> struct trace_ksym {
> struct hw_breakpoint *ksym_hbp;
> @@ -230,25 +229,33 @@ static ssize_t ksym_trace_filter_read(struct file *filp, char __user *ubuf,
> {
> struct trace_ksym *entry;
> struct hlist_node *node;
> - char buf[KSYM_FILTER_ENTRY_LEN * KSYM_TRACER_MAX];
> - ssize_t ret, cnt = 0;
> + struct trace_seq *s;
> + ssize_t cnt = 0;
> + int ret;
> +
> + s = kmalloc(sizeof(*s), GFP_KERNEL);
> + if (!s)
> + return -ENOMEM;
> + trace_seq_init(s);
>
> mutex_lock(&ksym_tracer_mutex);
>
> hlist_for_each_entry(entry, node, &ksym_filter_head, ksym_hlist) {
> - cnt += snprintf(&buf[cnt], KSYM_FILTER_ENTRY_LEN - cnt, "%s:",
> - entry->ksym_hbp->info.name);
> + ret = trace_seq_printf(s, "%s:", entry->ksym_hbp->info.name);
> if (entry->ksym_hbp->info.type == HW_BREAKPOINT_WRITE)
> - cnt += snprintf(&buf[cnt], KSYM_FILTER_ENTRY_LEN - cnt,
> - "-w-\n");
> + ret = trace_seq_puts(s, "-w-\n");
> else if (entry->ksym_hbp->info.type == HW_BREAKPOINT_RW)
> - cnt += snprintf(&buf[cnt], KSYM_FILTER_ENTRY_LEN - cnt,
> - "rw-\n");
> + ret = trace_seq_puts(s, "rw-\n");
> + WARN_ON_ONCE(!ret);
Given that this change uses the tracer defined functions and a seq-file
implementation, the patch is welcome.
Thanks,
K.Prasad
On Wed, Jul 08, 2009 at 01:18:37AM +0200, Frederic Weisbecker wrote:
> On Tue, Jul 07, 2009 at 01:52:17PM +0800, Li Zefan wrote:
> > Here are some fixes for ksym_tracer.
> >
> > [PATCH 1/8] ksym_tracer: Extract trace entry from struct trace_ksym
> > [PATCH 2/8] ksym_tracer: Rewrite ksym_trace_filter_read()
> > [PATCH 3/8] ksym_tracer: Fix validation of access type
> > [PATCH 4/8] ksym_tracer: Fix validation of length of access type
> > [PATCH 5/8] ksym_tracer: NIL-terminate user input filter
> > [PATCH 6/8] ksym_tracer: Report error when failed to re-register hbp
> > [PATCH 7/8] ksym_tracer: Fix memory leak
> > [PATCH 8/8] ksym_tracer: Fix the output of stat tracing
> > ---
> > kernel/trace/trace_ksym.c | 161 ++++++++++++++++++++++-----------------------
> > 1 files changed, 78 insertions(+), 83 deletions(-)
> >
>
> For the whole series:
>
> Acked-by: Frederic Weisbecker <[email protected]>
>
> Thanks a lot!
>
> Prasad, any objection?
>
Hi,
Sorry for the delayed response (just back from a week-long
vacation)..the patches do a fine-job of fixing the issues that escaped
attention during development.
Thanks for the patchset!
-- K.Prasad
Commit-ID: db59504d89db1462a5281fb55b1d962cb74a398f
Gitweb: http://git.kernel.org/tip/db59504d89db1462a5281fb55b1d962cb74a398f
Author: Li Zefan <[email protected]>
AuthorDate: Tue, 7 Jul 2009 13:52:36 +0800
Committer: Ingo Molnar <[email protected]>
CommitDate: Fri, 10 Jul 2009 11:59:40 +0200
ksym_tracer: Extract trace entry from struct trace_ksym
struct trace_ksym is used as an entry in hbp list, and is also
used as trace_entry stored in ring buffer.
This is not necessary and is a waste of memory in ring buffer.
There is also a bug that dereferencing field->ksym_hbp in
ksym_trace_output() can be invalid.
Signed-off-by: Li Zefan <[email protected]>
Acked-by: Frederic Weisbecker <[email protected]>
Cc: "K.Prasad" <[email protected]>
Cc: Alan Stern <[email protected]>
Cc: Steven Rostedt <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
---
kernel/trace/trace.h | 13 ++++---------
kernel/trace/trace_ksym.c | 26 ++++++++++++++++++--------
2 files changed, 22 insertions(+), 17 deletions(-)
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 7d5cc37..ff1ef41 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -215,17 +215,12 @@ struct syscall_trace_exit {
#define KSYM_SELFTEST_ENTRY "ksym_selftest_dummy"
extern int process_new_ksym_entry(char *ksymname, int op, unsigned long addr);
-struct trace_ksym {
+struct ksym_trace_entry {
struct trace_entry ent;
- struct hw_breakpoint *ksym_hbp;
- unsigned long ksym_addr;
unsigned long ip;
-#ifdef CONFIG_PROFILE_KSYM_TRACER
- unsigned long counter;
-#endif
- struct hlist_node ksym_hlist;
+ unsigned char type;
char ksym_name[KSYM_NAME_LEN];
- char p_name[TASK_COMM_LEN];
+ char cmd[TASK_COMM_LEN];
};
/*
@@ -343,7 +338,7 @@ extern void __ftrace_bad_type(void);
TRACE_SYSCALL_ENTER); \
IF_ASSIGN(var, ent, struct syscall_trace_exit, \
TRACE_SYSCALL_EXIT); \
- IF_ASSIGN(var, ent, struct trace_ksym, TRACE_KSYM); \
+ IF_ASSIGN(var, ent, struct ksym_trace_entry, TRACE_KSYM);\
__ftrace_bad_type(); \
} while (0)
diff --git a/kernel/trace/trace_ksym.c b/kernel/trace/trace_ksym.c
index eef97e7..085ff05 100644
--- a/kernel/trace/trace_ksym.c
+++ b/kernel/trace/trace_ksym.c
@@ -37,6 +37,15 @@
#define KSYM_TRACER_OP_LEN 3 /* rw- */
#define KSYM_FILTER_ENTRY_LEN (KSYM_NAME_LEN + KSYM_TRACER_OP_LEN + 1)
+struct trace_ksym {
+ struct hw_breakpoint *ksym_hbp;
+ unsigned long ksym_addr;
+#ifdef CONFIG_PROFILE_KSYM_TRACER
+ unsigned long counter;
+#endif
+ struct hlist_node ksym_hlist;
+};
+
static struct trace_array *ksym_trace_array;
static unsigned int ksym_filter_entry_count;
@@ -71,7 +80,7 @@ void ksym_hbp_handler(struct hw_breakpoint *hbp, struct pt_regs *regs)
{
struct ring_buffer_event *event;
struct trace_array *tr;
- struct trace_ksym *entry;
+ struct ksym_trace_entry *entry;
int pc;
if (!ksym_tracing_enabled)
@@ -85,11 +94,12 @@ void ksym_hbp_handler(struct hw_breakpoint *hbp, struct pt_regs *regs)
if (!event)
return;
- entry = ring_buffer_event_data(event);
+ entry = ring_buffer_event_data(event);
+ entry->ip = instruction_pointer(regs);
+ entry->type = hbp->info.type;
strlcpy(entry->ksym_name, hbp->info.name, KSYM_SYMBOL_LEN);
- entry->ksym_hbp = hbp;
- entry->ip = instruction_pointer(regs);
- strlcpy(entry->p_name, current->comm, TASK_COMM_LEN);
+ strlcpy(entry->cmd, current->comm, TASK_COMM_LEN);
+
#ifdef CONFIG_PROFILE_KSYM_TRACER
ksym_collect_stats(hbp->info.address);
#endif /* CONFIG_PROFILE_KSYM_TRACER */
@@ -380,7 +390,7 @@ static enum print_line_t ksym_trace_output(struct trace_iterator *iter)
{
struct trace_entry *entry = iter->ent;
struct trace_seq *s = &iter->seq;
- struct trace_ksym *field;
+ struct ksym_trace_entry *field;
char str[KSYM_SYMBOL_LEN];
int ret;
@@ -389,12 +399,12 @@ static enum print_line_t ksym_trace_output(struct trace_iterator *iter)
trace_assign_type(field, entry);
- ret = trace_seq_printf(s, "%-15s %-5d %-3d %-20s ", field->p_name,
+ ret = trace_seq_printf(s, "%-15s %-5d %-3d %-20s ", field->cmd,
entry->pid, iter->cpu, field->ksym_name);
if (!ret)
return TRACE_TYPE_PARTIAL_LINE;
- switch (field->ksym_hbp->info.type) {
+ switch (field->type) {
case HW_BREAKPOINT_WRITE:
ret = trace_seq_printf(s, " W ");
break;
Commit-ID: be9742e6cb107fe1d77db7a081ea4eb25e79e1ad
Gitweb: http://git.kernel.org/tip/be9742e6cb107fe1d77db7a081ea4eb25e79e1ad
Author: Li Zefan <[email protected]>
AuthorDate: Tue, 7 Jul 2009 13:52:52 +0800
Committer: Ingo Molnar <[email protected]>
CommitDate: Fri, 10 Jul 2009 11:59:40 +0200
ksym_tracer: Rewrite ksym_trace_filter_read()
Reading ksym_trace_filter gave me some arbitrary characters,
when it should show nothing. It's because buf is not initialized
when there's no filter.
Also reduce stack usage by about 512 bytes.
Signed-off-by: Li Zefan <[email protected]>
Acked-by: Frederic Weisbecker <[email protected]>
Cc: "K.Prasad" <[email protected]>
Cc: Alan Stern <[email protected]>
Cc: Steven Rostedt <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
---
kernel/trace/trace_ksym.c | 29 ++++++++++++++++++-----------
1 files changed, 18 insertions(+), 11 deletions(-)
diff --git a/kernel/trace/trace_ksym.c b/kernel/trace/trace_ksym.c
index 085ff05..b6710d3 100644
--- a/kernel/trace/trace_ksym.c
+++ b/kernel/trace/trace_ksym.c
@@ -35,7 +35,6 @@
#define KSYM_TRACER_MAX HBP_NUM
#define KSYM_TRACER_OP_LEN 3 /* rw- */
-#define KSYM_FILTER_ENTRY_LEN (KSYM_NAME_LEN + KSYM_TRACER_OP_LEN + 1)
struct trace_ksym {
struct hw_breakpoint *ksym_hbp;
@@ -230,25 +229,33 @@ static ssize_t ksym_trace_filter_read(struct file *filp, char __user *ubuf,
{
struct trace_ksym *entry;
struct hlist_node *node;
- char buf[KSYM_FILTER_ENTRY_LEN * KSYM_TRACER_MAX];
- ssize_t ret, cnt = 0;
+ struct trace_seq *s;
+ ssize_t cnt = 0;
+ int ret;
+
+ s = kmalloc(sizeof(*s), GFP_KERNEL);
+ if (!s)
+ return -ENOMEM;
+ trace_seq_init(s);
mutex_lock(&ksym_tracer_mutex);
hlist_for_each_entry(entry, node, &ksym_filter_head, ksym_hlist) {
- cnt += snprintf(&buf[cnt], KSYM_FILTER_ENTRY_LEN - cnt, "%s:",
- entry->ksym_hbp->info.name);
+ ret = trace_seq_printf(s, "%s:", entry->ksym_hbp->info.name);
if (entry->ksym_hbp->info.type == HW_BREAKPOINT_WRITE)
- cnt += snprintf(&buf[cnt], KSYM_FILTER_ENTRY_LEN - cnt,
- "-w-\n");
+ ret = trace_seq_puts(s, "-w-\n");
else if (entry->ksym_hbp->info.type == HW_BREAKPOINT_RW)
- cnt += snprintf(&buf[cnt], KSYM_FILTER_ENTRY_LEN - cnt,
- "rw-\n");
+ ret = trace_seq_puts(s, "rw-\n");
+ WARN_ON_ONCE(!ret);
}
- ret = simple_read_from_buffer(ubuf, count, ppos, buf, strlen(buf));
+
+ cnt = simple_read_from_buffer(ubuf, count, ppos, s->buffer, s->len);
+
mutex_unlock(&ksym_tracer_mutex);
- return ret;
+ kfree(s);
+
+ return cnt;
}
static ssize_t ksym_trace_filter_write(struct file *file,
Commit-ID: f088e5471297cc78d7465e1fd997cb1a91a48019
Gitweb: http://git.kernel.org/tip/f088e5471297cc78d7465e1fd997cb1a91a48019
Author: Li Zefan <[email protected]>
AuthorDate: Tue, 7 Jul 2009 13:53:18 +0800
Committer: Ingo Molnar <[email protected]>
CommitDate: Fri, 10 Jul 2009 11:59:41 +0200
ksym_tracer: Fix validation of access type
# echo 'pid_max:rw-' > ksym_trace_filter
# cat ksym_trace_filter
pid_max:rw-
# echo 'pid_max:ww-' > ksym_trace_filter
(should return -EINVAL)
# cat ksym_trace_filter
(but it ended up removing filter entry)
Signed-off-by: Li Zefan <[email protected]>
Acked-by: Frederic Weisbecker <[email protected]>
Cc: "K.Prasad" <[email protected]>
Cc: Alan Stern <[email protected]>
Cc: Steven Rostedt <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
---
kernel/trace/trace_ksym.c | 32 ++++++++++++++------------------
1 files changed, 14 insertions(+), 18 deletions(-)
diff --git a/kernel/trace/trace_ksym.c b/kernel/trace/trace_ksym.c
index b6710d3..9556009 100644
--- a/kernel/trace/trace_ksym.c
+++ b/kernel/trace/trace_ksym.c
@@ -114,24 +114,22 @@ void ksym_hbp_handler(struct hw_breakpoint *hbp, struct pt_regs *regs)
* --x : Set Execution Break points (Not available yet)
*
*/
-static int ksym_trace_get_access_type(char *access_str)
+static int ksym_trace_get_access_type(char *str)
{
- int pos, access = 0;
+ int access = 0;
- for (pos = 0; pos < KSYM_TRACER_OP_LEN; pos++) {
- switch (access_str[pos]) {
- case 'r':
- access += (pos == 0) ? 4 : -1;
- break;
- case 'w':
- access += (pos == 1) ? 2 : -1;
- break;
- case '-':
- break;
- default:
- return -EINVAL;
- }
- }
+ if (str[0] == 'r')
+ access += 4;
+ else if (str[0] != '-')
+ return -EINVAL;
+
+ if (str[1] == 'w')
+ access += 2;
+ else if (str[1] != '-')
+ return -EINVAL;
+
+ if (str[2] != '-')
+ return -EINVAL;
switch (access) {
case 6:
@@ -140,8 +138,6 @@ static int ksym_trace_get_access_type(char *access_str)
case 2:
access = HW_BREAKPOINT_WRITE;
break;
- case 0:
- access = 0;
}
return access;
Commit-ID: 92cf9f8f7e89c6bdbb1a724f879b8b18fc0dfe0f
Gitweb: http://git.kernel.org/tip/92cf9f8f7e89c6bdbb1a724f879b8b18fc0dfe0f
Author: Li Zefan <[email protected]>
AuthorDate: Tue, 7 Jul 2009 13:53:47 +0800
Committer: Ingo Molnar <[email protected]>
CommitDate: Fri, 10 Jul 2009 11:59:42 +0200
ksym_tracer: Fix validation of length of access type
Don't take newline into account, otherwise:
# echo 'pid_max:-w-' > ksym_trace_filter
# echo -n 'pid_max:rw-' > ksym_trace_filter
bash: echo: write error: Invalid argument
Signed-off-by: Li Zefan <[email protected]>
Acked-by: Frederic Weisbecker <[email protected]>
Cc: "K.Prasad" <[email protected]>
Cc: Alan Stern <[email protected]>
Cc: Steven Rostedt <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
---
kernel/trace/trace_ksym.c | 14 +++++++-------
1 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/kernel/trace/trace_ksym.c b/kernel/trace/trace_ksym.c
index 9556009..72fcb46 100644
--- a/kernel/trace/trace_ksym.c
+++ b/kernel/trace/trace_ksym.c
@@ -158,21 +158,21 @@ static int ksym_trace_get_access_type(char *str)
static int parse_ksym_trace_str(char *input_string, char **ksymname,
unsigned long *addr)
{
- char *delimiter = ":";
int ret;
- ret = -EINVAL;
- *ksymname = strsep(&input_string, delimiter);
+ strstrip(input_string);
+
+ *ksymname = strsep(&input_string, ":");
*addr = kallsyms_lookup_name(*ksymname);
/* Check for malformed request: (2), (1) and (5) */
if ((!input_string) ||
- (strlen(input_string) != (KSYM_TRACER_OP_LEN + 1)) ||
- (*addr == 0))
- goto return_code;
+ (strlen(input_string) != KSYM_TRACER_OP_LEN) ||
+ (*addr == 0))
+ return -EINVAL;;
+
ret = ksym_trace_get_access_type(input_string);
-return_code:
return ret;
}
Commit-ID: 011ed56853e07e30653d6f1bfddc56b396218664
Gitweb: http://git.kernel.org/tip/011ed56853e07e30653d6f1bfddc56b396218664
Author: Li Zefan <[email protected]>
AuthorDate: Tue, 7 Jul 2009 13:54:08 +0800
Committer: Ingo Molnar <[email protected]>
CommitDate: Fri, 10 Jul 2009 11:59:42 +0200
ksym_tracer: NIL-terminate user input filter
Make sure the user input string is NULL-terminated.
Signed-off-by: Li Zefan <[email protected]>
Acked-by: Frederic Weisbecker <[email protected]>
Cc: "K.Prasad" <[email protected]>
Cc: Alan Stern <[email protected]>
Cc: Steven Rostedt <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
---
kernel/trace/trace_ksym.c | 7 ++-----
1 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/kernel/trace/trace_ksym.c b/kernel/trace/trace_ksym.c
index 72fcb46..8cbed5a 100644
--- a/kernel/trace/trace_ksym.c
+++ b/kernel/trace/trace_ksym.c
@@ -264,11 +264,7 @@ static ssize_t ksym_trace_filter_write(struct file *file,
unsigned long ksym_addr = 0;
int ret, op, changed = 0;
- /* Ignore echo "" > ksym_trace_filter */
- if (count == 0)
- return 0;
-
- input_string = kzalloc(count, GFP_KERNEL);
+ input_string = kzalloc(count + 1, GFP_KERNEL);
if (!input_string)
return -ENOMEM;
@@ -276,6 +272,7 @@ static ssize_t ksym_trace_filter_write(struct file *file,
kfree(input_string);
return -EFAULT;
}
+ input_string[count] = '\0';
ret = op = parse_ksym_trace_str(input_string, &ksymname, &ksym_addr);
if (ret < 0) {
Commit-ID: 0d109c8f70eab8b9f693bd5caea23012394e4876
Gitweb: http://git.kernel.org/tip/0d109c8f70eab8b9f693bd5caea23012394e4876
Author: Li Zefan <[email protected]>
AuthorDate: Tue, 7 Jul 2009 13:54:28 +0800
Committer: Ingo Molnar <[email protected]>
CommitDate: Fri, 10 Jul 2009 11:59:43 +0200
ksym_tracer: Report error when failed to re-register hbp
When access type is changed, the hw break point will be
unregistered and then be registered again with new access
type. But the registration may fail, in this case, -errno
should be returned.
Signed-off-by: Li Zefan <[email protected]>
Acked-by: Frederic Weisbecker <[email protected]>
Cc: "K.Prasad" <[email protected]>
Cc: Alan Stern <[email protected]>
Cc: Steven Rostedt <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
---
kernel/trace/trace_ksym.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/trace/trace_ksym.c b/kernel/trace/trace_ksym.c
index 8cbed5a..891e3b8 100644
--- a/kernel/trace/trace_ksym.c
+++ b/kernel/trace/trace_ksym.c
@@ -302,13 +302,13 @@ static ssize_t ksym_trace_filter_write(struct file *file,
ret = count;
goto unlock_ret_path;
}
- }
+ } else
+ ret = count;
ksym_filter_entry_count--;
hlist_del_rcu(&(entry->ksym_hlist));
synchronize_rcu();
kfree(entry->ksym_hbp);
kfree(entry);
- ret = count;
goto err_ret;
} else {
/* Check for malformed request: (4) */
Commit-ID: 558df6c8f74ac4a0b9026ef85b0028280f364d96
Gitweb: http://git.kernel.org/tip/558df6c8f74ac4a0b9026ef85b0028280f364d96
Author: Li Zefan <[email protected]>
AuthorDate: Tue, 7 Jul 2009 13:54:48 +0800
Committer: Ingo Molnar <[email protected]>
CommitDate: Fri, 10 Jul 2009 11:59:43 +0200
ksym_tracer: Fix memory leak
- When remove a filter, we leak entry->ksym_hbp->info.name.
- With CONFIG_FTRAC_SELFTEST enabled, we leak ->info.name:
# echo ksym_tracer > current_tracer
# echo 'ksym_selftest_dummy:rw-' > ksym_trace_filter
# echo nop > current_tracer
Signed-off-by: Li Zefan <[email protected]>
Acked-by: Frederic Weisbecker <[email protected]>
Cc: "K.Prasad" <[email protected]>
Cc: Alan Stern <[email protected]>
Cc: Steven Rostedt <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
---
kernel/trace/trace_ksym.c | 61 ++++++++++++++++++++-------------------------
1 files changed, 27 insertions(+), 34 deletions(-)
diff --git a/kernel/trace/trace_ksym.c b/kernel/trace/trace_ksym.c
index 891e3b8..7d349d3 100644
--- a/kernel/trace/trace_ksym.c
+++ b/kernel/trace/trace_ksym.c
@@ -179,7 +179,7 @@ static int parse_ksym_trace_str(char *input_string, char **ksymname,
int process_new_ksym_entry(char *ksymname, int op, unsigned long addr)
{
struct trace_ksym *entry;
- int ret;
+ int ret = -ENOMEM;
if (ksym_filter_entry_count >= KSYM_TRACER_MAX) {
printk(KERN_ERR "ksym_tracer: Maximum limit:(%d) reached. No"
@@ -193,12 +193,13 @@ int process_new_ksym_entry(char *ksymname, int op, unsigned long addr)
return -ENOMEM;
entry->ksym_hbp = kzalloc(sizeof(struct hw_breakpoint), GFP_KERNEL);
- if (!entry->ksym_hbp) {
- kfree(entry);
- return -ENOMEM;
- }
+ if (!entry->ksym_hbp)
+ goto err;
+
+ entry->ksym_hbp->info.name = kstrdup(ksymname, GFP_KERNEL);
+ if (!entry->ksym_hbp->info.name)
+ goto err;
- entry->ksym_hbp->info.name = ksymname;
entry->ksym_hbp->info.type = op;
entry->ksym_addr = entry->ksym_hbp->info.address = addr;
#ifdef CONFIG_X86
@@ -210,14 +211,18 @@ int process_new_ksym_entry(char *ksymname, int op, unsigned long addr)
if (ret < 0) {
printk(KERN_INFO "ksym_tracer request failed. Try again"
" later!!\n");
- kfree(entry->ksym_hbp);
- kfree(entry);
- return -EAGAIN;
+ ret = -EAGAIN;
+ goto err;
}
hlist_add_head_rcu(&(entry->ksym_hlist), &ksym_filter_head);
ksym_filter_entry_count++;
-
return 0;
+err:
+ if (entry->ksym_hbp)
+ kfree(entry->ksym_hbp->info.name);
+ kfree(entry->ksym_hbp);
+ kfree(entry);
+ return ret;
}
static ssize_t ksym_trace_filter_read(struct file *filp, char __user *ubuf,
@@ -289,7 +294,7 @@ static ssize_t ksym_trace_filter_write(struct file *file,
if (entry->ksym_hbp->info.type != op)
changed = 1;
else
- goto err_ret;
+ goto out;
break;
}
}
@@ -298,34 +303,29 @@ static ssize_t ksym_trace_filter_write(struct file *file,
entry->ksym_hbp->info.type = op;
if (op > 0) {
ret = register_kernel_hw_breakpoint(entry->ksym_hbp);
- if (ret == 0) {
- ret = count;
- goto unlock_ret_path;
- }
- } else
- ret = count;
+ if (ret == 0)
+ goto out;
+ }
ksym_filter_entry_count--;
hlist_del_rcu(&(entry->ksym_hlist));
synchronize_rcu();
+ kfree(entry->ksym_hbp->info.name);
kfree(entry->ksym_hbp);
kfree(entry);
- goto err_ret;
+ goto out;
} else {
/* Check for malformed request: (4) */
if (op == 0)
- goto err_ret;
+ goto out;
ret = process_new_ksym_entry(ksymname, op, ksym_addr);
- if (ret)
- goto err_ret;
}
- ret = count;
- goto unlock_ret_path;
+out:
+ mutex_unlock(&ksym_tracer_mutex);
-err_ret:
kfree(input_string);
-unlock_ret_path:
- mutex_unlock(&ksym_tracer_mutex);
+ if (!ret)
+ ret = count;
return ret;
}
@@ -349,14 +349,7 @@ static void ksym_trace_reset(struct trace_array *tr)
ksym_filter_entry_count--;
hlist_del_rcu(&(entry->ksym_hlist));
synchronize_rcu();
- /* Free the 'input_string' only if reset
- * after startup self-test
- */
-#ifdef CONFIG_FTRACE_SELFTEST
- if (strncmp(entry->ksym_hbp->info.name, KSYM_SELFTEST_ENTRY,
- strlen(KSYM_SELFTEST_ENTRY)) != 0)
-#endif /* CONFIG_FTRACE_SELFTEST*/
- kfree(entry->ksym_hbp->info.name);
+ kfree(entry->ksym_hbp->info.name);
kfree(entry->ksym_hbp);
kfree(entry);
}
Commit-ID: 9d7e934408b52cd53dd85270eb36941a6a318cc5
Gitweb: http://git.kernel.org/tip/9d7e934408b52cd53dd85270eb36941a6a318cc5
Author: Li Zefan <[email protected]>
AuthorDate: Tue, 7 Jul 2009 13:55:18 +0800
Committer: Ingo Molnar <[email protected]>
CommitDate: Fri, 10 Jul 2009 11:59:44 +0200
ksym_tracer: Fix the output of stat tracing
- make ksym_tracer_stat_start() return head->first instead of
&head->first
- make the output properly aligned
Before:
Access type Symbol Counter
NA <NA> 0
RW pid_max 0
After:
Access Type Symbol Counter
----------- ------ -------
RW pid_max 0
Signed-off-by: Li Zefan <[email protected]>
Acked-by: Frederic Weisbecker <[email protected]>
Cc: "K.Prasad" <[email protected]>
Cc: Alan Stern <[email protected]>
Cc: Steven Rostedt <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
---
kernel/trace/trace_ksym.c | 20 +++++++++++---------
1 files changed, 11 insertions(+), 9 deletions(-)
diff --git a/kernel/trace/trace_ksym.c b/kernel/trace/trace_ksym.c
index 7d349d3..1256a6e 100644
--- a/kernel/trace/trace_ksym.c
+++ b/kernel/trace/trace_ksym.c
@@ -453,8 +453,10 @@ device_initcall(init_ksym_trace);
#ifdef CONFIG_PROFILE_KSYM_TRACER
static int ksym_tracer_stat_headers(struct seq_file *m)
{
- seq_printf(m, " Access type ");
- seq_printf(m, " Symbol Counter \n");
+ seq_puts(m, " Access Type ");
+ seq_puts(m, " Symbol Counter\n");
+ seq_puts(m, " ----------- ");
+ seq_puts(m, " ------ -------\n");
return 0;
}
@@ -472,27 +474,27 @@ static int ksym_tracer_stat_show(struct seq_file *m, void *v)
switch (access_type) {
case HW_BREAKPOINT_WRITE:
- seq_printf(m, " W ");
+ seq_puts(m, " W ");
break;
case HW_BREAKPOINT_RW:
- seq_printf(m, " RW ");
+ seq_puts(m, " RW ");
break;
default:
- seq_printf(m, " NA ");
+ seq_puts(m, " NA ");
}
if (lookup_symbol_name(entry->ksym_addr, fn_name) >= 0)
- seq_printf(m, " %s ", fn_name);
+ seq_printf(m, " %-36s", fn_name);
else
- seq_printf(m, " <NA> ");
+ seq_printf(m, " %-36s", "<NA>");
+ seq_printf(m, " %15lu\n", entry->counter);
- seq_printf(m, "%15lu\n", entry->counter);
return 0;
}
static void *ksym_tracer_stat_start(struct tracer_stat *trace)
{
- return &(ksym_filter_head.first);
+ return ksym_filter_head.first;
}
static void *