On Mon, Jan 30, 2017 at 07:41:59PM +0100, Dmitry Vyukov wrote: > Hello, > > The following program triggers use-after-free in timerfd_remove_cancel: > https://gist.githubusercontent.com/dvyukov/202576d437c84ffbbe52e9ccd77e1b44/raw/5562bff8626a73627157331ea2b837f59080ac84/gistfile1.txt > > BUG: KASAN: use-after-free in __list_del include/linux/list.h:104 > [inline] at addr ffff88006bab1410 > BUG: KASAN: use-after-free in __list_del_entry > include/linux/list.h:119 [inline] at addr ffff88006bab1410 > BUG: KASAN: use-after-free in list_del_rcu include/linux/rculist.h:129 > [inline] at addr ffff88006bab1410 > BUG: KASAN: use-after-free in timerfd_remove_cancel fs/timerfd.c:120 > [inline] at addr ffff88006bab1410 > BUG: KASAN: use-after-free in timerfd_release+0x28e/0x310 > fs/timerfd.c:209 at addr ffff88006bab1410 > Write of size 8 by task a.out/2897 > [..] > Seems that ctx->might_cancel is racy. > Indeed it is. Can you try the patch below please. If it works I'll send it in a nicer form. diff --git a/fs/timerfd.c b/fs/timerfd.c index c173cc1..63f91c3 100644 --- a/fs/timerfd.c +++ b/fs/timerfd.c @@ -112,14 +112,30 @@ void timerfd_clock_was_set(void) rcu_read_unlock(); } +static void timerfd_set_cancel(struct timerfd_ctx *ctx) +{ + if (ctx->might_cancel) + return; + + spin_lock(&cancel_lock); + if (!ctx->might_cancel) { + ctx->might_cancel = true; + list_add_rcu(&ctx->clist, &cancel_list); + } + spin_unlock(&cancel_lock); +} + static void timerfd_remove_cancel(struct timerfd_ctx *ctx) { + if (!ctx->might_cancel) + return; + + spin_lock(&cancel_lock); if (ctx->might_cancel) { ctx->might_cancel = false; - spin_lock(&cancel_lock); list_del_rcu(&ctx->clist); - spin_unlock(&cancel_lock); } + spin_unlock(&cancel_lock); } static bool timerfd_canceled(struct timerfd_ctx *ctx) @@ -134,16 +150,10 @@ static void timerfd_setup_cancel(struct timerfd_ctx *ctx, int flags) { if ((ctx->clockid == CLOCK_REALTIME || ctx->clockid == CLOCK_REALTIME_ALARM) && - (flags & TFD_TIMER_ABSTIME) && (flags & TFD_TIMER_CANCEL_ON_SET)) { - if (!ctx->might_cancel) { - ctx->might_cancel = true; - spin_lock(&cancel_lock); - list_add_rcu(&ctx->clist, &cancel_list); - spin_unlock(&cancel_lock); - } - } else if (ctx->might_cancel) { + (flags & TFD_TIMER_ABSTIME) && (flags & TFD_TIMER_CANCEL_ON_SET)) + timerfd_set_cancel(ctx); + else timerfd_remove_cancel(ctx); - } } static ktime_t timerfd_get_remaining(struct timerfd_ctx *ctx) -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
