Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

tty: rpmsg: Fix race condition releasing tty port

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 while it is still 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 :
- Introduce the .destruct port tty ops function to release the allocated
rpmsg_tty_port structure.
- Introduce the .hangup tty ops function to call tty_port_hangup.
- Manages the tty port refcounting to trig the .destruct port ops,
- Introduces the rpmsg_tty_cleanup function to ensure that the TTY is
removed before decreasing the port refcount.

Fixes: 7c0408d80579 ("tty: add rpmsg driver")
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@foss.st.com>
Link: https://lore.kernel.org/r/20220104163545.34710-1-arnaud.pouliquen@foss.st.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

authored by

Arnaud Pouliquen and committed by
Greg Kroah-Hartman
db7f19c0 f23653fe

+26 -14
+26 -14
drivers/tty/rpmsg_tty.c
··· 50 50 static int rpmsg_tty_install(struct tty_driver *driver, struct tty_struct *tty) 51 51 { 52 52 struct rpmsg_tty_port *cport = idr_find(&tty_idr, tty->index); 53 + struct tty_port *port; 53 54 54 55 tty->driver_data = cport; 55 56 56 - return tty_port_install(&cport->port, driver, tty); 57 + port = tty_port_get(&cport->port); 58 + return tty_port_install(port, driver, tty); 59 + } 60 + 61 + static void rpmsg_tty_cleanup(struct tty_struct *tty) 62 + { 63 + tty_port_put(tty->port); 57 64 } 58 65 59 66 static int rpmsg_tty_open(struct tty_struct *tty, struct file *filp) ··· 113 106 return size; 114 107 } 115 108 109 + static void rpmsg_tty_hangup(struct tty_struct *tty) 110 + { 111 + tty_port_hangup(tty->port); 112 + } 113 + 116 114 static const struct tty_operations rpmsg_tty_ops = { 117 115 .install = rpmsg_tty_install, 118 116 .open = rpmsg_tty_open, 119 117 .close = rpmsg_tty_close, 120 118 .write = rpmsg_tty_write, 121 119 .write_room = rpmsg_tty_write_room, 120 + .hangup = rpmsg_tty_hangup, 121 + .cleanup = rpmsg_tty_cleanup, 122 122 }; 123 123 124 124 static struct rpmsg_tty_port *rpmsg_tty_alloc_cport(void) ··· 151 137 return cport; 152 138 } 153 139 154 - static void rpmsg_tty_release_cport(struct rpmsg_tty_port *cport) 140 + static void rpmsg_tty_destruct_port(struct tty_port *port) 155 141 { 142 + struct rpmsg_tty_port *cport = container_of(port, struct rpmsg_tty_port, port); 143 + 156 144 mutex_lock(&idr_lock); 157 145 idr_remove(&tty_idr, cport->id); 158 146 mutex_unlock(&idr_lock); ··· 162 146 kfree(cport); 163 147 } 164 148 165 - static const struct tty_port_operations rpmsg_tty_port_ops = { }; 149 + static const struct tty_port_operations rpmsg_tty_port_ops = { 150 + .destruct = rpmsg_tty_destruct_port, 151 + }; 152 + 166 153 167 154 static int rpmsg_tty_probe(struct rpmsg_device *rpdev) 168 155 { ··· 185 166 cport->id, dev); 186 167 if (IS_ERR(tty_dev)) { 187 168 ret = dev_err_probe(dev, PTR_ERR(tty_dev), "Failed to register tty port\n"); 188 - goto err_destroy; 169 + tty_port_put(&cport->port); 170 + return ret; 189 171 } 190 172 191 173 cport->rpdev = rpdev; ··· 197 177 rpdev->src, rpdev->dst, cport->id); 198 178 199 179 return 0; 200 - 201 - err_destroy: 202 - tty_port_destroy(&cport->port); 203 - rpmsg_tty_release_cport(cport); 204 - 205 - return ret; 206 180 } 207 181 208 182 static void rpmsg_tty_remove(struct rpmsg_device *rpdev) ··· 206 192 dev_dbg(&rpdev->dev, "Removing rpmsg tty device %d\n", cport->id); 207 193 208 194 /* User hang up to release the tty */ 209 - if (tty_port_initialized(&cport->port)) 210 - tty_port_tty_hangup(&cport->port, false); 195 + tty_port_tty_hangup(&cport->port, false); 211 196 212 197 tty_unregister_device(rpmsg_tty_driver, cport->id); 213 198 214 - tty_port_destroy(&cport->port); 215 - rpmsg_tty_release_cport(cport); 199 + tty_port_put(&cport->port); 216 200 } 217 201 218 202 static struct rpmsg_device_id rpmsg_driver_tty_id_table[] = {