Re: [PATCH] block: Fix validation of ioprio level
From: Damien Le Moal <dlemoal@kernel.org>
Date: 2024-08-28 08:45:38
Also in:
lkml
On 8/28/24 17:28, Ajay Kaher wrote:
quoted hunk ↗ jump to hunk
The commit eca2040972b4 introduced a backward compatibility issue in the function ioprio_check_cap. Before the change, if ioprio contains a level greater than 0x7, it was treated as -EINVAL: data = ioprio & 0x1FFF if data >= 0x7, return -EINVAL Since the change, if ioprio contains a level greater than 0x7 say 0x8 it is calculated as 0x0: level = ioprio & 0x7 To maintain backward compatibility the kernel should return -EINVAL in the above case as well. Fixes: eca2040972b4 ("scsi: block: ioprio: Clean up interface definition") Signed-off-by: Ajay Kaher <ajay.kaher@broadcom.com> --- block/ioprio.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)diff --git a/block/ioprio.c b/block/ioprio.c index 73301a2..f08e76b 100644 --- a/block/ioprio.c +++ b/block/ioprio.c@@ -30,6 +30,15 @@ #include <linux/security.h> #include <linux/pid_namespace.h> +static inline int ioprio_check_level(int ioprio, int max_level) +{ + int data = IOPRIO_PRIO_DATA(ioprio); + + if (IOPRIO_BAD_VALUE(data, max_level)) + return -EINVAL;
No, this cannot possibly work correctly because the prio level part of the prio data is only 3 bits, so 0 to 7. The remaining 10 bits of the prio data are used for priority hints (IOPRIO_HINT_XXX). Your change will thus return an error for cases where the prio data has a level AND also a hint (e.g. for command duration limits). This change would break command duration limits. So NACK. The userspace header file has the ioprio_value() that a user should use to construct an ioprio. Bad values are checked in that function and errors will be returned if an invalid level is passed.
quoted hunk ↗ jump to hunk
+ return 0; +} + int ioprio_check_cap(int ioprio) { int class = IOPRIO_PRIO_CLASS(ioprio);@@ -49,7 +58,7 @@ int ioprio_check_cap(int ioprio) fallthrough; /* rt has prio field too */ case IOPRIO_CLASS_BE: - if (level >= IOPRIO_NR_LEVELS) + if (ioprio_check_level(ioprio, IOPRIO_NR_LEVELS)) return -EINVAL; break; case IOPRIO_CLASS_IDLE:
-- Damien Le Moal Western Digital Research