On 4/5/20 11:25 PM, Bart Van Assche wrote: > On 2020-04-05 18:27, Eric Sandeen wrote: >> The thing I can't figure out from reading the change log is >> >> 1) what the root cause of the problem is, and >> 2) how this patch fixes it? > > I think that the root cause is that do_blk_trace_setup() uses > debugfs_lookup() and that debugfs_lookup() may return a pointer > associated with a previous incarnation of the block device. > Additionally, I think the following changes fix that problem by using > q->debugfs_dir in the blktrace code instead of debugfs_lookup(): Yep, I gathered that from reading the patch, was just hoping for a commit log that makes it clear. > [ ... ] > --- a/kernel/trace/blktrace.c > +++ b/kernel/trace/blktrace.c > @@ -311,7 +311,6 @@ static void blk_trace_free(struct blk_trace *bt) > debugfs_remove(bt->msg_file); > debugfs_remove(bt->dropped_file); > relay_close(bt->rchan); > - debugfs_remove(bt->dir); > free_percpu(bt->sequence); > free_percpu(bt->msg_data); > kfree(bt); > [ ... ] > @@ -509,21 +510,19 @@ static int do_blk_trace_setup(struct request_queue > *q, char *name, dev_t dev, > > ret = -ENOENT; > > - dir = debugfs_lookup(buts->name, blk_debugfs_root); > - if (!dir) > - bt->dir = dir = debugfs_create_dir(buts->name, blk_debugfs_root); > - > bt->dev = dev; > atomic_set(&bt->dropped, 0); > INIT_LIST_HEAD(&bt->running_list); > > ret = -EIO; > - bt->dropped_file = debugfs_create_file("dropped", 0444, dir, bt, > + bt->dropped_file = debugfs_create_file("dropped", 0444, > + q->debugfs_dir, bt, > &blk_dropped_fops); One thing I'm not sure about, the block_trace *bt still points to a dentry that could get torn down via debugfs_remove_recursive when the queue is released, right, but could later be sent to blk_trace_free again? And yet this does seem to fix the use after free in my testing, so I must be missing something. Thanks, -Eric