All of lore.kernel.org
 help / color / mirror / Atom feed
From: Li Jinlin <lijinlin3@huawei.com>
To: <tj@kernel.org>, <josef@toxicpanda.com>, <axboe@kernel.dk>
Cc: <cgroups@vger.kernel.org>, <linux-block@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>, <liuzhiqiang26@huawei.com>
Subject: [PATCH] blk-iocost: fix shift-out-of-bounds in iocg_hick_delay()
Date: Sat, 26 Nov 2022 20:14:58 +0800	[thread overview]
Message-ID: <20221126121458.3564942-1-lijinlin3@huawei.com> (raw)

We got the following UBSAN report:
====================================================================
UBSAN: shift-out-of-bounds in block/blk-iocost.c:1294:23
shift exponent 18446744073709 is too large for 64-bit type ......
CPU: 1 PID: 1088217 Comm: fsstress Kdump: loaded Not tainted ......
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996) ......
Call Trace:
 dump_stack+0x9c/0xd3
 ubsan_epilogue+0xa/0x4e
 __ubsan_handle_shift_out_of_bounds.cold+0x87/0x137
 iocg_kick_delay.cold+0x18/0x60
 ioc_rqos_throttle+0x7f8/0x870
 __rq_qos_throttle+0x40/0x60
 blk_mq_submit_bio+0x24d/0xd60
 __submit_bio_noacct_mq+0x10b/0x270
 submit_bio_noacct+0x13d/0x150
 submit_bio+0xbf/0x280
 submit_bh_wbc+0x3aa/0x450
 ext4_read_bh_nowait+0xdb/0x180 [ext4]
 ext4_read_bh_lock+0x6d/0x90 [ext4]
 ext4_bread_batch+0x24c/0x2e0 [ext4]
 __ext4_find_entry+0x2d2/0x880 [ext4]
 ext4_lookup.part.0+0xbf/0x370 [ext4]
 ext4_lookup+0x3e/0x60 [ext4]
 lookup_open.isra.0+0x343/0x630
 open_last_lookups+0x1f2/0x750
 path_openat+0x133/0x330
 do_filp_open+0x122/0x270
 do_sys_openat2+0x3a8/0x550
 __x64_sys_creat+0xae/0xe0
 do_syscall_64+0x33/0x40
 entry_SYSCALL_64_after_hwframe+0x61/0xc6
===================================================================

The result of E1 >> E2 is E1 right-shifted E2 bit positions. From the
report, we know E2 is greater than the width of E1. In the C99 standard,
if the value of the E2 is negative or is greater than or equal to the
width of E1, the behavior is undefined.

In the actual test, if the E2 is greater than or equal to the width of
E1, the result of E1 >> E2 is E1 >> (E2 %/ E1width), which is not what we
want.

So letting the value of the right operand be less than the width of u64
in this expression.

Signed-off-by: Li Jinlin <lijinlin3@huawei.com>
---
 block/blk-iocost.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/blk-iocost.c b/block/blk-iocost.c
index 07c1a31dd495..2b837ac4b2ba 100644
--- a/block/blk-iocost.c
+++ b/block/blk-iocost.c
@@ -1332,7 +1332,7 @@ static bool iocg_kick_delay(struct ioc_gq *iocg, struct ioc_now *now)
 	/* calculate the current delay in effect - 1/2 every second */
 	tdelta = now->now - iocg->delay_at;
 	if (iocg->delay)
-		delay = iocg->delay >> div64_u64(tdelta, USEC_PER_SEC);
+		delay = iocg->delay >> min(div64_u64(tdelta, USEC_PER_SEC), 63);
 	else
 		delay = 0;
 
-- 
2.30.2


WARNING: multiple messages have this Message-ID (diff)
From: Li Jinlin <lijinlin3-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
To: tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
	josef-DigfWCa+lFGyeJad7bwFQA@public.gmane.org,
	axboe-tSWWG44O7X1aa/9Udqfwiw@public.gmane.org
Cc: cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-block-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	liuzhiqiang26-hv44wF8Li93QT0dZR+AlfA@public.gmane.org
Subject: [PATCH] blk-iocost: fix shift-out-of-bounds in iocg_hick_delay()
Date: Sat, 26 Nov 2022 20:14:58 +0800	[thread overview]
Message-ID: <20221126121458.3564942-1-lijinlin3@huawei.com> (raw)

We got the following UBSAN report:
====================================================================
UBSAN: shift-out-of-bounds in block/blk-iocost.c:1294:23
shift exponent 18446744073709 is too large for 64-bit type ......
CPU: 1 PID: 1088217 Comm: fsstress Kdump: loaded Not tainted ......
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996) ......
Call Trace:
 dump_stack+0x9c/0xd3
 ubsan_epilogue+0xa/0x4e
 __ubsan_handle_shift_out_of_bounds.cold+0x87/0x137
 iocg_kick_delay.cold+0x18/0x60
 ioc_rqos_throttle+0x7f8/0x870
 __rq_qos_throttle+0x40/0x60
 blk_mq_submit_bio+0x24d/0xd60
 __submit_bio_noacct_mq+0x10b/0x270
 submit_bio_noacct+0x13d/0x150
 submit_bio+0xbf/0x280
 submit_bh_wbc+0x3aa/0x450
 ext4_read_bh_nowait+0xdb/0x180 [ext4]
 ext4_read_bh_lock+0x6d/0x90 [ext4]
 ext4_bread_batch+0x24c/0x2e0 [ext4]
 __ext4_find_entry+0x2d2/0x880 [ext4]
 ext4_lookup.part.0+0xbf/0x370 [ext4]
 ext4_lookup+0x3e/0x60 [ext4]
 lookup_open.isra.0+0x343/0x630
 open_last_lookups+0x1f2/0x750
 path_openat+0x133/0x330
 do_filp_open+0x122/0x270
 do_sys_openat2+0x3a8/0x550
 __x64_sys_creat+0xae/0xe0
 do_syscall_64+0x33/0x40
 entry_SYSCALL_64_after_hwframe+0x61/0xc6
===================================================================

The result of E1 >> E2 is E1 right-shifted E2 bit positions. From the
report, we know E2 is greater than the width of E1. In the C99 standard,
if the value of the E2 is negative or is greater than or equal to the
width of E1, the behavior is undefined.

In the actual test, if the E2 is greater than or equal to the width of
E1, the result of E1 >> E2 is E1 >> (E2 %/ E1width), which is not what we
want.

So letting the value of the right operand be less than the width of u64
in this expression.

Signed-off-by: Li Jinlin <lijinlin3-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
---
 block/blk-iocost.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/blk-iocost.c b/block/blk-iocost.c
index 07c1a31dd495..2b837ac4b2ba 100644
--- a/block/blk-iocost.c
+++ b/block/blk-iocost.c
@@ -1332,7 +1332,7 @@ static bool iocg_kick_delay(struct ioc_gq *iocg, struct ioc_now *now)
 	/* calculate the current delay in effect - 1/2 every second */
 	tdelta = now->now - iocg->delay_at;
 	if (iocg->delay)
-		delay = iocg->delay >> div64_u64(tdelta, USEC_PER_SEC);
+		delay = iocg->delay >> min(div64_u64(tdelta, USEC_PER_SEC), 63);
 	else
 		delay = 0;
 
-- 
2.30.2


             reply	other threads:[~2022-11-26 12:16 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-26 12:14 Li Jinlin [this message]
2022-11-26 12:14 ` [PATCH] blk-iocost: fix shift-out-of-bounds in iocg_hick_delay() Li Jinlin
2022-11-27  6:38 ` kernel test robot
2022-11-28 17:09 ` Tejun Heo
2022-11-28 17:09   ` Tejun Heo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20221126121458.3564942-1-lijinlin3@huawei.com \
    --to=lijinlin3@huawei.com \
    --cc=axboe@kernel.dk \
    --cc=cgroups@vger.kernel.org \
    --cc=josef@toxicpanda.com \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=liuzhiqiang26@huawei.com \
    --cc=tj@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.