linux-kernel - Designware 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-next>] [day] [month] [year] [list]

Message-ID: <86057f94-3b52-7c12-21b5-be90564fcf85@schinagl.nl>
Date:   Wed, 3 May 2017 12:17:55 +0200
From:   Olliver Schinagl <oliver@...inagl.nl>
To:     jamie@...ieiles.com, tim.kryger@...aro.org
Cc:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        dev <dev@...ux-sunxi.org>,
        Maxime Ripard <maxime.ripard@...e-electrons.com>
Subject: Designware UART bug

Hey Jamie,

Several years ago you wrote the glue-code [0] for the DW 8250 IP. Over 
the years various 'fixes' have been applied to resolve certain 'weird' 
problems that Tim tried to fix with [1].

After going over the datasheets and code with a comb several times now, 
I think I may have found one (of a few others) reasons and would like 
both your and Tim's thoughts here.

The current (and original) code [2] uses the register offset 0x1f for 
the UART_USR register.

I searched far and wide, various datasheet of physical uarts (8250 - 
16950) and some IP cores and none seem to have the USR (and specifically 
the USR[0] bit) register, so it seems to be specific to the DW_apb_uart. 
However looking at the only databook available to me [3] of the UART IP, 
I cannot seem to find anything at register offset 0x1f.

The platform I'm using uses the Allwinner A20 SoC, which also features 
the DW uart IP and also here, there is nothing at offset 0x1f.

The intended register IS however actually at 0x7c.

My question is thus twofold.

Why was 0x1f used? Is this specific to a certain (version) UART or is 
this a long uncaught typo.

Tim, assuming it is a typo, could this the cause which made you 
implement [1]? From what I understand, you keep writing the LCR until it 
takes.

Initially, the UART_IIR_BUSY check looked like this:
	if (serial8250_handle_irq(p, iir)) {
                 return 1;
         } else if ((iir & UART_IIR_BUSY) == UART_IIR_BUSY) {
                 /* Clear the USR and write the LCR again. */
                 (void)p->serial_in(p, d->usr_reg);
                 p->serial_out(p, UART_LCR, d->last_lcr);

                 return 1;
         }

what I'm missing here is, that if UART_IIR_BUSY is set, we have:
* check the d->usr_reg (UART_USR) bit 0
* wait until it becomes cleared (do not allow new data to be pushed out, 
optionally force the data to be pushed out)
* write LCR register (and check if it took (and no longer loop over the 
LCR to see if the values stuck, in theory).
* if we never get un-busy, something is wrong?

All of this btw is currently moot anyway, as the only way to get into 
the (else) if, is if serial8250_handle_irq returns false. And from what 
I can see, this is only if iir == UART_IIR_NO_IRQ, in which case we 
never ever clear the USR and thus never ever clear the UART_IIR_BUSY flag.

Olliver

[0] 
https://github.com/torvalds/linux/commit/6b1a98d1c4851235d9b6764b3f7b7db7909fc760

[1] 
https://github.com/torvalds/linux/commit/c49436b657d0a56a6ad90d14a7c3041add7cf64d

[2] 
https://github.com/torvalds/linux/commit/6b1a98d1c4851235d9b6764b3f7b7db7909fc760#diff-d6e619fc4c51febf7880632fde5d0208R63

[3] http://linux-sunxi.org/images/d/d2/Dw_apb_uart_db.pdf