scsi: block: Improve ioprio value validity checks
authorDamien Le Moal <dlemoal@kernel.org>
Thu, 8 Jun 2023 09:55:56 +0000 (18:55 +0900)
committerMartin K. Petersen <martin.petersen@oracle.com>
Fri, 16 Jun 2023 16:04:30 +0000 (12:04 -0400)
The introduction of the macro IOPRIO_PRIO_LEVEL() in commit eca2040972b4
("scsi: block: ioprio: Clean up interface definition") results in an
iopriority level to always be masked using the macro IOPRIO_LEVEL_MASK, and
thus to the kernel always seeing an acceptable value for an I/O priority
level when checked in ioprio_check_cap().  Before this patch, this function
would return an error for some (but not all) invalid values for a level
valid range of [0..7].

Restore and improve the detection of invalid priority levels by introducing
the inline function ioprio_value() to check an ioprio class, level and hint
value before combining these fields into a single value to be used with
ioprio_set() or AIOs. If an invalid value for the class, level or hint of
an ioprio is detected, ioprio_value() returns an ioprio using the class
IOPRIO_CLASS_INVALID, indicating an invalid value and causing
ioprio_check_cap() to return -EINVAL.

Fixes: 6c913257226a ("scsi: block: Introduce ioprio hints")
Fixes: eca2040972b4 ("scsi: block: ioprio: Clean up interface definition")
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
Link: https://lore.kernel.org/r/20230608095556.124001-1-dlemoal@kernel.org
Reviewed-by: Niklas Cassel <niklas.cassel@wdc.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
block/ioprio.c
include/uapi/linux/ioprio.h

index f0d9e818abc5358b1ff6b2c0655fba2edcac683d..b5a942519a797ceab4729231dd7c673e50ab0613 100644 (file)
@@ -58,6 +58,7 @@ int ioprio_check_cap(int ioprio)
                        if (level)
                                return -EINVAL;
                        break;
+               case IOPRIO_CLASS_INVALID:
                default:
                        return -EINVAL;
        }
index 4c4806e8230bc6c73cce09ac3be975ca648e2ac0..99440b2e8c352da51cddfa61361db22a8a9bfcd8 100644 (file)
@@ -2,19 +2,20 @@
 #ifndef _UAPI_LINUX_IOPRIO_H
 #define _UAPI_LINUX_IOPRIO_H
 
+#include <linux/stddef.h>
+#include <linux/types.h>
+
 /*
  * Gives us 8 prio classes with 13-bits of data for each class
  */
 #define IOPRIO_CLASS_SHIFT     13
-#define IOPRIO_CLASS_MASK      0x07
+#define IOPRIO_NR_CLASSES      8
+#define IOPRIO_CLASS_MASK      (IOPRIO_NR_CLASSES - 1)
 #define IOPRIO_PRIO_MASK       ((1UL << IOPRIO_CLASS_SHIFT) - 1)
 
 #define IOPRIO_PRIO_CLASS(ioprio)      \
        (((ioprio) >> IOPRIO_CLASS_SHIFT) & IOPRIO_CLASS_MASK)
 #define IOPRIO_PRIO_DATA(ioprio)       ((ioprio) & IOPRIO_PRIO_MASK)
-#define IOPRIO_PRIO_VALUE(class, data) \
-       ((((class) & IOPRIO_CLASS_MASK) << IOPRIO_CLASS_SHIFT) | \
-        ((data) & IOPRIO_PRIO_MASK))
 
 /*
  * These are the io priority classes as implemented by the BFQ and mq-deadline
  * served when no one else is using the disk.
  */
 enum {
-       IOPRIO_CLASS_NONE,
-       IOPRIO_CLASS_RT,
-       IOPRIO_CLASS_BE,
-       IOPRIO_CLASS_IDLE,
+       IOPRIO_CLASS_NONE       = 0,
+       IOPRIO_CLASS_RT         = 1,
+       IOPRIO_CLASS_BE         = 2,
+       IOPRIO_CLASS_IDLE       = 3,
+
+       /* Special class to indicate an invalid ioprio value */
+       IOPRIO_CLASS_INVALID    = 7,
 };
 
 /*
@@ -73,15 +77,6 @@ enum {
 #define IOPRIO_PRIO_HINT(ioprio)       \
        (((ioprio) >> IOPRIO_HINT_SHIFT) & IOPRIO_HINT_MASK)
 
-/*
- * Alternate macro for IOPRIO_PRIO_VALUE() to define an I/O priority with
- * a class, level and hint.
- */
-#define IOPRIO_PRIO_VALUE_HINT(class, level, hint)              \
-       ((((class) & IOPRIO_CLASS_MASK) << IOPRIO_CLASS_SHIFT) | \
-        (((hint) & IOPRIO_HINT_MASK) << IOPRIO_HINT_SHIFT) |    \
-        ((level) & IOPRIO_LEVEL_MASK))
-
 /*
  * I/O hints.
  */
@@ -107,4 +102,25 @@ enum {
        IOPRIO_HINT_DEV_DURATION_LIMIT_7 = 7,
 };
 
+#define IOPRIO_BAD_VALUE(val, max) ((val) < 0 || (val) >= (max))
+
+/*
+ * Return an I/O priority value based on a class, a level and a hint.
+ */
+static __always_inline __u16 ioprio_value(int class, int level, int hint)
+{
+       if (IOPRIO_BAD_VALUE(class, IOPRIO_NR_CLASSES) ||
+           IOPRIO_BAD_VALUE(level, IOPRIO_NR_LEVELS) ||
+           IOPRIO_BAD_VALUE(hint, IOPRIO_NR_HINTS))
+               return IOPRIO_CLASS_INVALID << IOPRIO_CLASS_SHIFT;
+
+       return (class << IOPRIO_CLASS_SHIFT) |
+               (hint << IOPRIO_HINT_SHIFT) | level;
+}
+
+#define IOPRIO_PRIO_VALUE(class, level)                        \
+       ioprio_value(class, level, IOPRIO_HINT_NONE)
+#define IOPRIO_PRIO_VALUE_HINT(class, level, hint)     \
+       ioprio_value(class, level, hint)
+
 #endif /* _UAPI_LINUX_IOPRIO_H */