From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.0 required=3.0 tests=BAYES_00,HK_RANDOM_FROM, INCLUDES_CR_TRAILER,MAILING_LIST_MULTI,SPF_HELO_NONE,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id D1D9BC4338F for ; Fri, 6 Aug 2021 14:44:52 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 4B171611CC for ; Fri, 6 Aug 2021 14:44:52 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 4B171611CC Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=bu.edu Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=nongnu.org Received: from localhost ([::1]:37212 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mC15f-0000Kf-4d for qemu-devel@archiver.kernel.org; Fri, 06 Aug 2021 10:44:51 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:37228) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mC14e-0007Ah-4C; Fri, 06 Aug 2021 10:43:48 -0400 Received: from relay68.bu.edu ([128.197.228.73]:41381) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mC14c-0006Mk-D9; Fri, 06 Aug 2021 10:43:47 -0400 X-Envelope-From: alxndr@bu.edu X-BU-AUTH: mozz.bu.edu [128.197.127.33] Received: from BU-AUTH (localhost.localdomain [127.0.0.1]) (authenticated bits=0) by relay68.bu.edu (8.14.3/8.14.3) with ESMTP id 176Eg9pc027907 (version=TLSv1/SSLv3 cipher=AES256-GCM-SHA384 bits=256 verify=NO); Fri, 6 Aug 2021 10:42:12 -0400 Date: Fri, 6 Aug 2021 10:42:09 -0400 From: Alexander Bulekov To: Qiang Liu Subject: Re: [PATCH v2] hw/display/xlnx_dp: fix an out-of-bounds read in xlnx_dp_read Message-ID: <20210806144209.nbx4vlm6ofkza5pl@mozz.bu.edu> References: <1628059910-12060-1-git-send-email-cyruscyliu@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1628059910-12060-1-git-send-email-cyruscyliu@gmail.com> Received-SPF: pass client-ip=128.197.228.73; envelope-from=alxndr@bu.edu; helo=relay68.bu.edu X-Spam_score_int: -5 X-Spam_score: -0.6 X-Spam_bar: / X-Spam_report: (-0.6 / 5.0 requ) BAYES_00=-1.9, HK_RANDOM_ENVFROM=0.998, HK_RANDOM_FROM=0.998, RCVD_IN_DNSWL_LOW=-0.7, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=no 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: Laurent Vivier , Peter Maydell , Thomas Huth , Alistair Francis , "open list:All patches CC here" , Bandan Das , "open list:Xilinx ZynqMP and..." , Stefan Hajnoczi , Paolo Bonzini , "Edgar E. Iglesias" Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" On 210804 1451, Qiang Liu wrote: > xlnx_dp_read allows an out-of-bounds read at its default branch because > of an improper index. > > According to > https://www.xilinx.com/html_docs/registers/ug1087/ug1087-zynq-ultrascale-registers.html > (DP Module), registers 0x3A4/0x3A4/0x3AC are allowed. > > DP_INT_MASK 0x000003A4 32 mixed 0xFFFFF03F Interrupt Mask Register for intrN. > DP_INT_EN 0x000003A8 32 mixed 0x00000000 Interrupt Enable Register. > DP_INT_DS 0x000003AC 32 mixed 0x00000000 Interrupt Disable Register. > > In xlnx_dp_write, when the offset is 0x3A8 and 0x3AC, the virtual device > will write s->core_registers[0x3A4 > >> 2]. That is to say, the maxize of s->core_registers could be ((0x3A4 > >> 2) + 1). However, the current size of s->core_registers is (0x3AF >> > >> 2), that is ((0x3A4 >> 2) + 2), which is out of the range. > In xlxn_dp_read, the access to offset 0x3A8 or 0x3AC will be directed to > the offset 0x3A8 (incorrect functionality) or 0x3AC (out-of-bounds read) > rather than 0x3A4. > > This patch enforces the read access to offset 0x3A8 and 0x3AC to 0x3A4, > but does not adjust the size of s->core_registers to avoid breaking > migration. > > Fixes: 58ac482a66de ("introduce xlnx-dp") > Signed-off-by: Qiang Liu Acked-by: Alexander Bulekov If there is somehow a regression, the test won't fail in a fatal way, so maybe it makes sense to throw in a setenv(UBSAN_OPTIONS=halt_on_error=1) As a side note(not strictly related to this fix) should we continue joining reproducer patches with the fixes? In order to test the reproducer, you need to cleave the fix off the patch. At the same time we don't want to mess up bisection, so does it make sense to have the reproducer patch be separate but come last in the series? Thanks