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=-15.3 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, USER_AGENT_SANE_1 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 24686C433ED for ; Thu, 1 Apr 2021 08:56:50 +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 A98A66023B for ; Thu, 1 Apr 2021 08:56:49 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org A98A66023B Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=ilande.co.uk Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Received: from localhost ([::1]:59836 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lRt8C-0007vy-NA for qemu-devel@archiver.kernel.org; Thu, 01 Apr 2021 04:56:48 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:45548) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lRt7c-0007N7-Q9 for qemu-devel@nongnu.org; Thu, 01 Apr 2021 04:56:12 -0400 Received: from mail.ilande.co.uk ([2001:41c9:1:41f::167]:57304 helo=mail.default.ilande.uk0.bigv.io) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lRt7b-0007nw-2w for qemu-devel@nongnu.org; Thu, 01 Apr 2021 04:56:12 -0400 Received: from host86-148-103-9.range86-148.btcentralplus.com ([86.148.103.9] helo=[192.168.1.65]) by mail.default.ilande.uk0.bigv.io with esmtpsa (TLS1.3:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.92) (envelope-from ) id 1lRt7e-0004lD-LV; Thu, 01 Apr 2021 09:56:19 +0100 To: =?UTF-8?Q?Philippe_Mathieu-Daud=c3=a9?= , qemu-devel@nongnu.org, alxndr@bu.edu, laurent@vivier.eu, pbonzini@redhat.com References: <20210401074933.9923-1-mark.cave-ayland@ilande.co.uk> <20210401074933.9923-9-mark.cave-ayland@ilande.co.uk> <1aa39496-e535-5e38-552b-1e314fcb9905@amsat.org> From: Mark Cave-Ayland Message-ID: <38050d91-e718-f9c9-c4da-8962f5c9d0ef@ilande.co.uk> Date: Thu, 1 Apr 2021 09:56:02 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.9.0 MIME-Version: 1.0 In-Reply-To: <1aa39496-e535-5e38-552b-1e314fcb9905@amsat.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit X-SA-Exim-Connect-IP: 86.148.103.9 X-SA-Exim-Mail-From: mark.cave-ayland@ilande.co.uk Subject: Re: [PATCH v3 08/11] esp: don't overflow cmdfifo in get_cmd() X-SA-Exim-Version: 4.2.1 (built Wed, 08 May 2019 21:11:16 +0000) X-SA-Exim-Scanned: Yes (on mail.default.ilande.uk0.bigv.io) Received-SPF: pass client-ip=2001:41c9:1:41f::167; envelope-from=mark.cave-ayland@ilande.co.uk; helo=mail.default.ilande.uk0.bigv.io 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, NICE_REPLY_A=-0.001, SPF_HELO_NONE=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: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" On 01/04/2021 09:19, Philippe Mathieu-Daudé wrote: > On 4/1/21 9:49 AM, Mark Cave-Ayland wrote: >> If the guest tries to read a CDB using DMA and cmdfifo is not empty then it is >> possible to overflow cmdfifo. >> >> Since this can only occur by issuing deliberately incorrect instruction >> sequences, ensure that the maximum length of the CDB transferred to cmdfifo is >> limited to the available free space within cmdfifo. >> >> Buglink: https://bugs.launchpad.net/qemu/+bug/1909247 >> Signed-off-by: Mark Cave-Ayland >> --- >> hw/scsi/esp.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c >> index 7f49522e1d..c547c60395 100644 >> --- a/hw/scsi/esp.c >> +++ b/hw/scsi/esp.c >> @@ -243,6 +243,7 @@ static uint32_t get_cmd(ESPState *s, uint32_t maxlen) >> } >> if (s->dma_memory_read) { >> s->dma_memory_read(s->dma_opaque, buf, dmalen); >> + dmalen = MIN(fifo8_num_free(&s->fifo), dmalen); > > Ditto, GUEST_ERRORS? Possibly? But then there are several other places where this could also happen. The ESP uses the FIFO as a buffer for the SCSI bus in DMA mode, and so at the start of a DMA transfer the FIFO can contain leftover junk. This is why all the guest OSs I've seen send an explicit "Flush FIFO" command before each command to ensure that any junk is removed from the FIFO before sending the message out/CDB. > Reviewed-by: Philippe Mathieu-Daudé > >> fifo8_push_all(&s->cmdfifo, buf, dmalen); >> } else { >> if (esp_select(s) < 0) { >> ATB, Mark.