LU-18845 obdclass: NPE in with_imp_locked_nested

Unable to handle kernel NULL pointer dereference
at virtual address 0000000000000588

down_read+0x28/0x100
active_show+0x2c/0x90 [osp]
lustre_attr_show+0x1c/0x2c [obdclass]

active_show calls macro with_imp_locked with an
obd_device, which in turn gets a lock on
u.cli.cl_sem in obd_device. The exception is at offset 0x588,
and u.cli.cl_sem is at 0x588 in struct obd_device,
which confirms that a NULL obd was being used.

This fix adds checks for NULL obd to active_show.

Signed-off-by: Vandana Rungta <vrungta@amazon.com>
Change-Id: Ia0ff9e7a07f40bb7dd9c0d4cc0ecc38ea8d61d0b
Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/58499
Tested-by: jenkins <devops@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>
Reviewed-by: Neil Brown <neil@brown.name>
Reviewed-by: James Simmons <jsimmons@infradead.org>
Reviewed-by: Timothy Day <timday@amazon.com>

diff --git a/lustre/include/lprocfs_status.h b/lustre/include/lprocfs_status.h
index 54fc627..1821c2f 100644 (file)
--- a/lustre/include/lprocfs_status.h
+++ b/lustre/include/lprocfs_status.h
@@ -699,6 +699,36 @@ extern int lprocfs_seq_release(struct inode *i, struct file *f);
  * to get out of the statement.
  */
 
+/*
+ * The macro uses a for loop that executes a block of code maximum once.
+ * It allows for local variable declarations.
+ *
+ * Initialization: Lock Acquisition and Import Retrieval:
+ * --------------
+ *  for (down_read_nested(&(__obd)->u.cli.cl_sem, __nest),
+ *             __imp = (__obd)->u.cli.cl_import,
+ *             __rc = __imp ? 0 : -ENODEV;
+ * It acquires a read lock,
+ * retrieves the import pointer and stores it in __imp,
+ * sets the return code __rc
+ *     to 0 (success) if __imp is not NULL, or
+ *     to _ENODEV (failure) if __imp is NULL
+ *
+ * Condition: Conditional Lock Release:
+ * ---------
+ *  __imp ? 1 : (up_read(&(__obd)->u.cli.cl_sem), 0);
+ *
+ * If __imp is not NULL, it evaluates to 1, and nothing happens.
+ * This means the lock is kept as long as a valid import was obtained.
+ * If __imp is NULL, then it releases the read lock and evaluates to 0.
+ *
+ * Update: Nulling the Import Pointer
+ * ------
+ *  __imp = NULL)
+ *
+ * sets __imp to NULL. This will break out of the for loop, releasing the
+ * semaphore in the condition.
+ */
 #define with_imp_locked_nested(__obd, __imp, __rc, __nest)             \
        for (down_read_nested(&(__obd)->u.cli.cl_sem, __nest),          \
             __imp = (__obd)->u.cli.cl_import,                          \

diff --git a/lustre/osp/lproc_osp.c b/lustre/osp/lproc_osp.c
index 6b03245..4e7630a 100644 (file)
--- a/lustre/osp/lproc_osp.c
+++ b/lustre/osp/lproc_osp.c
@@ -37,6 +37,9 @@ static ssize_t active_show(struct kobject *kobj, struct attribute *attr,
        struct obd_import *imp;
        int rc;
 
+       if (!obd)
+               return -ENOENT;
+
        with_imp_locked(obd, imp, rc)
                rc = sprintf(buf, "%d\n", !imp->imp_deactive);
        return rc;