Skip to content

Commit 9466b6a

Browse files
committed
Merge tag 'trace-v6.11-rc2' of git://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace
Pull tracing fixes from Steven Rostedt: - Have reading of event format files test if the metadata still exists. When a event is freed, a flag (EVENT_FILE_FL_FREED) in the metadata is set to state that it is to prevent any new references to it from happening while waiting for existing references to close. When the last reference closes, the metadata is freed. But the "format" was missing a check to this flag (along with some other files) that allowed new references to happen, and a use-after-free bug to occur. - Have the trace event meta data use the refcount infrastructure instead of relying on its own atomic counters. - Have tracefs inodes use alloc_inode_sb() for allocation instead of using kmem_cache_alloc() directly. - Have eventfs_create_dir() return an ERR_PTR instead of NULL as the callers expect a real object or an ERR_PTR. - Have release_ei() use call_srcu() and not call_rcu() as all the protection is on SRCU and not RCU. - Fix ftrace_graph_ret_addr() to use the task passed in and not current. - Fix overflow bug in get_free_elt() where the counter can overflow the integer and cause an infinite loop. - Remove unused function ring_buffer_nr_pages() - Have tracefs freeing use the inode RCU infrastructure instead of creating its own. When the kernel had randomize structure fields enabled, the rcu field of the tracefs_inode was overlapping the rcu field of the inode structure, and corrupting it. Instead, use the destroy_inode() callback to do the initial cleanup of the code, and then have free_inode() free it. * tag 'trace-v6.11-rc2' of git://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace: tracefs: Use generic inode RCU for synchronizing freeing ring-buffer: Remove unused function ring_buffer_nr_pages() tracing: Fix overflow in get_free_elt() function_graph: Fix the ret_stack used by ftrace_graph_ret_addr() eventfs: Use SRCU for freeing eventfs_inodes eventfs: Don't return NULL in eventfs_create_dir() tracefs: Fix inode allocation tracing: Use refcount for trace_event_file reference counter tracing: Have format file honor EVENT_FILE_FL_FREED
2 parents b3f5620 + 0b6743b commit 9466b6a

File tree

13 files changed

+66
-54
lines changed

13 files changed

+66
-54
lines changed

fs/tracefs/event_inode.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ static void release_ei(struct kref *ref)
112112
entry->release(entry->name, ei->data);
113113
}
114114

115-
call_rcu(&ei->rcu, free_ei_rcu);
115+
call_srcu(&eventfs_srcu, &ei->rcu, free_ei_rcu);
116116
}
117117

118118
static inline void put_ei(struct eventfs_inode *ei)
@@ -736,7 +736,7 @@ struct eventfs_inode *eventfs_create_dir(const char *name, struct eventfs_inode
736736
/* Was the parent freed? */
737737
if (list_empty(&ei->list)) {
738738
cleanup_ei(ei);
739-
ei = NULL;
739+
ei = ERR_PTR(-EBUSY);
740740
}
741741
return ei;
742742
}

fs/tracefs/inode.c

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ static struct inode *tracefs_alloc_inode(struct super_block *sb)
4242
struct tracefs_inode *ti;
4343
unsigned long flags;
4444

45-
ti = kmem_cache_alloc(tracefs_inode_cachep, GFP_KERNEL);
45+
ti = alloc_inode_sb(sb, tracefs_inode_cachep, GFP_KERNEL);
4646
if (!ti)
4747
return NULL;
4848

@@ -53,24 +53,21 @@ static struct inode *tracefs_alloc_inode(struct super_block *sb)
5353
return &ti->vfs_inode;
5454
}
5555

56-
static void tracefs_free_inode_rcu(struct rcu_head *rcu)
56+
static void tracefs_free_inode(struct inode *inode)
5757
{
58-
struct tracefs_inode *ti;
58+
struct tracefs_inode *ti = get_tracefs(inode);
5959

60-
ti = container_of(rcu, struct tracefs_inode, rcu);
6160
kmem_cache_free(tracefs_inode_cachep, ti);
6261
}
6362

64-
static void tracefs_free_inode(struct inode *inode)
63+
static void tracefs_destroy_inode(struct inode *inode)
6564
{
6665
struct tracefs_inode *ti = get_tracefs(inode);
6766
unsigned long flags;
6867

6968
spin_lock_irqsave(&tracefs_inode_lock, flags);
7069
list_del_rcu(&ti->list);
7170
spin_unlock_irqrestore(&tracefs_inode_lock, flags);
72-
73-
call_rcu(&ti->rcu, tracefs_free_inode_rcu);
7471
}
7572

7673
static ssize_t default_read_file(struct file *file, char __user *buf,
@@ -437,6 +434,7 @@ static int tracefs_drop_inode(struct inode *inode)
437434
static const struct super_operations tracefs_super_operations = {
438435
.alloc_inode = tracefs_alloc_inode,
439436
.free_inode = tracefs_free_inode,
437+
.destroy_inode = tracefs_destroy_inode,
440438
.drop_inode = tracefs_drop_inode,
441439
.statfs = simple_statfs,
442440
.show_options = tracefs_show_options,

fs/tracefs/internal.h

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,7 @@ enum {
1010
};
1111

1212
struct tracefs_inode {
13-
union {
14-
struct inode vfs_inode;
15-
struct rcu_head rcu;
16-
};
13+
struct inode vfs_inode;
1714
/* The below gets initialized with memset_after(ti, 0, vfs_inode) */
1815
struct list_head list;
1916
unsigned long flags;

include/linux/ring_buffer.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,6 @@ void ring_buffer_set_clock(struct trace_buffer *buffer,
193193
void ring_buffer_set_time_stamp_abs(struct trace_buffer *buffer, bool abs);
194194
bool ring_buffer_time_stamp_abs(struct trace_buffer *buffer);
195195

196-
size_t ring_buffer_nr_pages(struct trace_buffer *buffer, int cpu);
197196
size_t ring_buffer_nr_dirty_pages(struct trace_buffer *buffer, int cpu);
198197

199198
struct buffer_data_read_page;

include/linux/trace_events.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -680,7 +680,7 @@ struct trace_event_file {
680680
* caching and such. Which is mostly OK ;-)
681681
*/
682682
unsigned long flags;
683-
atomic_t ref; /* ref count for opened files */
683+
refcount_t ref; /* ref count for opened files */
684684
atomic_t sm_ref; /* soft-mode reference counter */
685685
atomic_t tm_ref; /* trigger-mode reference counter */
686686
};

kernel/trace/fgraph.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -902,7 +902,7 @@ unsigned long ftrace_graph_ret_addr(struct task_struct *task, int *idx,
902902

903903
i = *idx ? : task->curr_ret_stack;
904904
while (i > 0) {
905-
ret_stack = get_ret_stack(current, i, &i);
905+
ret_stack = get_ret_stack(task, i, &i);
906906
if (!ret_stack)
907907
break;
908908
/*

kernel/trace/ring_buffer.c

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -692,18 +692,6 @@ u64 ring_buffer_event_time_stamp(struct trace_buffer *buffer,
692692
return ts;
693693
}
694694

695-
/**
696-
* ring_buffer_nr_pages - get the number of buffer pages in the ring buffer
697-
* @buffer: The ring_buffer to get the number of pages from
698-
* @cpu: The cpu of the ring_buffer to get the number of pages from
699-
*
700-
* Returns the number of pages used by a per_cpu buffer of the ring buffer.
701-
*/
702-
size_t ring_buffer_nr_pages(struct trace_buffer *buffer, int cpu)
703-
{
704-
return buffer->buffers[cpu]->nr_pages;
705-
}
706-
707695
/**
708696
* ring_buffer_nr_dirty_pages - get the number of used pages in the ring buffer
709697
* @buffer: The ring_buffer to get the number of pages from

kernel/trace/trace.h

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1634,6 +1634,29 @@ static inline void *event_file_data(struct file *filp)
16341634
extern struct mutex event_mutex;
16351635
extern struct list_head ftrace_events;
16361636

1637+
/*
1638+
* When the trace_event_file is the filp->i_private pointer,
1639+
* it must be taken under the event_mutex lock, and then checked
1640+
* if the EVENT_FILE_FL_FREED flag is set. If it is, then the
1641+
* data pointed to by the trace_event_file can not be trusted.
1642+
*
1643+
* Use the event_file_file() to access the trace_event_file from
1644+
* the filp the first time under the event_mutex and check for
1645+
* NULL. If it is needed to be retrieved again and the event_mutex
1646+
* is still held, then the event_file_data() can be used and it
1647+
* is guaranteed to be valid.
1648+
*/
1649+
static inline struct trace_event_file *event_file_file(struct file *filp)
1650+
{
1651+
struct trace_event_file *file;
1652+
1653+
lockdep_assert_held(&event_mutex);
1654+
file = READ_ONCE(file_inode(filp)->i_private);
1655+
if (!file || file->flags & EVENT_FILE_FL_FREED)
1656+
return NULL;
1657+
return file;
1658+
}
1659+
16371660
extern const struct file_operations event_trigger_fops;
16381661
extern const struct file_operations event_hist_fops;
16391662
extern const struct file_operations event_hist_debug_fops;

kernel/trace/trace_events.c

Lines changed: 24 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -992,18 +992,18 @@ static void remove_subsystem(struct trace_subsystem_dir *dir)
992992

993993
void event_file_get(struct trace_event_file *file)
994994
{
995-
atomic_inc(&file->ref);
995+
refcount_inc(&file->ref);
996996
}
997997

998998
void event_file_put(struct trace_event_file *file)
999999
{
1000-
if (WARN_ON_ONCE(!atomic_read(&file->ref))) {
1000+
if (WARN_ON_ONCE(!refcount_read(&file->ref))) {
10011001
if (file->flags & EVENT_FILE_FL_FREED)
10021002
kmem_cache_free(file_cachep, file);
10031003
return;
10041004
}
10051005

1006-
if (atomic_dec_and_test(&file->ref)) {
1006+
if (refcount_dec_and_test(&file->ref)) {
10071007
/* Count should only go to zero when it is freed */
10081008
if (WARN_ON_ONCE(!(file->flags & EVENT_FILE_FL_FREED)))
10091009
return;
@@ -1386,12 +1386,12 @@ event_enable_read(struct file *filp, char __user *ubuf, size_t cnt,
13861386
char buf[4] = "0";
13871387

13881388
mutex_lock(&event_mutex);
1389-
file = event_file_data(filp);
1389+
file = event_file_file(filp);
13901390
if (likely(file))
13911391
flags = file->flags;
13921392
mutex_unlock(&event_mutex);
13931393

1394-
if (!file || flags & EVENT_FILE_FL_FREED)
1394+
if (!file)
13951395
return -ENODEV;
13961396

13971397
if (flags & EVENT_FILE_FL_ENABLED &&
@@ -1424,8 +1424,8 @@ event_enable_write(struct file *filp, const char __user *ubuf, size_t cnt,
14241424
case 1:
14251425
ret = -ENODEV;
14261426
mutex_lock(&event_mutex);
1427-
file = event_file_data(filp);
1428-
if (likely(file && !(file->flags & EVENT_FILE_FL_FREED))) {
1427+
file = event_file_file(filp);
1428+
if (likely(file)) {
14291429
ret = tracing_update_buffers(file->tr);
14301430
if (ret < 0) {
14311431
mutex_unlock(&event_mutex);
@@ -1540,7 +1540,8 @@ enum {
15401540

15411541
static void *f_next(struct seq_file *m, void *v, loff_t *pos)
15421542
{
1543-
struct trace_event_call *call = event_file_data(m->private);
1543+
struct trace_event_file *file = event_file_data(m->private);
1544+
struct trace_event_call *call = file->event_call;
15441545
struct list_head *common_head = &ftrace_common_fields;
15451546
struct list_head *head = trace_get_fields(call);
15461547
struct list_head *node = v;
@@ -1572,7 +1573,8 @@ static void *f_next(struct seq_file *m, void *v, loff_t *pos)
15721573

15731574
static int f_show(struct seq_file *m, void *v)
15741575
{
1575-
struct trace_event_call *call = event_file_data(m->private);
1576+
struct trace_event_file *file = event_file_data(m->private);
1577+
struct trace_event_call *call = file->event_call;
15761578
struct ftrace_event_field *field;
15771579
const char *array_descriptor;
15781580

@@ -1627,12 +1629,14 @@ static int f_show(struct seq_file *m, void *v)
16271629

16281630
static void *f_start(struct seq_file *m, loff_t *pos)
16291631
{
1632+
struct trace_event_file *file;
16301633
void *p = (void *)FORMAT_HEADER;
16311634
loff_t l = 0;
16321635

16331636
/* ->stop() is called even if ->start() fails */
16341637
mutex_lock(&event_mutex);
1635-
if (!event_file_data(m->private))
1638+
file = event_file_file(m->private);
1639+
if (!file)
16361640
return ERR_PTR(-ENODEV);
16371641

16381642
while (l < *pos && p)
@@ -1706,8 +1710,8 @@ event_filter_read(struct file *filp, char __user *ubuf, size_t cnt,
17061710
trace_seq_init(s);
17071711

17081712
mutex_lock(&event_mutex);
1709-
file = event_file_data(filp);
1710-
if (file && !(file->flags & EVENT_FILE_FL_FREED))
1713+
file = event_file_file(filp);
1714+
if (file)
17111715
print_event_filter(file, s);
17121716
mutex_unlock(&event_mutex);
17131717

@@ -1736,9 +1740,13 @@ event_filter_write(struct file *filp, const char __user *ubuf, size_t cnt,
17361740
return PTR_ERR(buf);
17371741

17381742
mutex_lock(&event_mutex);
1739-
file = event_file_data(filp);
1740-
if (file)
1741-
err = apply_event_filter(file, buf);
1743+
file = event_file_file(filp);
1744+
if (file) {
1745+
if (file->flags & EVENT_FILE_FL_FREED)
1746+
err = -ENODEV;
1747+
else
1748+
err = apply_event_filter(file, buf);
1749+
}
17421750
mutex_unlock(&event_mutex);
17431751

17441752
kfree(buf);
@@ -2485,7 +2493,6 @@ static int event_callback(const char *name, umode_t *mode, void **data,
24852493
if (strcmp(name, "format") == 0) {
24862494
*mode = TRACE_MODE_READ;
24872495
*fops = &ftrace_event_format_fops;
2488-
*data = call;
24892496
return 1;
24902497
}
24912498

@@ -2996,7 +3003,7 @@ trace_create_new_event(struct trace_event_call *call,
29963003
atomic_set(&file->tm_ref, 0);
29973004
INIT_LIST_HEAD(&file->triggers);
29983005
list_add(&file->list, &tr->events);
2999-
event_file_get(file);
3006+
refcount_set(&file->ref, 1);
30003007

30013008
return file;
30023009
}

kernel/trace/trace_events_hist.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5601,7 +5601,7 @@ static int hist_show(struct seq_file *m, void *v)
56015601

56025602
mutex_lock(&event_mutex);
56035603

5604-
event_file = event_file_data(m->private);
5604+
event_file = event_file_file(m->private);
56055605
if (unlikely(!event_file)) {
56065606
ret = -ENODEV;
56075607
goto out_unlock;
@@ -5880,7 +5880,7 @@ static int hist_debug_show(struct seq_file *m, void *v)
58805880

58815881
mutex_lock(&event_mutex);
58825882

5883-
event_file = event_file_data(m->private);
5883+
event_file = event_file_file(m->private);
58845884
if (unlikely(!event_file)) {
58855885
ret = -ENODEV;
58865886
goto out_unlock;

0 commit comments

Comments
 (0)