linux-kernel - Re: [PATCH] serial_core: fix uart PORT

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: <20140409133909.2073453a@alan.etchedpixels.co.uk>
Date:	Wed, 9 Apr 2014 13:39:09 +0100
From:	One Thousand Gnomes <gnomes@...rguk.ukuu.org.uk>
To:	Thomas Pfaff <tpfaff@....com>
Cc:	"gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
	"jslaby@...e.cz" <jslaby@...e.cz>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] serial_core: fix uart PORT_UNKNOWN handling

On Wed, 9 Apr 2014 13:22:14 +0200
Thomas Pfaff <tpfaff@....com> wrote:

> From: "Thomas Pfaff" <tpfaff@....com>
> 
> While porting a RS485 driver from 2.6.29 to 3.14, i noticed that the serial tty 
> driver could break it by using uart ports that it does not own :

Yes thats one spectacularly ugly driver that doesn't want integrating
into 8250.c - although you do need to pull some things the other way for
modern ports as it'll blow up spectacularly if 8250.c has set it up for
DMA etc!

You ought however to be able to re-implement it as a line discipline at
least for fixed ports (it's never going to work on USB ones).

The low level driver calls the ldisc->dcd_change method on a DCD, and you
can issue set_termios requests in order to change the other pins. Adding
a method for other bits used that way is trivial and better than the
existing staging horror.

> 1. uart_change_pm ist called during uart_open and calls the uart pm function
>    without checking for PORT_UNKNOWN.

Removing this breaks other parts of the code assume that the port will be
powered up (notably setserial paths). So it makes sense that
uart_change_pm for a "none" port is a no-op but needs logic in the
setserial path to power up a port when we move none->known and power it
down on known->none

> 2. uart_shutdown is called from uart_set_info and does not check it either.

I don't see why this one matters. We would have done

	uart_startup
		uart_port_startup
			uport->type == PORT_UNKNOWN
			return 1;
		ASYNCB_INITIALIZED is not set

	uart_shutdown
		ASYNCB_INITIALISED is not set
			Skip call to uart_port_shutdown

So that code looks correct to me.

> 3. The return code from the uart request_port call in uart_set_info is not
>    handled properly, leading to the situation that the serial driver also
>    thinks it owns the uart ports.
>    This can triggered by doing following actions :
> 
>    setserial /dev/ttyS0 uart none    # release the uart ports
>    modprobe lirc-serial              # or any other device that uses the uart

This bit looks correct.

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