lkml.org 
[lkml]   [2021]   [Dec]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH] tty: rpmsg: Fix race condition releasing tty port
From
Hi,

On 13. 12. 21, 20:53, Arnaud Pouliquen wrote:
> In current implementation the tty_port struct is part of the
> rpmsg_tty_port structure.The issue is that the rpmsg_tty_port structure is
> freed on rpmsg_tty_remove but also referenced in the tty_struct.
> Its release is not predictable due to workqueues.
>
> For instance following ftrace shows that rpmsg_tty_close is called after
> rpmsg_tty_release_cport:
>
> nr_test.sh-389 [000] ..... 212.093752: rpmsg_tty_remove <-rpmsg_dev_
> remove
> cat-1191 [001] ..... 212.095697: tty_release <-__fput
> nr_test.sh-389 [000] ..... 212.099166: rpmsg_tty_release_cport <-rpm
> sg_tty_remove
> cat-1191 [001] ..... 212.115352: rpmsg_tty_close <-tty_release
> cat-1191 [001] ..... 212.115371: release_tty <-tty_release_str
>
> As consequence, the port must be free only when user has released the TTY
> interface.
>
> This path (inspired from vcc.c):
> - moves the management of the port in the install and clean-up tty ops,
> - allocates the tty_port struct independently of the rpmsg_tty_port structure,

This looks rather wrong. Why not to use tty_port refcounting?

> - uses tty_vhangup and tty_port_hangup.

OK, but don't store a tty pointer as it looks racy. You should use
tty_port_tty_get instead.

Hm, we look we need tty_port_tty_vhangup (aside from
tty_port_tty_hangup). There are plenty of drivers doing:
tty = tty_port_tty_get(port);
if (tty) {
tty_vhangup(port->tty);
tty_kref_put(tty);


> Fixes: 7c0408d80579 ("tty: add rpmsg driver")
> Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@foss.st.com>

thanks,
--
js
suse labs

\
 
 \ /
  Last update: 2021-12-14 07:43    [W:0.134 / U:0.428 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site