• Ye Bin's avatar
    blktrace: fix possible memleak in '__blk_trace_remove' · dcd1a59c
    Ye Bin authored
    When test as follows:
    step1: ioctl(sda, BLKTRACESETUP, &arg)
    step2: ioctl(sda, BLKTRACESTART, NULL)
    step3: ioctl(sda, BLKTRACETEARDOWN, NULL)
    step4: ioctl(sda, BLKTRACESETUP, &arg)
    Got issue as follows:
    debugfs: File 'dropped' in directory 'sda' already present!
    debugfs: File 'msg' in directory 'sda' already present!
    debugfs: File 'trace0' in directory 'sda' already present!
    
    And also find syzkaller report issue like "KASAN: use-after-free Read in relay_switch_subbuf"
    "https://syzkaller.appspot.com/bug?id=13849f0d9b1b818b087341691be6cc3ac6a6bfb7"
    
    If remove block trace without stop(BLKTRACESTOP) block trace, '__blk_trace_remove'
    will just set 'q->blk_trace' with NULL. However, debugfs file isn't removed, so
    will report file already present when call BLKTRACESETUP.
    static int __blk_trace_remove(struct request_queue *q)
    {
            struct blk_trace *bt;
    
            bt = rcu_replace_pointer(q->blk_trace, NULL,
                                     lockdep_is_held(&q->debugfs_mutex));
            if (!bt)
                    return -EINVAL;
    
    	if (bt->trace_state != Blktrace_running)
            	blk_trace_cleanup(q, bt);
    
            return 0;
    }
    
    If do test as follows:
    step1: ioctl(sda, BLKTRACESETUP, &arg)
    step2: ioctl(sda, BLKTRACESTART, NULL)
    step3: ioctl(sda, BLKTRACETEARDOWN, NULL)
    step4: remove sda
    
    There will remove debugfs directory which will remove recursively all file
    under directory.
    >> blk_release_queue
    >>	debugfs_remove_recursive(q->debugfs_dir)
    So all files which created in 'do_blk_trace_setup' are removed, and
    'dentry->d_inode' is NULL. But 'q->blk_trace' is still in 'running_trace_lock',
    'trace_note_tsk' will traverse 'running_trace_lock' all nodes.
    >>trace_note_tsk
    >>  trace_note
    >>    relay_reserve
    >>       relay_switch_subbuf
    >>        d_inode(buf->dentry)->i_size
    
    To solve above issues, reference commit '5afedf67', call 'blk_trace_cleanup'
    unconditionally in '__blk_trace_remove' and first stop block trace in
    'blk_trace_cleanup'.
    Signed-off-by: default avatarYe Bin <yebin10@huawei.com>
    Reviewed-by: default avatarChristoph Hellwig <hch@lst.de>
    Link: https://lore.kernel.org/r/20221019033602.752383-3-yebin@huaweicloud.comSigned-off-by: default avatarJens Axboe <axboe@kernel.dk>
    dcd1a59c
blktrace.c 46.4 KB