lkml.org 
[lkml]   [2023]   [Apr]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] serial: fix TIOCSRS485 locking
On Wed, 12 Apr 2023, Johan Hovold wrote:

> The RS485 multipoint addressing support for some reason added a new
> ADDRB termios cflag which is (only!) updated from one of the RS485
> ioctls.
>
> Make sure to take the termios rw semaphore for the right ioctl (i.e.
> set, not get).
>
> Fixes: ae50bb275283 ("serial: take termios_rwsem for ->rs485_config() & pass termios as param")
> Cc: stable@vger.kernel.org # 6.0
> Cc: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> Signed-off-by: Johan Hovold <johan@kernel.org>
> ---
>
> I did not have time to review the multipoint addressing patches at the
> time and only skimmed the archives now, but I can't seem to find any
> motivation for why a precious termios bit was seemingly wasted on ADDRB
> when it is only updated from the RS485 ioctls.
>
> I hope it wasn't done just to simplify the implementation of
> tty_get_frame_size()? Or was it a left-over from the RFC which
> apparently actually used termios to enable this feature?

No. I made it intentionally. It felt natural place for storing it because
ADDRB does impact the wire format and cflag is where other wire-format
impacting bits are also stored.

> Should we consider dropping the Linux-specific ADDRB bit again?
>
> Johan
>
>
> drivers/tty/serial/serial_core.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> index 2bd32c8ece39..728cb72be066 100644
> --- a/drivers/tty/serial/serial_core.c
> +++ b/drivers/tty/serial/serial_core.c
> @@ -1552,7 +1552,7 @@ uart_ioctl(struct tty_struct *tty, unsigned int cmd, unsigned long arg)
> goto out;
>
> /* rs485_config requires more locking than others */
> - if (cmd == TIOCGRS485)
> + if (cmd == TIOCSRS485)
> down_write(&tty->termios_rwsem);
>
> mutex_lock(&port->mutex);
> @@ -1595,7 +1595,7 @@ uart_ioctl(struct tty_struct *tty, unsigned int cmd, unsigned long arg)
> }
> out_up:
> mutex_unlock(&port->mutex);
> - if (cmd == TIOCGRS485)
> + if (cmd == TIOCSRS485)
> up_write(&tty->termios_rwsem);
> out:
> return ret;
>

Indeed, the caps are so blinding.

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

--
i.
\
 
 \ /
  Last update: 2023-04-12 15:06    [W:0.026 / U:1.388 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site