Commit c1e2148f authored Mar 04, 2020 by Jens Axboe

io_uring: free fixed_file_data after RCU grace period



The percpu refcount protects this structure, and we can have an atomic
switch in progress when exiting. This makes it unsafe to just free the
struct normally, and can trigger the following KASAN warning:

BUG: KASAN: use-after-free in percpu_ref_switch_to_atomic_rcu+0xfa/0x1b0
Read of size 1 at addr ffff888181a19a30 by task swapper/0/0

CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.6.0-rc4+ #5747
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1ubuntu1 04/01/2014
Call Trace:
 <IRQ>
 dump_stack+0x76/0xa0
 print_address_description.constprop.0+0x3b/0x60
 ? percpu_ref_switch_to_atomic_rcu+0xfa/0x1b0
 ? percpu_ref_switch_to_atomic_rcu+0xfa/0x1b0
 __kasan_report.cold+0x1a/0x3d
 ? percpu_ref_switch_to_atomic_rcu+0xfa/0x1b0
 percpu_ref_switch_to_atomic_rcu+0xfa/0x1b0
 rcu_core+0x370/0x830
 ? percpu_ref_exit+0x50/0x50
 ? rcu_note_context_switch+0x7b0/0x7b0
 ? run_rebalance_domains+0x11d/0x140
 __do_softirq+0x10a/0x3e9
 irq_exit+0xd5/0xe0
 smp_apic_timer_interrupt+0x86/0x200
 apic_timer_interrupt+0xf/0x20
 </IRQ>
RIP: 0010:default_idle+0x26/0x1f0

Fix this by punting the final exit and free of the struct to RCU, then
we know that it's safe to do so. Jann suggested the approach of using a
double rcu callback to achieve this. It's important that we do a nested
call_rcu() callback, as otherwise the free could be ordered before the
atomic switch, even if the latter was already queued.

Reported-by:  <syzbot+e017e49c39ab484ac87a@syzkaller.appspotmail.com>
Suggested-by: Jann Horn <jannh@google.com>
Reviewed-by: Paul E. McKenney <paulmck@kernel.org>
Signed-off-by: Jens Axboe <axboe@kernel.dk>

parent 80ad8943

fs/io_uring.c

+22 −2

Original line number	Diff line number	Diff line
		@@ -191,6 +191,7 @@ struct fixed_file_data {
		struct llist_head put_llist;
		struct work_struct ref_work;
		struct completion done;
		struct rcu_head rcu;
		};

		struct io_ring_ctx {
		@@ -5329,6 +5330,26 @@ static void io_file_ref_kill(struct percpu_ref *ref)
		complete(&data->done);
		}

		static void __io_file_ref_exit_and_free(struct rcu_head *rcu)
		{
		struct fixed_file_data *data = container_of(rcu, struct fixed_file_data,
		rcu);
		percpu_ref_exit(&data->refs);
		kfree(data);
		}

		static void io_file_ref_exit_and_free(struct rcu_head *rcu)
		{
		/*
		* We need to order our exit+free call against the potentially
		* existing call_rcu() for switching to atomic. One way to do that
		* is to have this rcu callback queue the final put and free, as we
		* could otherwise have a pre-existing atomic switch complete _after_
		* the free callback we queued.
		*/
		call_rcu(rcu, __io_file_ref_exit_and_free);
		}

		static int io_sqe_files_unregister(struct io_ring_ctx *ctx)
		{
		struct fixed_file_data *data = ctx->file_data;
		@@ -5341,14 +5362,13 @@ static int io_sqe_files_unregister(struct io_ring_ctx *ctx)
		flush_work(&data->ref_work);
		wait_for_completion(&data->done);
		io_ring_file_ref_flush(data);
		percpu_ref_exit(&data->refs);

		__io_sqe_files_unregister(ctx);
		nr_tables = DIV_ROUND_UP(ctx->nr_user_files, IORING_MAX_FILES_TABLE);
		for (i = 0; i < nr_tables; i++)
		kfree(data->table[i].files);
		kfree(data->table);
		kfree(data);
		call_rcu(&data->rcu, io_file_ref_exit_and_free);
		ctx->file_data = NULL;
		ctx->nr_user_files = 0;
		return 0;

Admin message