get_mdevice() returns a referenced struct device, and mISDN sockets keep that reference in _pms(sk)->dev after bind. That means mISDN_unregister_device() cannot tear the device down as if no socket can still reach it. Several teardown paths can otherwise run after delete_stack() has freed the stack, or after the driver has freed the embedding object once mISDN_unregister_device() returns. Close sockets that still point at the device before delete_stack() runs, wait for the final device release before returning from mISDN_unregister_device(), and serialize bind against unregister with the device lock so a new socket cannot attach after unregister has started. While tightening the close path, reset channel state after CLOSE_CHANNEL so later socket release does not try to tear the same B-channel down twice, and make recvmsg/getname tolerate sockets whose device pointer was cleared by unregister. Fixes: b36b654a7e82 ("mISDN: Create /sys/class/mISDN") Cc: stable@vger.kernel.org Assisted-by: Codex:GPT-5.3-Codex Signed-off-by: Shuvam Pandey --- drivers/isdn/mISDN/core.c | 19 ++- drivers/isdn/mISDN/core.h | 1 + drivers/isdn/mISDN/socket.c | 224 +++++++++++++++++++++++++++++++----- include/linux/mISDNif.h | 1 + 4 files changed, 216 insertions(+), 29 deletions(-) diff --git a/drivers/isdn/mISDN/core.c b/drivers/isdn/mISDN/core.c index 8ec2d4d4f1352..4e2be8f03119b 100644 --- a/drivers/isdn/mISDN/core.c +++ b/drivers/isdn/mISDN/core.c @@ -26,7 +26,9 @@ static DEFINE_RWLOCK(bp_lock); static void mISDN_dev_release(struct device *dev) { - /* nothing to do: the device is part of its parent's data structure */ + struct mISDNdevice *mdev = container_of(dev, struct mISDNdevice, dev); + + complete(&mdev->released); } static ssize_t id_show(struct device *dev, @@ -219,6 +221,7 @@ mISDN_register_device(struct mISDNdevice *dev, return err; dev->id = err; + init_completion(&dev->released); device_initialize(&dev->dev); if (name && name[0]) dev_set_name(&dev->dev, "%s", name); @@ -257,12 +260,24 @@ mISDN_unregister_device(struct mISDNdevice *dev) { printk(KERN_DEBUG "mISDN_unregister %s %d\n", dev_name(&dev->dev), dev->id); /* sysfs_remove_link(&dev->dev.kobj, "device"); */ + /* + * Remove the device from sysfs before taking dev->mutex so bind-side + * get_mdevice() users will fail the later device_is_registered() + * recheck after they acquire device_lock(). + */ device_del(&dev->dev); dev_set_drvdata(&dev->dev, NULL); - + device_lock(&dev->dev); + misdn_sock_release_device(dev); test_and_clear_bit(dev->id, (u_long *)&device_ids); delete_stack(dev); + device_unlock(&dev->dev); put_device(&dev->dev); + /* + * Drivers free the enclosing object after unregister returns, so wait + * until the last outstanding device reference is dropped. + */ + wait_for_completion(&dev->released); } EXPORT_SYMBOL(mISDN_unregister_device); diff --git a/drivers/isdn/mISDN/core.h b/drivers/isdn/mISDN/core.h index 5617c06de8e4d..2cd89293bc211 100644 --- a/drivers/isdn/mISDN/core.h +++ b/drivers/isdn/mISDN/core.h @@ -41,6 +41,7 @@ extern int connect_layer1(struct mISDNdevice *, struct mISDNchannel *, u_int, struct sockaddr_mISDN *); extern int create_l2entity(struct mISDNdevice *, struct mISDNchannel *, u_int, struct sockaddr_mISDN *); +void misdn_sock_release_device(struct mISDNdevice *dev); extern int create_stack(struct mISDNdevice *); extern int create_teimanager(struct mISDNdevice *); diff --git a/drivers/isdn/mISDN/socket.c b/drivers/isdn/mISDN/socket.c index 77b900db1cac2..bf3ad0a2a42bc 100644 --- a/drivers/isdn/mISDN/socket.c +++ b/drivers/isdn/mISDN/socket.c @@ -7,6 +7,7 @@ */ #include +#include #include #include #include "core.h" @@ -57,6 +58,97 @@ static void mISDN_sock_unlink(struct mISDN_sock_list *l, struct sock *sk) write_unlock_bh(&l->lock); } +/* + * Socket teardown driven by unregister takes device_lock(dev) before + * lock_sock(sk). Bind paths take the same order so unregister can close a + * socket without racing a new bind onto the same device. + */ +static struct sock * +misdn_sock_get(struct mISDN_sock_list *l, struct mISDNdevice *dev) +{ + struct sock *sk; + + read_lock_bh(&l->lock); + sk_for_each(sk, &l->head) { + if (READ_ONCE(_pms(sk)->dev) != dev) + continue; + sock_hold(sk); + read_unlock_bh(&l->lock); + return sk; + } + read_unlock_bh(&l->lock); + return NULL; +} + +static void data_sock_reset_channel(struct sock *sk) +{ + _pms(sk)->ch.protocol = ISDN_P_NONE; + _pms(sk)->ch.nr = 0; + _pms(sk)->ch.addr = 0; + _pms(sk)->ch.st = NULL; + _pms(sk)->ch.peer = NULL; + _pms(sk)->ch.recv = NULL; +} + +static void data_sock_close(struct sock *sk) +{ + bool active = _pms(sk)->ch.protocol != ISDN_P_NONE; + + sk->sk_state = MISDN_CLOSED; + + if (active) + delete_channel(&_pms(sk)->ch); + + data_sock_reset_channel(sk); + + if (_pms(sk)->dev) { + put_device(&_pms(sk)->dev->dev); + _pms(sk)->dev = NULL; + } +} + +static void base_sock_close(struct sock *sk) +{ + sk->sk_state = MISDN_CLOSED; + + if (_pms(sk)->dev) { + put_device(&_pms(sk)->dev->dev); + _pms(sk)->dev = NULL; + } +} + +void +misdn_sock_release_device(struct mISDNdevice *dev) +{ + struct sock *sk; + + if (dev->D.st) { + while ((sk = misdn_sock_get(&dev->D.st->l1sock, dev))) { + lock_sock(sk); + if (_pms(sk)->dev == dev) + data_sock_close(sk); + release_sock(sk); + sock_put(sk); + } + } + + while ((sk = misdn_sock_get(&data_sockets, dev))) { + lock_sock(sk); + if (_pms(sk)->dev == dev) + data_sock_close(sk); + release_sock(sk); + sock_put(sk); + } + + while ((sk = misdn_sock_get(&base_sockets, dev))) { + lock_sock(sk); + if (_pms(sk)->dev == dev) + base_sock_close(sk); + release_sock(sk); + sock_put(sk); + } +} + static int mISDN_send(struct mISDNchannel *ch, struct sk_buff *skb) { @@ -86,6 +178,14 @@ mISDN_ctrl(struct mISDNchannel *ch, u_int cmd, void *arg) switch (cmd) { case CLOSE_CHANNEL: msk->sk.sk_state = MISDN_CLOSED; + if (msk->ch.protocol >= ISDN_P_B_START) { + msk->ch.protocol = ISDN_P_NONE; + msk->ch.nr = 0; + msk->ch.addr = 0; + msk->ch.st = NULL; + msk->ch.peer = NULL; + msk->ch.recv = NULL; + } break; } return 0; @@ -127,18 +227,30 @@ mISDN_sock_recvmsg(struct socket *sock, struct msghdr *msg, size_t len, if (msg->msg_name) { DECLARE_SOCKADDR(struct sockaddr_mISDN *, maddr, msg->msg_name); + int dev_id, ch_nr, ch_addr; + + lock_sock(sk); + if (!_pms(sk)->dev) { + release_sock(sk); + skb_free_datagram(sk, skb); + return -EBADFD; + } + dev_id = _pms(sk)->dev->id; + ch_nr = _pms(sk)->ch.nr; + ch_addr = _pms(sk)->ch.addr; + release_sock(sk); maddr->family = AF_ISDN; - maddr->dev = _pms(sk)->dev->id; + maddr->dev = dev_id; if ((sk->sk_protocol == ISDN_P_LAPD_TE) || (sk->sk_protocol == ISDN_P_LAPD_NT)) { maddr->channel = (mISDN_HEAD_ID(skb) >> 16) & 0xff; maddr->tei = (mISDN_HEAD_ID(skb) >> 8) & 0xff; maddr->sapi = mISDN_HEAD_ID(skb) & 0xff; } else { - maddr->channel = _pms(sk)->ch.nr; - maddr->sapi = _pms(sk)->ch.addr & 0xFF; - maddr->tei = (_pms(sk)->ch.addr >> 8) & 0xFF; + maddr->channel = ch_nr; + maddr->sapi = ch_addr & 0xFF; + maddr->tei = (ch_addr >> 8) & 0xFF; } msg->msg_namelen = sizeof(*maddr); } @@ -241,16 +353,14 @@ data_sock_release(struct socket *sock) printk(KERN_DEBUG "%s(%p) sk=%p\n", __func__, sock, sk); if (!sk) return 0; + + lock_sock(sk); + switch (sk->sk_protocol) { case ISDN_P_TE_S0: case ISDN_P_NT_S0: case ISDN_P_TE_E1: case ISDN_P_NT_E1: - if (sk->sk_state == MISDN_BOUND) - delete_channel(&_pms(sk)->ch); - else - mISDN_sock_unlink(&data_sockets, sk); - break; case ISDN_P_LAPD_TE: case ISDN_P_LAPD_NT: case ISDN_P_B_RAW: @@ -259,13 +369,11 @@ data_sock_release(struct socket *sock) case ISDN_P_B_L2DTMF: case ISDN_P_B_L2DSP: case ISDN_P_B_L2DSPHDLC: - delete_channel(&_pms(sk)->ch); + data_sock_close(sk); mISDN_sock_unlink(&data_sockets, sk); break; } - lock_sock(sk); - sock_orphan(sk); skb_queue_purge(&sk->sk_receive_queue); @@ -466,6 +574,7 @@ data_sock_bind(struct socket *sock, struct sockaddr_unsized *addr, int addr_len) { struct sockaddr_mISDN *maddr = (struct sockaddr_mISDN *) addr; struct sock *sk = sock->sk; + struct mISDNdevice *dev, *lockdev; struct sock *csk; int err = 0; @@ -477,13 +586,35 @@ data_sock_bind(struct socket *sock, struct sockaddr_unsized *addr, int addr_len) return -EINVAL; lock_sock(sk); + if (sk->sk_state == MISDN_CLOSED) { + release_sock(sk); + return -EBADFD; + } + if (_pms(sk)->dev) { + release_sock(sk); + return -EALREADY; + } + release_sock(sk); + + dev = get_mdevice(maddr->dev); + if (!dev) + return -ENODEV; + + lockdev = dev; + device_lock(&dev->dev); + lock_sock(sk); + + if (sk->sk_state == MISDN_CLOSED) { + err = -EBADFD; + goto done; + } if (_pms(sk)->dev) { err = -EALREADY; goto done; } - _pms(sk)->dev = get_mdevice(maddr->dev); - if (!_pms(sk)->dev) { + + if (!device_is_registered(&dev->dev)) { err = -ENODEV; goto done; } @@ -493,7 +624,7 @@ data_sock_bind(struct socket *sock, struct sockaddr_unsized *addr, int addr_len) sk_for_each(csk, &data_sockets.head) { if (sk == csk) continue; - if (_pms(csk)->dev != _pms(sk)->dev) + if (READ_ONCE(_pms(csk)->dev) != dev) continue; if (csk->sk_protocol >= ISDN_P_B_START) continue; @@ -516,15 +647,15 @@ data_sock_bind(struct socket *sock, struct sockaddr_unsized *addr, int addr_len) case ISDN_P_TE_E1: case ISDN_P_NT_E1: mISDN_sock_unlink(&data_sockets, sk); - err = connect_layer1(_pms(sk)->dev, &_pms(sk)->ch, - sk->sk_protocol, maddr); + err = connect_layer1(dev, &_pms(sk)->ch, sk->sk_protocol, + maddr); if (err) mISDN_sock_link(&data_sockets, sk); break; case ISDN_P_LAPD_TE: case ISDN_P_LAPD_NT: - err = create_l2entity(_pms(sk)->dev, &_pms(sk)->ch, - sk->sk_protocol, maddr); + err = create_l2entity(dev, &_pms(sk)->ch, sk->sk_protocol, + maddr); break; case ISDN_P_B_RAW: case ISDN_P_B_HDLC: @@ -532,19 +663,26 @@ data_sock_bind(struct socket *sock, struct sockaddr_unsized *addr, int addr_len) case ISDN_P_B_L2DTMF: case ISDN_P_B_L2DSP: case ISDN_P_B_L2DSPHDLC: - err = connect_Bstack(_pms(sk)->dev, &_pms(sk)->ch, - sk->sk_protocol, maddr); + err = connect_Bstack(dev, &_pms(sk)->ch, sk->sk_protocol, + maddr); break; default: err = -EPROTONOSUPPORT; } - if (err) + if (err) { + data_sock_reset_channel(sk); goto done; + } + _pms(sk)->dev = dev; + dev = NULL; sk->sk_state = MISDN_BOUND; _pms(sk)->ch.protocol = sk->sk_protocol; done: release_sock(sk); + device_unlock(&lockdev->dev); + if (dev) + put_device(&dev->dev); return err; } @@ -555,10 +693,11 @@ data_sock_getname(struct socket *sock, struct sockaddr *addr, struct sockaddr_mISDN *maddr = (struct sockaddr_mISDN *) addr; struct sock *sk = sock->sk; - if (!_pms(sk)->dev) - return -EBADFD; - lock_sock(sk); + if (!_pms(sk)->dev) { + release_sock(sk); + return -EBADFD; + } maddr->family = AF_ISDN; maddr->dev = _pms(sk)->dev->id; @@ -623,6 +762,10 @@ base_sock_release(struct socket *sock) if (!sk) return 0; + lock_sock(sk); + base_sock_close(sk); + release_sock(sk); + mISDN_sock_unlink(&base_sockets, sk); sock_orphan(sk); sock_put(sk); @@ -700,6 +843,7 @@ base_sock_bind(struct socket *sock, struct sockaddr_unsized *addr, int addr_len) { struct sockaddr_mISDN *maddr = (struct sockaddr_mISDN *) addr; struct sock *sk = sock->sk; + struct mISDNdevice *dev, *lockdev; int err = 0; if (addr_len < sizeof(struct sockaddr_mISDN)) @@ -709,21 +853,47 @@ base_sock_bind(struct socket *sock, struct sockaddr_unsized *addr, int addr_len) return -EINVAL; lock_sock(sk); + if (sk->sk_state == MISDN_CLOSED) { + release_sock(sk); + return -EBADFD; + } + if (_pms(sk)->dev) { + release_sock(sk); + return -EALREADY; + } + release_sock(sk); + + dev = get_mdevice(maddr->dev); + if (!dev) + return -ENODEV; + + lockdev = dev; + device_lock(&dev->dev); + lock_sock(sk); + + if (sk->sk_state == MISDN_CLOSED) { + err = -EBADFD; + goto done; + } if (_pms(sk)->dev) { err = -EALREADY; goto done; } - _pms(sk)->dev = get_mdevice(maddr->dev); - if (!_pms(sk)->dev) { + if (!device_is_registered(&dev->dev)) { err = -ENODEV; goto done; } + _pms(sk)->dev = dev; + dev = NULL; sk->sk_state = MISDN_BOUND; done: release_sock(sk); + device_unlock(&lockdev->dev); + if (dev) + put_device(&dev->dev); return err; } diff --git a/include/linux/mISDNif.h b/include/linux/mISDNif.h index 7aab4a7697369..ce26d70c1ebfb 100644 --- a/include/linux/mISDNif.h +++ b/include/linux/mISDNif.h @@ -499,6 +499,7 @@ struct mISDNdevice { u_char channelmap[MISDN_CHMAP_SIZE]; struct list_head bchannels; struct mISDNchannel *teimgr; + struct completion released; struct device dev; }; -- 2.50.1 (Apple Git-155)