Skip to content

Commit 1a967e9

Browse files
mhiramatrostedt
authored andcommitted
tracing: Remove "__attribute__()" from the type field of event format
With CONFIG_DEBUG_INFO_BTF=y and PAHOLE_HAS_BTF_TAG=y, `__user` is converted to `__attribute__((btf_type_tag("user")))`. In this case, some syscall events have it for __user data, like below; /sys/kernel/tracing # cat events/syscalls/sys_enter_openat/format name: sys_enter_openat ID: 720 format: field:unsigned short common_type; offset:0; size:2; signed:0; field:unsigned char common_flags; offset:2; size:1; signed:0; field:unsigned char common_preempt_count; offset:3; size:1; signed:0; field:int common_pid; offset:4; size:4; signed:1; field:int __syscall_nr; offset:8; size:4; signed:1; field:int dfd; offset:16; size:8; signed:0; field:const char __attribute__((btf_type_tag("user"))) * filename; offset:24; size:8; signed:0; field:int flags; offset:32; size:8; signed:0; field:umode_t mode; offset:40; size:8; signed:0; Then the trace event filter fails to set the string acceptable flag (FILTER_PTR_STRING) to the field and rejects setting string filter; # echo 'filename.ustring ~ "*ftracetest-dir.wbx24v*"' \ >> events/syscalls/sys_enter_openat/filter sh: write error: Invalid argument # cat error_log [ 723.743637] event filter parse error: error: Expecting numeric field Command: filename.ustring ~ "*ftracetest-dir.wbx24v*" Since this __attribute__ makes format parsing complicated and not needed, remove the __attribute__(.*) from the type string. Cc: Mathieu Desnoyers <[email protected]> Link: https://lore.kernel.org/175376583493.1688759.12333973498014733551.stgit@mhiramat.tok.corp.google.com Signed-off-by: Masami Hiramatsu (Google) <[email protected]> Signed-off-by: Steven Rostedt (Google) <[email protected]>
1 parent 9ba817f commit 1a967e9

File tree

3 files changed

+127
-33
lines changed

3 files changed

+127
-33
lines changed

kernel/trace/trace.c

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5930,17 +5930,27 @@ static inline void trace_insert_eval_map_file(struct module *mod,
59305930
struct trace_eval_map **start, int len) { }
59315931
#endif /* !CONFIG_TRACE_EVAL_MAP_FILE */
59325932

5933-
static void trace_insert_eval_map(struct module *mod,
5934-
struct trace_eval_map **start, int len)
5933+
static void
5934+
trace_event_update_with_eval_map(struct module *mod,
5935+
struct trace_eval_map **start,
5936+
int len)
59355937
{
59365938
struct trace_eval_map **map;
59375939

5938-
if (len <= 0)
5939-
return;
5940+
/* Always run sanitizer only if btf_type_tag attr exists. */
5941+
if (len <= 0) {
5942+
if (!(IS_ENABLED(CONFIG_DEBUG_INFO_BTF) &&
5943+
IS_ENABLED(CONFIG_PAHOLE_HAS_BTF_TAG) &&
5944+
__has_attribute(btf_type_tag)))
5945+
return;
5946+
}
59405947

59415948
map = start;
59425949

5943-
trace_event_eval_update(map, len);
5950+
trace_event_update_all(map, len);
5951+
5952+
if (len <= 0)
5953+
return;
59445954

59455955
trace_insert_eval_map_file(mod, start, len);
59465956
}
@@ -10334,7 +10344,7 @@ static void __init eval_map_work_func(struct work_struct *work)
1033410344
int len;
1033510345

1033610346
len = __stop_ftrace_eval_maps - __start_ftrace_eval_maps;
10337-
trace_insert_eval_map(NULL, __start_ftrace_eval_maps, len);
10347+
trace_event_update_with_eval_map(NULL, __start_ftrace_eval_maps, len);
1033810348
}
1033910349

1034010350
static int __init trace_eval_init(void)
@@ -10387,17 +10397,15 @@ bool module_exists(const char *module)
1038710397

1038810398
static void trace_module_add_evals(struct module *mod)
1038910399
{
10390-
if (!mod->num_trace_evals)
10391-
return;
10392-
1039310400
/*
1039410401
* Modules with bad taint do not have events created, do
1039510402
* not bother with enums either.
1039610403
*/
1039710404
if (trace_module_has_bad_taint(mod))
1039810405
return;
1039910406

10400-
trace_insert_eval_map(mod, mod->trace_evals, mod->num_trace_evals);
10407+
/* Even if no trace_evals, this need to sanitize field types. */
10408+
trace_event_update_with_eval_map(mod, mod->trace_evals, mod->num_trace_evals);
1040110409
}
1040210410

1040310411
#ifdef CONFIG_TRACE_EVAL_MAP_FILE

kernel/trace/trace.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2125,13 +2125,13 @@ static inline const char *get_syscall_name(int syscall)
21252125

21262126
#ifdef CONFIG_EVENT_TRACING
21272127
void trace_event_init(void);
2128-
void trace_event_eval_update(struct trace_eval_map **map, int len);
2128+
void trace_event_update_all(struct trace_eval_map **map, int len);
21292129
/* Used from boot time tracer */
21302130
extern int ftrace_set_clr_event(struct trace_array *tr, char *buf, int set);
21312131
extern int trigger_process_regex(struct trace_event_file *file, char *buff);
21322132
#else
21332133
static inline void __init trace_event_init(void) { }
2134-
static inline void trace_event_eval_update(struct trace_eval_map **map, int len) { }
2134+
static inline void trace_event_update_all(struct trace_eval_map **map, int len) { }
21352135
#endif
21362136

21372137
#ifdef CONFIG_TRACER_SNAPSHOT

kernel/trace/trace_events.c

Lines changed: 107 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -3264,43 +3264,120 @@ static void add_str_to_module(struct module *module, char *str)
32643264
list_add(&modstr->next, &module_strings);
32653265
}
32663266

3267+
#define ATTRIBUTE_STR "__attribute__("
3268+
#define ATTRIBUTE_STR_LEN (sizeof(ATTRIBUTE_STR) - 1)
3269+
3270+
/* Remove all __attribute__() from @type. Return allocated string or @type. */
3271+
static char *sanitize_field_type(const char *type)
3272+
{
3273+
char *attr, *tmp, *next, *ret = (char *)type;
3274+
int depth;
3275+
3276+
next = (char *)type;
3277+
while ((attr = strstr(next, ATTRIBUTE_STR))) {
3278+
/* Retry if "__attribute__(" is a part of another word. */
3279+
if (attr != next && !isspace(attr[-1])) {
3280+
next = attr + ATTRIBUTE_STR_LEN;
3281+
continue;
3282+
}
3283+
3284+
if (ret == type) {
3285+
ret = kstrdup(type, GFP_KERNEL);
3286+
if (WARN_ON_ONCE(!ret))
3287+
return NULL;
3288+
attr = ret + (attr - type);
3289+
}
3290+
3291+
/* the ATTRIBUTE_STR already has the first '(' */
3292+
depth = 1;
3293+
next = attr + ATTRIBUTE_STR_LEN;
3294+
do {
3295+
tmp = strpbrk(next, "()");
3296+
/* There is unbalanced parentheses */
3297+
if (WARN_ON_ONCE(!tmp)) {
3298+
kfree(ret);
3299+
return (char *)type;
3300+
}
3301+
3302+
if (*tmp == '(')
3303+
depth++;
3304+
else
3305+
depth--;
3306+
next = tmp + 1;
3307+
} while (depth > 0);
3308+
next = skip_spaces(next);
3309+
strcpy(attr, next);
3310+
next = attr;
3311+
}
3312+
return ret;
3313+
}
3314+
3315+
static char *find_replacable_eval(const char *type, const char *eval_string,
3316+
int len)
3317+
{
3318+
char *ptr;
3319+
3320+
if (!eval_string)
3321+
return NULL;
3322+
3323+
ptr = strchr(type, '[');
3324+
if (!ptr)
3325+
return NULL;
3326+
ptr++;
3327+
3328+
if (!isalpha(*ptr) && *ptr != '_')
3329+
return NULL;
3330+
3331+
if (strncmp(eval_string, ptr, len) != 0)
3332+
return NULL;
3333+
3334+
return ptr;
3335+
}
3336+
32673337
static void update_event_fields(struct trace_event_call *call,
32683338
struct trace_eval_map *map)
32693339
{
32703340
struct ftrace_event_field *field;
3341+
const char *eval_string = NULL;
32713342
struct list_head *head;
3343+
int len = 0;
32723344
char *ptr;
32733345
char *str;
3274-
int len = strlen(map->eval_string);
32753346

32763347
/* Dynamic events should never have field maps */
3277-
if (WARN_ON_ONCE(call->flags & TRACE_EVENT_FL_DYNAMIC))
3348+
if (call->flags & TRACE_EVENT_FL_DYNAMIC)
32783349
return;
32793350

3351+
if (map) {
3352+
eval_string = map->eval_string;
3353+
len = strlen(map->eval_string);
3354+
}
3355+
32803356
head = trace_get_fields(call);
32813357
list_for_each_entry(field, head, link) {
3282-
ptr = strchr(field->type, '[');
3283-
if (!ptr)
3284-
continue;
3285-
ptr++;
3286-
3287-
if (!isalpha(*ptr) && *ptr != '_')
3288-
continue;
3358+
str = sanitize_field_type(field->type);
3359+
if (!str)
3360+
return;
32893361

3290-
if (strncmp(map->eval_string, ptr, len) != 0)
3291-
continue;
3362+
ptr = find_replacable_eval(str, eval_string, len);
3363+
if (ptr) {
3364+
if (str == field->type) {
3365+
str = kstrdup(field->type, GFP_KERNEL);
3366+
if (WARN_ON_ONCE(!str))
3367+
return;
3368+
ptr = str + (ptr - field->type);
3369+
}
32923370

3293-
str = kstrdup(field->type, GFP_KERNEL);
3294-
if (WARN_ON_ONCE(!str))
3295-
return;
3296-
ptr = str + (ptr - field->type);
3297-
ptr = eval_replace(ptr, map, len);
3298-
/* enum/sizeof string smaller than value */
3299-
if (WARN_ON_ONCE(!ptr)) {
3300-
kfree(str);
3301-
continue;
3371+
ptr = eval_replace(ptr, map, len);
3372+
/* enum/sizeof string smaller than value */
3373+
if (WARN_ON_ONCE(!ptr)) {
3374+
kfree(str);
3375+
continue;
3376+
}
33023377
}
33033378

3379+
if (str == field->type)
3380+
continue;
33043381
/*
33053382
* If the event is part of a module, then we need to free the string
33063383
* when the module is removed. Otherwise, it will stay allocated
@@ -3310,14 +3387,18 @@ static void update_event_fields(struct trace_event_call *call,
33103387
add_str_to_module(call->module, str);
33113388

33123389
field->type = str;
3390+
if (field->filter_type == FILTER_OTHER)
3391+
field->filter_type = filter_assign_type(field->type);
33133392
}
33143393
}
33153394

3316-
void trace_event_eval_update(struct trace_eval_map **map, int len)
3395+
/* Update all events for replacing eval and sanitizing */
3396+
void trace_event_update_all(struct trace_eval_map **map, int len)
33173397
{
33183398
struct trace_event_call *call, *p;
33193399
const char *last_system = NULL;
33203400
bool first = false;
3401+
bool updated;
33213402
int last_i;
33223403
int i;
33233404

@@ -3330,6 +3411,7 @@ void trace_event_eval_update(struct trace_eval_map **map, int len)
33303411
last_system = call->class->system;
33313412
}
33323413

3414+
updated = false;
33333415
/*
33343416
* Since calls are grouped by systems, the likelihood that the
33353417
* next call in the iteration belongs to the same system as the
@@ -3349,8 +3431,12 @@ void trace_event_eval_update(struct trace_eval_map **map, int len)
33493431
}
33503432
update_event_printk(call, map[i]);
33513433
update_event_fields(call, map[i]);
3434+
updated = true;
33523435
}
33533436
}
3437+
/* If not updated yet, update field for sanitizing. */
3438+
if (!updated)
3439+
update_event_fields(call, NULL);
33543440
cond_resched();
33553441
}
33563442
up_write(&trace_event_sem);

0 commit comments

Comments
 (0)