linux-kernel - Re: [PATCH 1/3] TTY: serial, remove BTM from wait_until

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: <201107122325.35013.arnd@arndb.de>
Date:	Tue, 12 Jul 2011 23:25:34 +0200
From:	Arnd Bergmann <arnd@...db.de>
To:	Jiri Slaby <jslaby@...e.cz>
Cc:	gregkh@...e.de, linux-serial@...r.kernel.org,
	linux-kernel@...r.kernel.org, jirislaby@...il.com,
	Alan Cox <alan@...ux.intel.com>, Andreas Bombe <aeb@...ian.org>
Subject: Re: [PATCH 1/3] TTY: serial, remove BTM from wait_until_sent

On Tuesday 12 July 2011 22:43:18 Jiri Slaby wrote:
> During the BKL removal process, the BKL was switched to tty_lock
> (BTM). Now we should start pruning the BTM further. Let's start with
> wait_until_sent of the serial layer. This will allow us to switch to
> the tty port helpers and thus clean it up much.
> 
> In wait_until_sent there are some uport members accessed, but neither
> of them is protected by BTM at the location they are set ('=>' means
> function call):
> * uport->fifosize (set in tty_ioctl => uart_ioctl => uart_set_info)
> * uport->type (set in add_one_port prior to tty_register_device)
> * uport->timeout (set usually in tty_ioctl => tty_mode_ioctl =>
>   tty_set_termios => uart_set_termios => uart_change_speed =>
>   uport->ops->set_termios => uart_update_timeout)
> * call to uport->ops->tx_empty()
> 
> If the tx_empty hook needs some lock to protect accesses to registers,
> it should take &uport->lock spinlock like 8250 does. Otherwise there
> still might be races e.g. with ISRs.
> 
> This should also fix the issue Andreas is seeing (BTM in comparison to
> BKL doesn't have any hidden functionality like unlocking during
> sleeping).

Looks good to me. I would suggest also cleaning up the amiserial.c
driver, which uses a different approach to do the same, by effectively
implementing its own recursive lock.

If uart_wait_until_sent doesn't need the BTM, then amiserial also
shouldn't need it. It does an MMIO access, but it doesn't look like
that needs to be protected. After that is done, we should be able
to kill tty_locked() and __big_tty_mutex_owner as well.

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