From nobody Fri May 17 09:38:17 2024 Delivered-To: importer@patchew.org Authentication-Results: mx.zohomail.com; spf=pass (zohomail.com: domain of gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org Return-Path: Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) by mx.zohomail.com with SMTPS id 1629350381996724.0322835828528; Wed, 18 Aug 2021 22:19:41 -0700 (PDT) Received: from localhost ([::1]:37784 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mGaSr-0003H7-2O for importer@patchew.org; Thu, 19 Aug 2021 01:19:41 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:53000) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mGaRv-00029u-U6; Thu, 19 Aug 2021 01:18:43 -0400 Received: from mail.salt-inc.org ([104.244.79.104]:11330 helo=vm0.salt-inc.org) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mGaRt-0002eZ-Mg; Thu, 19 Aug 2021 01:18:43 -0400 Received: from pc1 (unknown [109.132.229.92]) by vm0.salt-inc.org (Postfix) with ESMTPSA id 761ACFEC3B; Wed, 19 Aug 2020 07:17:59 +0200 (CEST) Received: from david by pc1 with local (Exim 4.92) (envelope-from ) id 1mGaQp-0004XW-Bg; Thu, 19 Aug 2021 07:17:35 +0200 From: David Hoppenbrouwers To: qemu-devel@nongnu.org Subject: [PATCH v3] hw/intc/sifive_clint: Fix overflow in sifive_clint_write_timecmp() Date: Thu, 19 Aug 2021 07:12:11 +0200 Message-Id: <20210819051209.17237-1-david@salt-inc.org> X-Mailer: git-send-email 2.20.1 MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Received-SPF: pass (zohomail.com: domain of gnu.org designates 209.51.188.17 as permitted sender) client-ip=209.51.188.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Received-SPF: pass client-ip=104.244.79.104; envelope-from=david@salt-inc.org; helo=vm0.salt-inc.org X-Spam_score_int: -18 X-Spam_score: -1.9 X-Spam_bar: - X-Spam_report: (-1.9 / 5.0 requ) BAYES_00=-1.9, SPF_HELO_PASS=-0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Bin Meng , Palmer Dabbelt , David Hoppenbrouwers , Alistair Francis , "open list:SiFive Machines" Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZM-MESSAGEID: 1629350385261100001 Content-Type: text/plain; charset="utf-8" `muldiv64` would overflow in cases where the final 96-bit value does not fit in a `uint64_t`. This would result in small values that cause an interrupt to be triggered much sooner than intended. The overflow can be detected in most cases by checking if the new value is smaller than the previous value. If the final result is larger than `diff` it is either correct or it doesn't matter as it is effectively infinite anyways. `next` is an `uint64_t` value, but `timer_mod` takes an `int64_t`. This resulted in high values such as `UINT64_MAX` being converted to `-1`, which caused an immediate timer interrupt. By limiting `next` to `INT64_MAX` no overflow will happen while the timer will still be effectively set to "infinitely" far in the future. Resolves: https://gitlab.com/qemu-project/qemu/-/issues/493 Signed-off-by: David Hoppenbrouwers --- I did not account for the multiplication overflow mentioned in the bug report. I've amended the patch and I do not spot any erroneous interrupts anymore. I see that the previous patch already got applied to riscv-to-apply.next. Do I need to create a new patch? David hw/intc/sifive_clint.c | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/hw/intc/sifive_clint.c b/hw/intc/sifive_clint.c index 0f41e5ea1c..aa76e639a9 100644 --- a/hw/intc/sifive_clint.c +++ b/hw/intc/sifive_clint.c @@ -59,8 +59,23 @@ static void sifive_clint_write_timecmp(RISCVCPU *cpu, ui= nt64_t value, riscv_cpu_update_mip(cpu, MIP_MTIP, BOOL_TO_MASK(0)); diff =3D cpu->env.timecmp - rtc_r; /* back to ns (note args switched in muldiv64) */ - next =3D qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + - muldiv64(diff, NANOSECONDS_PER_SECOND, timebase_freq); + uint64_t ns_diff =3D muldiv64(diff, NANOSECONDS_PER_SECOND, timebase_f= req); + + /* + * check if ns_diff overflowed and check if the addition would potenti= ally + * overflow + */ + if ((NANOSECONDS_PER_SECOND > timebase_freq && ns_diff < diff) || + ns_diff > INT64_MAX) { + next =3D INT64_MAX; + } else { + /* + * as it is very unlikely qemu_clock_get_ns will return a value + * greater than INT64_MAX, no additional check is needed. + */ + next =3D qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + ns_diff; + } + timer_mod(cpu->env.timer, next); } =20 --=20 2.20.1