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 Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 20A7EC4332F for ; Mon, 3 Oct 2022 11:29:28 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229633AbiJCL31 (ORCPT ); Mon, 3 Oct 2022 07:29:27 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:34218 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229480AbiJCL3Z (ORCPT ); Mon, 3 Oct 2022 07:29:25 -0400 Received: from smtp1.axis.com (smtp1.axis.com [195.60.68.17]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 232162CC8D; Mon, 3 Oct 2022 04:29:19 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=axis.com; q=dns/txt; s=axis-central1; t=1664796560; x=1696332560; h=date:from:to:cc:subject:message-id:references: mime-version:in-reply-to; bh=ZwcYVLOdHsCgTakuvlsIcAUfd5Jd8zD73dYPiE5yHpo=; b=gqBxCs1H2ZvxteM1ijq5qjn3T8QZVK7b44Rgrv5iSr/ILVNjia0TdQAh UnH6+jR3XfwSJsFaOgi1Up3P/nOifNVgFgGjBAnUqFqJCNUsYI1ZG8v0T sNehcITFZd3CT+0jHjfzJXo+ddLdX75iItRxuENxuqSWxwSmcaQujRuZB P2KEBI57XABuRySi9vF7l04lGfkAQbPJP5DeFvrNo4d9H1wPz8WZwVyNk IMfjWv7KvdM/rW0b0Nw882Ujikn4thHRYzONJQyZaJbBKGK9FLVBCPRal 163HTa0eSq3EI3IWsrxKh/KEX7oKj54oCysZQRB2X97jIc/K3umgoBdSU Q==; Date: Mon, 3 Oct 2022 13:29:18 +0200 From: Vincent Whitchurch To: Robin Murphy CC: Marek Szyprowski , "broonie@kernel.org" , "krzysztof.kozlowski@linaro.org" , "andi@etezian.org" , Christoph Hellwig , kernel , "alim.akhtar@samsung.com" , "linux-spi@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "linux-samsung-soc@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , "iommu@lists.linux-foundation.org" Subject: Re: [PATCH v2 2/4] spi: Fix cache corruption due to DMA/PIO overlap Message-ID: References: <20220927112117.77599-1-vincent.whitchurch@axis.com> <20220927112117.77599-3-vincent.whitchurch@axis.com> <461a5187-fc7a-b7f6-84da-0e947f764a0a@arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: <461a5187-fc7a-b7f6-84da-0e947f764a0a@arm.com> Precedence: bulk List-ID: X-Mailing-List: linux-samsung-soc@vger.kernel.org On Fri, Sep 30, 2022 at 02:10:28PM +0200, Robin Murphy wrote: > That said, maybe this is something that's better to catch than to paper > over? Arguably the real bug here is that spi_unmap_buf() and the new > sync functions should use the same "{tx,rx}_buf != NULL" condition that > spi_map_buf() used for the DMA mapping decision in the first place. The "{tx,rx}_buf != NULL" condition would not sufficient on its own; the call to ->can_dma() is also part of the condition. __spi_unmap_msg() already does the ->can_dma() call even though it checks for the orig_nents != 0 condition instead of the tx,rx_buf != NULL, but I omitted that call in the new sync functions, incorrectly believing it to be redundant. It looks like __spi_unmap_msg() would have triggered a similar crash even before this patch, if a client had reused an xfer with both rx and tx the first time, and only one of them enabled the next time around (and with ->can_dma() returning true both times). Testing the {tx,rx}_buf instead of sgt->orig_nents would have avoided that, as you say.