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 for Android: free password hash cracker in your pocket

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

Date:	Thu, 19 Nov 2009 17:38:55 +0100 (CET)
From:	Jiri Kosina <jkosina@...e.cz>
To:	Alan Cox <alan@...ux.intel.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Greg Kroah-Hartman <gregkh@...e.de>
Cc:	linux-serial@...r.kernel.org, linux-kernel@...r.kernel.org,
	Markus Armbruster <armbru@...hat.com>,
	Ian Jackson <ian.jackson@...citrix.com>,
	Michal Hocko <mhocko@...e.cz>
Subject: Re: [PATCH] Do not read IIR in serial8250_start_tx when
 UART_BUG_TXEN

On Wed, 18 Nov 2009, Jiri Kosina wrote:

> The patch below has been lost several times (the last submission happened 
> probably on [1]). I am currently aware of a real hardware which triggers 
> this problem quite reliably, so we should rather have that really fixed.
> 
> [1] http://lkml.org/lkml/2009/3/12/379
> 
> 
> 
> 
> From: Ian Jackson <ian.jackson@...citrix.com>
> 
> Do not read IIR in serial8250_start_tx when UART_BUG_TXEN
> 
> Reading the IIR clears some oustanding interrupts so it is not safe.
> Instead, simply transmit immediately if the buffer is empty without
> regard to IIR.
> 
> Signed-off-by: Ian Jackson <ian.jackson@...citrix.com>
> Reviewed-by: Markus Armbruster <armbru@...hat.com>
> Reviewed-by: Jiri Kosina <jkosina@...e.cz>
> 
> ---
>  drivers/serial/8250.c |    8 +++-----
>  1 files changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/serial/8250.c b/drivers/serial/8250.c
> index 737b4c9..807042b 100644
> --- a/drivers/serial/8250.c
> +++ b/drivers/serial/8250.c
> @@ -1339,14 +1339,12 @@ static void serial8250_start_tx(struct uart_port *port)
>  		serial_out(up, UART_IER, up->ier);
>  
>  		if (up->bugs & UART_BUG_TXEN) {
> -			unsigned char lsr, iir;
> +			unsigned char lsr;
>  			lsr = serial_in(up, UART_LSR);
>  			up->lsr_saved_flags |= lsr & LSR_SAVE_FLAGS;
> -			iir = serial_in(up, UART_IIR) & 0x0f;
>  			if ((up->port.type == PORT_RM9000) ?
> -				(lsr & UART_LSR_THRE &&
> -				(iir == UART_IIR_NO_INT || iir == UART_IIR_THRI)) :
> -				(lsr & UART_LSR_TEMT && iir & UART_IIR_NO_INT))
> +				(lsr & UART_LSR_THRE) :
> +				(lsr & UART_LSR_TEMT))
>  				transmit_chars(up);
>  		}
>  	}

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).

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.

Alan, Greg, any comments?

Thanks,

-- 
Jiri Kosina
SUSE Labs, Novell Inc.
--
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/