fix: lookup_fd_rcu replaced by lookup_fdget_rcu in linux 6.7.0-rc1

See upstream commit:

    commit 0ede61d8589cc2d93aa78230d74ac58b5b8d0244
    Author: Christian Brauner <brauner@kernel.org>
    Date:   Fri Sep 29 08:45:59 2023 +0200

        file: convert to SLAB_TYPESAFE_BY_RCU

        In recent discussions around some performance improvements in the file
        handling area we discussed switching the file cache to rely on
        SLAB_TYPESAFE_BY_RCU which allows us to get rid of call_rcu() based
        freeing for files completely. This is a pretty sensitive change overall
        but it might actually be worth doing.

        The main downside is the subtlety. The other one is that we should
        really wait for Jann's patch to land that enables KASAN to handle
        SLAB_TYPESAFE_BY_RCU UAFs. Currently it doesn't but a patch for this
        exists.

        With SLAB_TYPESAFE_BY_RCU objects may be freed and reused multiple times
        which requires a few changes. So it isn't sufficient anymore to just
        acquire a reference to the file in question under rcu using
        atomic_long_inc_not_zero() since the file might have already been
        recycled and someone else might have bumped the reference.

        In other words, callers might see reference count bumps from newer
        users. For this reason it is necessary to verify that the pointer is the
        same before and after the reference count increment. This pattern can be
        seen in get_file_rcu() and __files_get_rcu().

        In addition, it isn't possible to access or check fields in struct file
        without first aqcuiring a reference on it. Not doing that was always
        very dodgy and it was only usable for non-pointer data in struct file.
        With SLAB_TYPESAFE_BY_RCU it is necessary that callers first acquire a
        reference under rcu or they must hold the files_lock of the fdtable.
        Failing to do either one of this is a bug.

        Thanks to Jann for pointing out that we need to ensure memory ordering
        between reallocations and pointer check by ensuring that all subsequent
        loads have a dependency on the second load in get_file_rcu() and
        providing a fixup that was folded into this patch.

Signed-off-by: Kienan Stewart <kstewart@efficios.com>
Change-Id: Iba3663f19a54820afd31a8eeec24b3b5d4b06589
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>

diff --git a/include/wrapper/fdtable.h b/include/wrapper/fdtable.h
index e07dfebc91c5dd6e64624363a2bd791769c70c97..9c11d02e9b853bae014ed9afa246541f04a45193 100644 (file)
--- a/include/wrapper/fdtable.h
+++ b/include/wrapper/fdtable.h
@@ -12,17 +12,32 @@
 #include <linux/fdtable.h>
 #include <linux/sched.h>
 
-#if (LTTNG_LINUX_VERSION_CODE >= LTTNG_KERNEL_VERSION(5,11,0))
+#if (LTTNG_LINUX_VERSION_CODE >= LTTNG_KERNEL_VERSION(6,7,0))
 static inline
-struct file *lttng_lookup_fd_rcu(unsigned int fd)
+struct file *lttng_lookup_fdget_rcu(unsigned int fd)
 {
-       return lookup_fd_rcu(fd);
+       return lookup_fdget_rcu(fd);
+}
+
+#elif (LTTNG_LINUX_VERSION_CODE >= LTTNG_KERNEL_VERSION(5,11,0))
+static inline
+struct file *lttng_lookup_fdget_rcu(unsigned int fd)
+{
+       struct file* file = lookup_fd_rcu(fd);
+
+       if (unlikely(!file || !get_file_rcu(file)))
+               return NULL;
+       return file;
 }
 #else
 static inline
-struct file *lttng_lookup_fd_rcu(unsigned int fd)
+struct file *lttng_lookup_fdget_rcu(unsigned int fd)
 {
-       return fcheck(fd);
+       struct file* file = fcheck(fd);
+
+       if (unlikely(!file || !get_file_rcu(file)))
+               return NULL;
+       return file;
 }
 #endif
 

diff --git a/src/probes/lttng-uprobes.c b/src/probes/lttng-uprobes.c
index ae5d09419ad11fca88b4de9bc039fbc6d7040783..38cd0f84c604a273e92b6b7d9bd8886070d505f6 100644 (file)
--- a/src/probes/lttng-uprobes.c
+++ b/src/probes/lttng-uprobes.c
@@ -10,6 +10,7 @@
  */
 
 #include <wrapper/fdtable.h>
+#include <linux/file.h>
 #include <linux/list.h>
 #include <linux/module.h>
 #include <linux/namei.h>
@@ -180,7 +181,7 @@ static struct inode *get_inode_from_fd(int fd)
         * Returns the file backing the given fd. Needs to be done inside an RCU
         * critical section.
         */
-       file = lttng_lookup_fd_rcu(fd);
+       file = lttng_lookup_fdget_rcu(fd);
        if (file == NULL) {
                printk(KERN_WARNING "LTTng: Cannot access file backing the fd(%d)\n", fd);
                inode = NULL;
@@ -191,8 +192,11 @@ static struct inode *get_inode_from_fd(int fd)
        inode = igrab(file->f_path.dentry->d_inode);
        if (inode == NULL)
                printk(KERN_WARNING "LTTng: Cannot grab a reference on the inode.\n");
+
 error:
        rcu_read_unlock();
+       if (file)
+               fput(file);
        return inode;
 }