linux-kernel - Re: [PATCH] Do not read IIR in serial8250_start_tx when UART_BUG

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] [day] [month] [year] [list]

Date:	Thu, 19 Nov 2009 18:15:49 +0000
From:	Ian Jackson <Ian.Jackson@...citrix.com>
To:	Jiri Kosina <jkosina@...e.cz>
CC:	Alan Cox <alan@...ux.intel.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Greg Kroah-Hartman <gregkh@...e.de>,
	"linux-serial@...r.kernel.org" <linux-serial@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Markus Armbruster <armbru@...hat.com>,
	Michal Hocko <mhocko@...e.cz>
Subject: Re: [PATCH] Do not read IIR in serial8250_start_tx when
 UART_BUG_TXEN

Jiri Kosina writes ("Re: [PATCH] Do not read IIR in serial8250_start_tx when UART_BUG_TXEN"):
> Does anyone have any comments about this please?
> 
> In a nutshell -- this is needed so that we make the UART_BUG_TXEN really 
> harmless (which I guess it originally inteded to be, but reading IIR has 
> some unwanted sideeffects and is in fact not needed).

Yes.

> We need to have this in to handle properly the cases in which BUG_TXEN is 
> misdetected, and we can't blacklist such systems as we do for some SoL 
> hardware (see commit b6adea334c6c (" 8250: fix boot hang with serial 
> console when using with Serial Over Lan port"). Also there doesn't seem to 
> be any straightforward way to workaround the misdetection, so this seems 
> to be proper fix, unbreaking all the possible scenarios.

Unless the code has changed, the UART_BUG_TXEN detection is
fundamentally broken and racy.  So fixing that too would be good (and
possible, when I looked at it).

But even without fixing the UART_BUG_TXEN detection, the IIR read is
absolutely and definitely wrong.  The only purpose of the IIR read is
to avoid an unnecessary but harmless call to transmit_chars.

It seems to me that the author of the UART_BUG_TXEN patch did not
appreciate that reading the IIR was not side effect free, and that
they were just trying a minor (and perhaps ill-advised) optimisation.

In any case we don't have any useful information (in comments or
commit logs) explaining why they did it this way and it is (a) easily
seen to be buggy and (b) causes actual real problems.

So my patch should be applied.  The last time we discussed this we had
some kind of FUD along the lines of "but real serial ports often have
bugs so you can't reason about them properly".  I don't think that's a
sensible basis for discussion.  How can it be falsified ?

If there is some specific bug that real ports have that might be
relevant, sure, take it into account.  But we should not have the
situation where we hear the mere assertion that there often are bugs;
therefore reasoning doesn't show change to be safe; therefore no
change can be made.

Ian.
--
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/