Re: [PATCH] spi: atmel: Prevent spi transfers from being killed

From: Miquel Raynal <miquel.raynal@bootlin.com>
To: Richard Weinberger <richard@nod.at>
Cc: Mark Brown <broonie@kernel.org>,
	linux-spi <linux-spi@vger.kernel.org>,
	Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
	Ryan Wanner <ryan.wanner@microchip.com>,
	Ronald Wahl <ronald.wahl@raritan.com>,
	David Laight <David.Laight@ACULAB.COM>,
	Nicolas Ferre <nicolas.ferre@microchip.com>,
	Claudiu Beznea <claudiu.beznea@tuxon.dev>,
	Alexandre Belloni <alexandre.belloni@bootlin.com>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
	stable <stable@vger.kernel.org>
Subject: Re: [PATCH] spi: atmel: Prevent spi transfers from being killed
Date: Tue, 5 Dec 2023 10:28:21 +0100	[thread overview]
Message-ID: <20231205102821.224ccbe6@xps-13> (raw)
In-Reply-To: <1788823860.72909.1701768176780.JavaMail.zimbra@nod.at>

Hi Richard,

richard@nod.at wrote on Tue, 5 Dec 2023 10:22:56 +0100 (CET):

> ----- Ursprüngliche Mail -----
> > Von: "Miquel Raynal" <miquel.raynal@bootlin.com>
> > All being well, it was reported that JFFS2 was showing a splat when
> > interrupting a transfer. After some more debate about whether JFFS2
> > should be fixed and how, it was also pointed out that the whole
> > consistency of the filesystem in case of parallel I/O would be
> > compromised. Changing JFFS2 behavior would in theory be possible but
> > nobody has the energy and time and knowledge to do this now, so better
> > prevent spi transfers to be interrupted by the user.  
> 
> Well, it's not really an JFFS2 issue.
> The real problem is, that with the said change anyone can abort an IO.
> Usually file systems assume that an IO can only fail in fatal situations.
> That's why UBIFS, for example, switches immediately to read-only mode.
> So, an unprivileged user can force UBIFS into read-only mode, which is a
> local DoS attack vector.

Right.

> JFFS2, on the other hand, dies a different death. If you abort one IO,
> another IO path can still be active and will violate the order of written
> data.
> 
> Long story short, aborting pure user inflicted IO is fine. This is the "dd"
> use case.
> But as soon a filesystem is on top, things get complicated.
> 
> Maybe it is possible to teach the SPI subsystem whether an IO comes from spidev
> or the kernel itself?

Well, it would only partially fix the problem, I was playing with a
spi-nor or spi-nand chip (don't remember) which was supported in the
kernel, just making big reads/writes (without fs, at this time). I
don't think deliberating on whether the driver requesting the transfer
is in the kernel or in userspace matters, but whether there is a
filesystem on top or not. But TBH I don't think this can be solved
without ugly hacks...

Thanks,
Miquèl