From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 2E440388374; Tue, 12 May 2026 09:40:39 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1778578840; cv=none; b=usJt5GOHXz6tKcObh0V5+JaUiGa4o1LQ0tqj2e1tIbfUiAlF6eueAyWzQ9EO9uhsSGOIEzAn7lOKYGRuPUy5PWrbXxwDeenF1ta7V+x+9iqx/CmDnKc9MdnXjRlgTmSlNc70Zlfu9oxJeayiwqrK/CztHCJvkojobkGsEuvziU4= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1778578840; c=relaxed/simple; bh=MQw7xk7AK7vUUFTxVKsaMlXHU3CigVS/H91vrn522b8=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=ZhUNmc44qX/L3V4LEf8sqa1/BgEhFAmhiv+T4NXAFxitCnea02T9Zw7qYnYESd5eBbcxz6eITJwXPbS4QWq5faODaV3ro2txcwvpln7548Q4SFRLpj80ONheqr05/xUa5/nmfhe/go7XR8CUASa6btzC/SkrXb3J5VjsLXOuJvI= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=UD4r/Q8S; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="UD4r/Q8S" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 0E2D3C2BCF5; Tue, 12 May 2026 09:40:34 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1778578839; bh=MQw7xk7AK7vUUFTxVKsaMlXHU3CigVS/H91vrn522b8=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=UD4r/Q8SkhsDvHSeevTzvHmw6vz0Z+dZ58YkwWZkda6Ul7/l5f/CCzmlRP0IomKA5 aynp0zH103Kmr4Eixzpci+QzROxprbZXnvAw0q4d0E7PC5QHBqpo0cU9PttPlRhM2o Bdw0s7lt2PrsE/IHKalO15/J1qYM4kTagd1v6cpiQ0mKs7DaCXmN0MRyaLV1YrKq8+ RJUw/11YbCCJnOd2h1H2WCehjtJquNzS9aYRZboEYm25V81Ez7OhUt59g/Z2oSEvmv vOi0ZyWhaAd7tY1WjTQJ4L1LIQsKnm2wiBFX87ptwvUgcm1pwEg7kA05doboaeXvDK fJfUQXv/NTBTA== Date: Tue, 12 May 2026 10:40:32 +0100 From: Simon Horman To: Arnd Bergmann Cc: Arnd Bergmann , Netdev , Aaro Koskinen , Andreas Kemnade , Bartosz Golaszewski , =?utf-8?Q?Beno=C3=AEt?= Cousson , "David S . Miller" , Dmitry Torokhov , Eric Dumazet , Felipe Balbi , Jakub Kicinski , Johannes Berg , Kevin Hilman , krzk+dt@kernel.org, Linus Walleij , Paolo Abeni , Rob Herring , Roger Quadros , Tony Lindgren , linux-wireless@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, "open list:GPIO SUBSYSTEM" , Linux-OMAP , Krzysztof Kozlowski Subject: Re: [PATCH 2/3] [v5 net-next] p54spi: convert to devicetree Message-ID: <20260512094032.GI27589@horms.kernel.org> References: <20260507212451.3333185-3-arnd@kernel.org> <20260511161243.49098-3-horms@kernel.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: On Mon, May 11, 2026 at 09:45:18PM +0200, Arnd Bergmann wrote: > On Mon, May 11, 2026, at 18:12, Simon Horman wrote: > > > > Simon says: As a pre-existing issue I don't think we need to be concerned > > about this in the scope of this patch-set. But it may > > be a potential for follow-up. > > > > This problem wasn't introduced by this patch, but I noticed a pre-existing > > issue in the context here. Does the p54spi_spi_read() function perform an SPI > > transfer using a stack-allocated buffer? > > > > Looking at p54spi_spi_read(), the addr variable is allocated on the stack and > > passed as a transfer buffer to spi_sync(). The SPI framework maps transfer > > buffers for DMA. Passing a stack-allocated buffer is illegal, particularly with > > CONFIG_VMAP_STACK enabled, because stack memory resides in vmalloc space and > > cannot be safely DMA-mapped. > > > > Could this cause memory corruption or a kernel panic during the transfer? > > I already commented on this earlier: This is indeed a theoretical > bug, but it works in practice because DMA-capable SPI controllers > tend to all use MMIO for short transfers instead of DMA. > > SPI is very fragile this way, and I do have some patches to improve > that overall, but it seems out of scope for this one driver. Thanks, I agree this is out of scope. > >> - > >> - gpio_direction_output(p54spi_gpio_power, 0); > >> - gpio_direction_input(p54spi_gpio_irq); > >> - > >> - ret = request_irq(gpio_to_irq(p54spi_gpio_irq), > >> - p54spi_interrupt, IRQF_NO_AUTOEN, "p54spi", > >> - priv->spi); > >> + ret = request_irq(spi->irq, p54spi_interrupt, IRQF_NO_AUTOEN, "p54spi", priv->spi); > > > > Simon says: This one seems more concerning in the context of this patch-set. > > > > Is there missing validation for spi->irq before calling request_irq()? > > > > If the device tree node lacks an interrupts property, the SPI core may set > > spi->irq to 0 or a negative error code. > > > > Passing this directly to request_irq() without validation can successfully > > hijack a core system interrupt like the system timer on some architectures, > > leading to system instability or deadlocks. > > > > Should there be a check like if (spi->irq <= 0) to fail the probe gracefully? > > I also commented on this: request_irq() already fails gracefully > with -EINVAL when presented with an invalid IRQ. IRQ 0 is guaranteed > to be invalid on any target that uses devicetree. Thanks, and sorry for not seeing your earlier comment. Or realising it is a false positive.