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@xxxxxxxxxxx>
thanks,
--
js
suse labs
