linux-kernel - Re: [PATCH] mmc_spi: use EILSEQ for possible transmission errors

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <200905201035.23964.wolfgang.mues@auerswald.de>
Date:	Wed, 20 May 2009 10:35:23 +0200
From:	Wolfgang Mües <wolfgang.mues@...rswald.de>
To:	"David Brownell" <david-b@...bell.net>
Cc:	"Matt Fleming" <matt@...sole-pimps.org>,
	"Pierre Ossman" <drzeus@...eus.cx>,
	"Andrew Morton" <akpm@...ux-foundation.org>,
	"Mike Frysinger" <vapier.adi@...il.com>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] mmc_spi: use EILSEQ for possible transmission errors

Am Mittwoch, 20. Mai 2009 schrieb David Brownell:
> On Tuesday 19 May 2009, Matt Fleming wrote:
> > Hmm, always returning -EILSEQ is devious. What happens if we sent an
> > illegal command? The value of "value" is passed up to the callers via
> > cmd->error and so may eventually get printed in the pr_debug() call in
> > mmc_request_done(), line 86.
>
> True, but a pr_debug from mmc_spi could help that.  A patch doing
> that would need to be less aggressive about ripping out the current
> fault-parsing logic, but it could continue reporting -EILSEQ to
> cope with the possible response mangling.

what about the following proposal which is not so aggressive?

The current mapping in mmc_spi.c is:

R1_SPI_PARAMETER => EINVAL
R1_SPI_ADDRESS => EINVAL
R1_SPI_ILLEGAL_COMMAND => EINVAL
R1_SPI_COM_CRC => EILSEQ
R1_SPI_ERASE_SEQ => EIO
R1_SPI_ERASE_RESET => EIO

R1_SPI_COM_CRC is a transmission error - no doubt. Mapping to EILSEQ is OK.

R1_SPI_ERASE_SEQ and R1_SPI_ERASE_RESET are responses to a sector erase 
command. block.c is not sending such commands, so block.c can safely assume 
that each such response is a transmission error.

R1_SPI_PARAMETER and R1_SPI_ADDRESS are errors from the card controller
because a parameter or an address is out of range/invalid. It is mapped to 
EINVAL, and so the caller can not distinguish between errors from the host 
controller and driver or the card. This has to be changed to some other code,
maybe EFAULT /* Bad address */.

R1_SPI_ILLEGAL_COMMAND is mapped to EINVAL, and I suggest it to replace with
ENOSYS /* Function not implemented */.

A quick scan in the mmc directory for EINVAL reveals that there are no callers 
which need to be changed.

As block.c does not send a command, parameter or address which is invalid/out 
of range, these errors must be considered transmission errors in the command 
(if CRC is off) or in the response.

regards

i. A. Wolfgang Mües
-- 
Auerswald GmbH & Co. KG
Hardware Development
Telefon: +49 (0)5306 9219 0
Telefax: +49 (0)5306 9219 94 
E-Mail: Wolfgang.Mues@...rswald.de
Web: http://www.auerswald.de

--------------------------------------------------------------
Auerswald GmbH & Co. KG, Vor den Grashöfen 1, 38162 Cremlingen
Registriert beim AG Braunschweig HRA 13289
p.h.G Auerswald Geschäftsführungsges. mbH
Registriert beim AG Braunschweig HRB 7463
Geschäftsführer: Dipl-Ing. Gerhard Auerswald
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/