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: <20090520075327.GA11473@console-pimps.org>
Date:	Wed, 20 May 2009 08:53:27 +0100
From:	Matt Fleming <matt@...sole-pimps.org>
To:	Wolfgang Mües <wolfgang.mues@...rswald.de>,
	David Brownell <david-b@...bell.net>
Cc:	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

On Tue, May 19, 2009 at 09:49:07PM -0700, David Brownell wrote:
> 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.
> 

Yes, a less aggressive patch would be fine.

> > Whereas before the error would display EINVAL for an illegal command
> > now it'll display EILSEQ, which makes no sense. Seeing EILSEQ in my
> > log when really the error is EINVAL is gonna really confuse me.
> > 
> > IMHO always assuming that command errors are caused by transmission
> > problems is not the right solution.
> 
> Do you have a better solution to propose though?  If Wolfgang
> is actually observing transmission errors there, I'm not sure
> a better one is to be had.
> 

You're right. It's not cool for me to criticise and not give any
suggestions. I've been trying to come up with some middle-ground
solution since yesterday.

I think if the patch was less aggressive at returning EILSEQ, it'd be
fine. For instance, returning EILSEQ if the response you get doesn't
make any sense given the command you sent or if the response is just
garbage.

It might even make sense to retry commands (say twice, and only if
it's safe to do so) and if the response codes differ in bizarre ways
then the status bits have probably been garbled on the bus and EILSEQ
is the correct error code to use. I'm less sure about this bit though,
it depends on exactly what we're trying to fix.

Wolfgang, is there a particular scenario where you're seeing the
status bits trashed by noise on the line? Or is this just a cautionary
patch?

On Tue, May 19, 2009 at 01:47:45PM +0200, Wolfgang Mües wrote:
> 
> > IMHO always assuming that command errors are caused by transmission
> > problems is not the right solution.
> 
> It's not your or my fault, it's the fault of the spi SD/MMC card protocol 
> designer. You may send him to me, already prepared: naked and in chains ;-)

This is the funniest reply I've ever read.
--
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/