[RAMEN9610-22013]futex: Fix inode life-time issue

commit 8019ad13ef7f64be44d4f892af9c840179009254 upstream.

As reported by Jann, ihold() does not in fact guarantee inode
persistence. And instead of making it so, replace the usage of inode
pointers with a per boot, machine wide, unique inode identifier.

This sequence number is global, but shared (file backed) futexes are
rare enough that this should not become a performance issue.

Change-Id: I71ce183a546dc536fddd1c1f982fa96a577e5a2d
Reported-by: Jann Horn <jannh@google.com>
Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/fs/inode.c b/fs/inode.c
index 602e28f0c762b28514dcae9fd704c6b277a8fbce..f86b11e2863b9d5443ebe9dffd088a332f320ef1 100644 (file)
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -135,6 +135,7 @@ int inode_init_always(struct super_block *sb, struct inode *inode)
        inode->i_sb = sb;
        inode->i_blkbits = sb->s_blocksize_bits;
        inode->i_flags = 0;
+       atomic64_set(&inode->i_sequence, 0);
        atomic_set(&inode->i_count, 1);
        inode->i_op = &empty_iops;
        inode->i_fop = &no_open_fops;

diff --git a/include/linux/fs.h b/include/linux/fs.h
index c62a079fbf39f1efe94a2c8592e09eea4c3141c4..55d8932dbf904c9e5ca0c54a7116ae51121b2c4c 100644 (file)
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -642,6 +642,7 @@ struct inode {
                struct rcu_head         i_rcu;
        };
        u64                     i_version;
+       atomic64_t              i_sequence; /* see futex */
        atomic_t                i_count;
        atomic_t                i_dio_count;
        atomic_t                i_writecount;

diff --git a/include/linux/futex.h b/include/linux/futex.h
index c0fb9a24bbd245c980723bce4b0dc5777bd4fb64..3b770567b8d9fe8a915d9301049c8be8b2db97a9 100644 (file)
--- a/include/linux/futex.h
+++ b/include/linux/futex.h
@@ -35,23 +35,26 @@ handle_futex_death(u32 __user *uaddr, struct task_struct *curr, int pi);
 
 union futex_key {
        struct {
+               u64 i_seq;
                unsigned long pgoff;
-               struct inode *inode;
-               int offset;
+               unsigned int offset;
        } shared;
        struct {
+               union {
+                       struct mm_struct *mm;
+                       u64 __tmp;
+               };
                unsigned long address;
-               struct mm_struct *mm;
-               int offset;
+               unsigned int offset;
        } private;
        struct {
+               u64 ptr;
                unsigned long word;
-               void *ptr;
-               int offset;
+               unsigned int offset;
        } both;
 };
 
-#define FUTEX_KEY_INIT (union futex_key) { .both = { .ptr = NULL } }
+#define FUTEX_KEY_INIT (union futex_key) { .both = { .ptr = 0ULL } }
 
 #ifdef CONFIG_FUTEX
 extern void exit_robust_list(struct task_struct *curr);

diff --git a/kernel/futex.c b/kernel/futex.c
index f2fa48c6c47662312e817c01c60b9ea9b87fd1e1..6c37a7723116d038db4658827ae8a4f7ea94e72a 100644 (file)
--- a/kernel/futex.c
+++ b/kernel/futex.c
@@ -436,7 +436,7 @@ static void get_futex_key_refs(union futex_key *key)
 
        switch (key->both.offset & (FUT_OFF_INODE|FUT_OFF_MMSHARED)) {
        case FUT_OFF_INODE:
-               ihold(key->shared.inode); /* implies smp_mb(); (B) */
+               smp_mb();               /* explicit smp_mb(); (B) */
                break;
        case FUT_OFF_MMSHARED:
                futex_get_mm(key); /* implies smp_mb(); (B) */
@@ -470,7 +470,6 @@ static void drop_futex_key_refs(union futex_key *key)
 
        switch (key->both.offset & (FUT_OFF_INODE|FUT_OFF_MMSHARED)) {
        case FUT_OFF_INODE:
-               iput(key->shared.inode);
                break;
        case FUT_OFF_MMSHARED:
                mmdrop(key->private.mm);
@@ -478,6 +477,46 @@ static void drop_futex_key_refs(union futex_key *key)
        }
 }
 
+/*
+ * Generate a machine wide unique identifier for this inode.
+ *
+ * This relies on u64 not wrapping in the life-time of the machine; which with
+ * 1ns resolution means almost 585 years.
+ *
+ * This further relies on the fact that a well formed program will not unmap
+ * the file while it has a (shared) futex waiting on it. This mapping will have
+ * a file reference which pins the mount and inode.
+ *
+ * If for some reason an inode gets evicted and read back in again, it will get
+ * a new sequence number and will _NOT_ match, even though it is the exact same
+ * file.
+ *
+ * It is important that match_futex() will never have a false-positive, esp.
+ * for PI futexes that can mess up the state. The above argues that false-negatives
+ * are only possible for malformed programs.
+ */
+static u64 get_inode_sequence_number(struct inode *inode)
+{
+       static atomic64_t i_seq;
+       u64 old;
+
+       /* Does the inode already have a sequence number? */
+       old = atomic64_read(&inode->i_sequence);
+       if (likely(old))
+               return old;
+
+       for (;;) {
+               u64 new = atomic64_add_return(1, &i_seq);
+               if (WARN_ON_ONCE(!new))
+                       continue;
+
+               old = atomic64_cmpxchg_relaxed(&inode->i_sequence, 0, new);
+               if (old)
+                       return old;
+               return new;
+       }
+}
+
 /**
  * get_futex_key() - Get parameters which are the keys for a futex
  * @uaddr:     virtual address of the futex
@@ -490,9 +529,15 @@ static void drop_futex_key_refs(union futex_key *key)
  *
  * The key words are stored in @key on success.
  *
- * For shared mappings, it's (page->index, file_inode(vma->vm_file),
- * offset_within_page).  For private mappings, it's (uaddr, current->mm).
- * We can usually work out the index without swapping in the page.
+ * For shared mappings (when @fshared), the key is:
+ *   ( inode->i_sequence, page->index, offset_within_page )
+ * [ also see get_inode_sequence_number() ]
+ *
+ * For private mappings (or when !@fshared), the key is:
+ *   ( current->mm, address, 0 )
+ *
+ * This allows (cross process, where applicable) identification of the futex
+ * without keeping the page pinned for the duration of the FUTEX_WAIT.
  *
  * lock_page() might sleep, the caller should not hold a spinlock.
  */
@@ -632,8 +677,6 @@ again:
                key->private.mm = mm;
                key->private.address = address;
 
-               get_futex_key_refs(key); /* implies smp_mb(); (B) */
-
        } else {
                struct inode *inode;
 
@@ -665,40 +708,14 @@ again:
                        goto again;
                }
 
-               /*
-                * Take a reference unless it is about to be freed. Previously
-                * this reference was taken by ihold under the page lock
-                * pinning the inode in place so i_lock was unnecessary. The
-                * only way for this check to fail is if the inode was
-                * truncated in parallel which is almost certainly an
-                * application bug. In such a case, just retry.
-                *
-                * We are not calling into get_futex_key_refs() in file-backed
-                * cases, therefore a successful atomic_inc return below will
-                * guarantee that get_futex_key() will still imply smp_mb(); (B).
-                */
-               if (!atomic_inc_not_zero(&inode->i_count)) {
-                       rcu_read_unlock();
-                       put_page(page);
-
-                       goto again;
-               }
-
-               /* Should be impossible but lets be paranoid for now */
-               if (WARN_ON_ONCE(inode->i_mapping != mapping)) {
-                       err = -EFAULT;
-                       rcu_read_unlock();
-                       iput(inode);
-
-                       goto out;
-               }
-
                key->both.offset |= FUT_OFF_INODE; /* inode-based key */
-               key->shared.inode = inode;
+               key->shared.i_seq = get_inode_sequence_number(inode);
                key->shared.pgoff = basepage_index(tail);
                rcu_read_unlock();
        }
 
+       get_futex_key_refs(key); /* implies smp_mb(); (B) */
+
 out:
        put_page(page);
        return err;