From nobody Mon Apr 29 17:36:31 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 1630078125680781.9902022111872; Fri, 27 Aug 2021 08:28:45 -0700 (PDT) Received: from localhost ([::1]:56182 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mJdme-00038j-7M for importer@patchew.org; Fri, 27 Aug 2021 11:28:44 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:60052) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mJdlk-0002DR-2G; Fri, 27 Aug 2021 11:27:48 -0400 Received: from mail.salt-inc.org ([104.244.79.104]:59481 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 1mJdlh-0000JG-V6; Fri, 27 Aug 2021 11:27:47 -0400 Received: from pc1 (92.229-132-109.adsl-dyn.isp.belgacom.be [109.132.229.92]) by vm0.salt-inc.org (Postfix) with ESMTPSA id CF928FEE09; Fri, 27 Aug 2021 05:26:39 +0200 (CEST) Received: from david by pc1 with local (Exim 4.92) (envelope-from ) id 1mJdke-00024Z-C4; Fri, 27 Aug 2021 17:26:40 +0200 From: David Hoppenbrouwers To: qemu-devel@nongnu.org Subject: [PATCH v4] hw/intc/sifive_clint: Fix muldiv64 overflow in sifive_clint_write_timecmp() Date: Fri, 27 Aug 2021 17:23:25 +0200 Message-Id: <20210827152324.5201-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: 1630078127551100001 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 Reviewed-by: Alistair Francis --- I addressed the potential signed integer overflow if `ns_diff` is `INT64_MAX`. An unsigned integer overflow still should not be able to happen practically. David hw/intc/sifive_clint.c | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/hw/intc/sifive_clint.c b/hw/intc/sifive_clint.c index 0f41e5ea1c..99c870ced2 100644 --- a/hw/intc/sifive_clint.c +++ b/hw/intc/sifive_clint.c @@ -59,8 +59,29 @@ 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 for an + * unsigned integer overflow. + */ + next =3D qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + ns_diff; + /* + * if ns_diff is INT64_MAX next may still be outside the range + * of a signed integer. + */ + next =3D MIN(next, INT64_MAX); + } + timer_mod(cpu->env.timer, next); } =20 --=20 2.20.1