Re: timerfd: use-after-free in timerfd_remove_cancel

[Thomas Gleixner]

On Mon, 30 Jan 2017, Dmitry Vyukov wrote:
> 
> Seems that ctx->might_cancel is racy.

Yes, it is. Fix below.

8<-------------------

--- a/fs/timerfd.c
+++ b/fs/timerfd.c
@@ -40,9 +40,12 @@ struct timerfd_ctx {
        short unsigned settime_flags;   /* to show in fdinfo */
        struct rcu_head rcu;
        struct list_head clist;
-       bool might_cancel;
+       unsigned long flags;
 };
 
+/* Bit positions for ctx->flags */
+#define MIGHT_CANCEL   0
+
 static LIST_HEAD(cancel_list);
 static DEFINE_SPINLOCK(cancel_lock);
 
@@ -99,7 +102,7 @@ void timerfd_clock_was_set(void)
 
        rcu_read_lock();
        list_for_each_entry_rcu(ctx, &cancel_list, clist) {
-               if (!ctx->might_cancel)
+               if (!test_bit(MIGHT_CANCEL, &ctx->flags))
                        continue;
                spin_lock_irqsave(&ctx->wqh.lock, flags);
                if (ctx->moffs != moffs) {
@@ -114,8 +117,7 @@ void timerfd_clock_was_set(void)
 
 static void timerfd_remove_cancel(struct timerfd_ctx *ctx)
 {
-       if (ctx->might_cancel) {
-               ctx->might_cancel = false;
+       if (test_and_clear_bit(MIGHT_CANCEL, &ctx->flags)) {
                spin_lock(&cancel_lock);
                list_del_rcu(&ctx->clist);
                spin_unlock(&cancel_lock);
@@ -124,7 +126,7 @@ static void timerfd_remove_cancel(struct
 
 static bool timerfd_canceled(struct timerfd_ctx *ctx)
 {
-       if (!ctx->might_cancel || ctx->moffs != KTIME_MAX)
+       if (!test_bit(MIGHT_CANCEL, &ctx->flags) || ctx->moffs != KTIME_MAX)
                return false;
        ctx->moffs = ktime_mono_to_real(0);
        return true;
@@ -135,13 +137,12 @@ static void timerfd_setup_cancel(struct
        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;
+               if (test_and_set_bit(MIGHT_CANCEL, &ctx->flags)) {
                        spin_lock(&cancel_lock);
                        list_add_rcu(&ctx->clist, &cancel_list);
                        spin_unlock(&cancel_lock);
                }
-       } else if (ctx->might_cancel) {
+       } else {
                timerfd_remove_cancel(ctx);
        }
 }