Linux-Block Archive mirror
 help / color / mirror / Atom feed
* [PATCH block-6.7] blk-iocost: Fix an UBSAN shift-out-of-bounds warning
@ 2023-11-20 22:25 Tejun Heo
  2024-02-08 17:12 ` Jens Axboe
  0 siblings, 1 reply; 2+ messages in thread
From: Tejun Heo @ 2023-11-20 22:25 UTC (permalink / raw
  To: Jens Axboe; +Cc: Breno Leitão, linux-block, linux-kernel

When iocg_kick_delay() is called from a CPU different than the one which set
the delay, @now may be in the past of @iocg->delay_at leading to the
following warning:

  UBSAN: shift-out-of-bounds in block/blk-iocost.c:1359:23
  shift exponent 18446744073709 is too large for 64-bit type 'u64' (aka 'unsigned long long')
  ...
  Call Trace:
   <TASK>
   dump_stack_lvl+0x79/0xc0
   __ubsan_handle_shift_out_of_bounds+0x2ab/0x300
   iocg_kick_delay+0x222/0x230
   ioc_rqos_merge+0x1d7/0x2c0
   __rq_qos_merge+0x2c/0x80
   bio_attempt_back_merge+0x83/0x190
   blk_attempt_plug_merge+0x101/0x150
   blk_mq_submit_bio+0x2b1/0x720
   submit_bio_noacct_nocheck+0x320/0x3e0
   __swap_writepage+0x2ab/0x9d0

The underflow itself doesn't really affect the behavior in any meaningful
way; however, the past timestamp may exaggerate the delay amount calculated
later in the code, which shouldn't be a material problem given the nature of
the delay mechanism.

If @now is in the past, this CPU is racing another CPU which recently set up
the delay and there's nothing this CPU can contribute w.r.t. the delay.
Let's bail early from iocg_kick_delay() in such cases.

Reported-by: Breno Leitão <leitao@debian.org>
Signed-off-by: Tejun Heo <tj@kernel.org>
Fixes: 5160a5a53c0c ("blk-iocost: implement delay adjustment hysteresis")
---
 block/blk-iocost.c |    7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/block/blk-iocost.c b/block/blk-iocost.c
index 089fcb9cfce3..7ee8d85c2c68 100644
--- a/block/blk-iocost.c
+++ b/block/blk-iocost.c
@@ -1353,6 +1353,13 @@ static bool iocg_kick_delay(struct ioc_gq *iocg, struct ioc_now *now)
 
 	lockdep_assert_held(&iocg->waitq.lock);
 
+	/*
+	 * If the delay is set by another CPU, we may be in the past. No need to
+	 * change anything if so. This avoids decay calculation underflow.
+	 */
+	if (time_before64(now->now, iocg->delay_at))
+		return false;
+
 	/* calculate the current delay in effect - 1/2 every second */
 	tdelta = now->now - iocg->delay_at;
 	if (iocg->delay)

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH block-6.7] blk-iocost: Fix an UBSAN shift-out-of-bounds warning
  2023-11-20 22:25 [PATCH block-6.7] blk-iocost: Fix an UBSAN shift-out-of-bounds warning Tejun Heo
@ 2024-02-08 17:12 ` Jens Axboe
  0 siblings, 0 replies; 2+ messages in thread
From: Jens Axboe @ 2024-02-08 17:12 UTC (permalink / raw
  To: Tejun Heo; +Cc: Breno Leitão, linux-block, linux-kernel


On Mon, 20 Nov 2023 12:25:56 -1000, Tejun Heo wrote:
> When iocg_kick_delay() is called from a CPU different than the one which set
> the delay, @now may be in the past of @iocg->delay_at leading to the
> following warning:
> 
>   UBSAN: shift-out-of-bounds in block/blk-iocost.c:1359:23
>   shift exponent 18446744073709 is too large for 64-bit type 'u64' (aka 'unsigned long long')
>   ...
>   Call Trace:
>    <TASK>
>    dump_stack_lvl+0x79/0xc0
>    __ubsan_handle_shift_out_of_bounds+0x2ab/0x300
>    iocg_kick_delay+0x222/0x230
>    ioc_rqos_merge+0x1d7/0x2c0
>    __rq_qos_merge+0x2c/0x80
>    bio_attempt_back_merge+0x83/0x190
>    blk_attempt_plug_merge+0x101/0x150
>    blk_mq_submit_bio+0x2b1/0x720
>    submit_bio_noacct_nocheck+0x320/0x3e0
>    __swap_writepage+0x2ab/0x9d0
> 
> [...]

Applied, thanks!

[1/1] blk-iocost: Fix an UBSAN shift-out-of-bounds warning
      (no commit info)

Best regards,
-- 
Jens Axboe




^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2024-02-08 17:12 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-11-20 22:25 [PATCH block-6.7] blk-iocost: Fix an UBSAN shift-out-of-bounds warning Tejun Heo
2024-02-08 17:12 ` Jens Axboe

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).