[PATCH] mISDN: socket: drop device references acquired by get_mdevice()

[PATCH] mISDN: socket: drop device references acquired by get_mdevice()

[Shuvam Pandey]

get_mdevice() wraps class_find_device(), which returns a device with a
reference held.

socket.c leaks those references in two places. IMGETDEVINFO and
IMSETDEVNAME never drop the temporary lookup reference, and the
references stored in _pms(sk)->dev by base_sock_bind() and
data_sock_bind() are never released when the socket is closed.

Drop the temporary references after the ioctl completes, and release the
stored device reference from the base and data socket release paths.

Fixes: b36b654a7e82 ("mISDN: Create /sys/class/mISDN")
Cc: stable@vger.kernel.org
Signed-off-by: Shuvam Pandey <shuvampandey1@gmail.com>
---
 drivers/isdn/mISDN/socket.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/isdn/mISDN/socket.c b/drivers/isdn/mISDN/socket.c
index 77b900db1ca..9209ee68f9c 100644
--- a/drivers/isdn/mISDN/socket.c
+++ b/drivers/isdn/mISDN/socket.c
@@ -266,6 +266,11 @@ data_sock_release(struct socket *sock)
 
 	lock_sock(sk);
 
+	if (_pms(sk)->dev) {
+		put_device(&_pms(sk)->dev->dev);
+		_pms(sk)->dev = NULL;
+	}
+
 	sock_orphan(sk);
 	skb_queue_purge(&sk->sk_receive_queue);
 
@@ -387,6 +392,7 @@ data_sock_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
 			strscpy(di.name, dev_name(&dev->dev), sizeof(di.name));
 			if (copy_to_user((void __user *)arg, &di, sizeof(di)))
 				err = -EFAULT;
+			put_device(&dev->dev);
 		} else
 			err = -ENODEV;
 		break;
@@ -623,6 +629,11 @@ base_sock_release(struct socket *sock)
 	if (!sk)
 		return 0;
 
+	if (_pms(sk)->dev) {
+		put_device(&_pms(sk)->dev->dev);
+		_pms(sk)->dev = NULL;
+	}
+
 	mISDN_sock_unlink(&base_sockets, sk);
 	sock_orphan(sk);
 	sock_put(sk);
@@ -670,6 +681,7 @@ base_sock_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
 			strscpy(di.name, dev_name(&dev->dev), sizeof(di.name));
 			if (copy_to_user((void __user *)arg, &di, sizeof(di)))
 				err = -EFAULT;
+			put_device(&dev->dev);
 		} else
 			err = -ENODEV;
 		break;
@@ -683,10 +695,12 @@ base_sock_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
 		}
 		dn.name[sizeof(dn.name) - 1] = '\0';
 		dev = get_mdevice(dn.id);
-		if (dev)
+		if (dev) {
 			err = device_rename(&dev->dev, dn.name);
-		else
+			put_device(&dev->dev);
+		} else {
 			err = -ENODEV;
+		}
 	}
 	break;
 	default:
-- 
2.50.1 (Apple Git-155)

Re: [PATCH] mISDN: socket: drop device references acquired by get_mdevice()

[Simon Horman]

From: 'Simon Horman' <horms@kernel.org>

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev
---
mISDN: socket: drop device references acquired by get_mdevice()

This patch attempts to fix a memory leak where device references acquired
by get_mdevice() are not dropped. It adds put_device() calls in the ioctl
completion paths and in the socket release paths.

> diff --git a/drivers/isdn/mISDN/socket.c b/drivers/isdn/mISDN/socket.c
> index 77b900db1cac2..9209ee68f9cda 100644
> --- a/drivers/isdn/mISDN/socket.c
> +++ b/drivers/isdn/mISDN/socket.c
> @@ -266,6 +266,11 @@ data_sock_release(struct socket *sock)
>  
>  	lock_sock(sk);
>  
> +	if (_pms(sk)->dev) {
> +		put_device(&_pms(sk)->dev->dev);
> +		_pms(sk)->dev = NULL;
> +	}
> +

Could this introduce a use-after-free regression if the underlying hardware
is removed before the socket is closed?

It appears the mISDN subsystem handles kobject lifetimes unconventionally.
Hardware drivers manually call kfree() on their private structures immediately
after calling mISDN_unregister_device(), without waiting for the kref to
reach zero.

If a socket holds a reference to a device when the module is unloaded, the
device memory is synchronously freed. When the socket is later closed, this
put_device() would attempt to decrement a kref inside an already freed driver
structure.

This isn't a regression, but is there a use-after-free in the stack teardown
when the hardware is removed?

When a socket is bound via data_sock_bind(), connect_layer1() stores a pointer
to the device's stack on the channel (ch->st = dev->D.st). If the hardware is
removed, mISDN_unregister_device() calls delete_stack(), which unconditionally
frees the stack pointer.

When the user eventually closes the socket, data_sock_release() executes:

    if (sk->sk_state == MISDN_BOUND)
        delete_channel(&_pms(sk)->ch);

And delete_channel() dereferences the dangling stack pointer:

    write_lock_bh(&ch->st->l1sock.lock);

This also isn't a regression, but can data_sock_release() allow an
unprivileged user to spam the kernel log?

If an unprivileged user creates a SOCK_DGRAM socket with protocol
ISDN_P_LAPD_TE and closes it without binding, data_sock_release() evaluates
the closure:

    case ISDN_P_LAPD_TE:
        delete_channel(&_pms(sk)->ch);

Since it was never bound, the channel's stack pointer remains NULL.
delete_channel() handles this by executing:

    if (!ch->st) {
        printk(KERN_WARNING "%s: no stack\n", __func__);
        return;
    }

Since there is no ratelimiting on this printk, could a user continuously create
and close these sockets to flood dmesg?

>  	sock_orphan(sk);
>  	skb_queue_purge(&sk->sk_receive_queue);
>  
> @@ -387,6 +392,7 @@ data_sock_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
>  			strscpy(di.name, dev_name(&dev->dev), sizeof(di.name));

This isn't a regression, but might this read from freed memory if a device
rename happens concurrently?

In IMGETDEVINFO, the device name is read via dev_name() without holding locks.
Concurrently, a privileged user can issue the IMSETDEVNAME ioctl, which
executes device_rename().

Since device_rename() updates the device's kobject name, allocating a new
string and calling kfree() on the old name string, it seems strscpy() could
read from the freed name string if executed at the same time.

>  			if (copy_to_user((void __user *)arg, &di, sizeof(di)))
>  				err = -EFAULT;
> +			put_device(&dev->dev);
>  		} else
>  			err = -ENODEV;
>  		break;

[ ... ]

Re: [PATCH] mISDN: socket: drop device references acquired by get_mdevice()

[Shuvam Pandey]

> Could this introduce a use-after-free regression if the underlying
> hardware is removed before the socket is closed?
>
> [...]
>
> This isn't a regression, but is there a use-after-free in the stack
> teardown when the hardware is removed?

Thanks for the review.

Looking at it more closely, I agree the close-path part of this patch is
not safe as posted.

While get_mdevice() does return a referenced device, the sockets also
keep raw mISDNdevice / mISDNstack pointers across bind, and
mISDN_unregister_device() tears the stack down while those pointers can
still be reached later from socket release. In addition, several mISDN
drivers free the enclosing allocation immediately after
mISDN_unregister_device() returns, so adding put_device() in the socket
release paths can turn this into a close-time UAF. The delete_channel()
/ ch->st case on the data-socket side is the same underlying lifetime
problem.

I'll drop this version and revisit it after reworking the unregister /
socket lifetime handling first. I also want to re-check whether the
temporary get_mdevice() lookup references should be fixed separately or
only once that lifetime side is addressed.

Thanks,
Shuvam

[PATCH v2 0/3] mISDN: fix socket/device lifetime and naming races

[Shuvam Pandey]

This is a respin of the original get_mdevice() reference leak fix.

Patch 1 makes unregister wait for the last device reference and closes
sockets that still point at the device before delete_stack() runs. It
also serializes bind against unregister so a new socket cannot attach
once teardown has started.

Patch 2 drops the temporary get_mdevice() references from IMGETDEVINFO
and IMSETDEVNAME, and serializes those ioctl paths against unregister.

Patch 3 keeps an mISDN-owned copy of the device name so ioctl, sysfs, and
debug paths no longer depend on the kobject name storage remaining stable
across device_rename().

Previous discussion:
https://lore.kernel.org/r/20260414071322.30851-1-shuvampandey1@gmail.com

This series was developed with AI assistance. I reviewed, revised, and
tested it, and I take responsibility for the submission.

---
Changes in v2:
- split the fix into three focused patches
- close sockets before delete_stack() and wait for the final device release
- serialize bind and ioctl lookup/rename paths against unregister
- cache stable device names for mISDN paths outside the kobject
- keep existing debug behavior while switching layer1 name reads to the cached name
- document the device_lock(dev) -> lock_sock(sk) ordering
- build-test the series on arm64 with W=1 for drivers/isdn/mISDN and
  drivers/isdn/hardware/mISDN

Shuvam Pandey (3):
  mISDN: serialize socket teardown against device unregister
  mISDN: socket: drop temporary references from get_mdevice()
  mISDN: cache stable device names outside the kobject

 drivers/isdn/mISDN/core.c   |  37 ++++-
 drivers/isdn/mISDN/core.h   |   1 +
 drivers/isdn/mISDN/layer1.c |   2 +-
 drivers/isdn/mISDN/socket.c | 263 +++++++++++++++++++++++++++++++-----
 drivers/isdn/mISDN/stack.c  |  36 ++---
 drivers/isdn/mISDN/tei.c    |   2 +-
 include/linux/mISDNif.h     |   2 +
 7 files changed, 284 insertions(+), 59 deletions(-)

-- 
2.50.1 (Apple Git-155)

[PATCH v2 2/3] mISDN: socket: drop temporary references from get_mdevice()

[Shuvam Pandey]

IMGETDEVINFO and IMSETDEVNAME only use get_mdevice() for a temporary
lookup, but neither path drops the reference returned by
class_find_device().

Drop the temporary reference once the ioctl finishes. Serialize the name
read and rename paths with device_lock() while doing so, and reject
lookups that raced with unregister after device_del().

Fixes: b36b654a7e82 ("mISDN: Create /sys/class/mISDN")
Cc: stable@vger.kernel.org
Assisted-by: Codex:GPT-5.3-Codex
Signed-off-by: Shuvam Pandey <shuvampandey1@gmail.com>
---
 drivers/isdn/mISDN/socket.c | 31 ++++++++++++++++++++++++++++---
 1 file changed, 28 insertions(+), 3 deletions(-)

diff --git a/drivers/isdn/mISDN/socket.c b/drivers/isdn/mISDN/socket.c
index bf3ad0a2a42bc..42cda5b8bbe16 100644
--- a/drivers/isdn/mISDN/socket.c
+++ b/drivers/isdn/mISDN/socket.c
@@ -484,6 +484,13 @@ data_sock_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
 		if (dev) {
 			struct mISDN_devinfo di;
 
+			device_lock(&dev->dev);
+			if (!device_is_registered(&dev->dev)) {
+				device_unlock(&dev->dev);
+				put_device(&dev->dev);
+				err = -ENODEV;
+				break;
+			}
 			memset(&di, 0, sizeof(di));
 			di.id = dev->id;
 			di.Dprotocols = dev->Dprotocols;
@@ -493,8 +500,10 @@ data_sock_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
 			       sizeof(di.channelmap));
 			di.nrbchan = dev->nrbchan;
 			strscpy(di.name, dev_name(&dev->dev), sizeof(di.name));
+			device_unlock(&dev->dev);
 			if (copy_to_user((void __user *)arg, &di, sizeof(di)))
 				err = -EFAULT;
+			put_device(&dev->dev);
 		} else
 			err = -ENODEV;
 		break;
@@ -802,6 +811,13 @@ base_sock_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
 		if (dev) {
 			struct mISDN_devinfo di;
 
+			device_lock(&dev->dev);
+			if (!device_is_registered(&dev->dev)) {
+				device_unlock(&dev->dev);
+				put_device(&dev->dev);
+				err = -ENODEV;
+				break;
+			}
 			memset(&di, 0, sizeof(di));
 			di.id = dev->id;
 			di.Dprotocols = dev->Dprotocols;
@@ -811,8 +827,10 @@ base_sock_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
 			       sizeof(di.channelmap));
 			di.nrbchan = dev->nrbchan;
 			strscpy(di.name, dev_name(&dev->dev), sizeof(di.name));
+			device_unlock(&dev->dev);
 			if (copy_to_user((void __user *)arg, &di, sizeof(di)))
 				err = -EFAULT;
+			put_device(&dev->dev);
 		} else
 			err = -ENODEV;
 		break;
@@ -826,10 +844,17 @@ base_sock_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
 		}
 		dn.name[sizeof(dn.name) - 1] = '\0';
 		dev = get_mdevice(dn.id);
-		if (dev)
-			err = device_rename(&dev->dev, dn.name);
-		else
+		if (dev) {
+			device_lock(&dev->dev);
+			if (!device_is_registered(&dev->dev))
+				err = -ENODEV;
+			else
+				err = device_rename(&dev->dev, dn.name);
+			device_unlock(&dev->dev);
+			put_device(&dev->dev);
+		} else {
 			err = -ENODEV;
+		}
 	}
 	break;
 	default:
-- 
2.50.1 (Apple Git-155)

[PATCH v2 3/3] mISDN: cache stable device names outside the kobject

[Shuvam Pandey]

mISDN prints and exports device names from several paths that are not tied
to the kobject rename internals. device_rename() replaces the kobject name
string, so reading it directly leaves mISDN dependent on that storage
remaining stable while those paths run.

Keep an mISDN-owned copy of the device name, update it on registration and
successful rename, and use that cached name from the socket, sysfs, stack,
and debug paths.

Assisted-by: Codex:GPT-5.3-Codex
Signed-off-by: Shuvam Pandey <shuvampandey1@gmail.com>
---
 drivers/isdn/mISDN/core.c   | 18 +++++++++++++-----
 drivers/isdn/mISDN/layer1.c |  2 +-
 drivers/isdn/mISDN/socket.c | 12 ++++++++----
 drivers/isdn/mISDN/stack.c  | 36 ++++++++++++++++++------------------
 drivers/isdn/mISDN/tei.c    |  2 +-
 include/linux/mISDNif.h     |  1 +
 6 files changed, 42 insertions(+), 29 deletions(-)

diff --git a/drivers/isdn/mISDN/core.c b/drivers/isdn/mISDN/core.c
index 4e2be8f03119b..d89c9e54cb5fa 100644
--- a/drivers/isdn/mISDN/core.c
+++ b/drivers/isdn/mISDN/core.c
@@ -89,8 +89,15 @@ static DEVICE_ATTR_RO(protocol);
 static ssize_t name_show(struct device *dev,
 			 struct device_attribute *attr, char *buf)
 {
-	strcpy(buf, dev_name(dev));
-	return strlen(buf);
+	struct mISDNdevice *mdev = dev_to_mISDN(dev);
+	ssize_t len;
+
+	if (!mdev)
+		return -ENODEV;
+	device_lock(dev);
+	len = sysfs_emit(buf, "%s", mdev->name);
+	device_unlock(dev);
+	return len;
 }
 static DEVICE_ATTR_RO(name);
 
@@ -227,9 +234,10 @@ mISDN_register_device(struct mISDNdevice *dev,
 		dev_set_name(&dev->dev, "%s", name);
 	else
 		dev_set_name(&dev->dev, "mISDN%d", dev->id);
+	strscpy(dev->name, dev_name(&dev->dev), sizeof(dev->name));
 	if (debug & DEBUG_CORE)
 		printk(KERN_DEBUG "mISDN_register %s %d\n",
-		       dev_name(&dev->dev), dev->id);
+		       dev->name, dev->id);
 	dev->dev.class = &mISDN_class;
 
 	err = create_stack(dev);
@@ -258,7 +266,7 @@ void
 mISDN_unregister_device(struct mISDNdevice *dev) {
 	if (debug & DEBUG_CORE)
 		printk(KERN_DEBUG "mISDN_unregister %s %d\n",
-		       dev_name(&dev->dev), dev->id);
+		       dev->name, dev->id);
 	/* sysfs_remove_link(&dev->dev.kobj, "device"); */
 	/*
 	 * Remove the device from sysfs before taking dev->mutex so bind-side
@@ -358,7 +366,7 @@ const char *mISDNDevName4ch(struct mISDNchannel *ch)
 		return msg_no_stack;
 	if (!ch->st->dev)
 		return msg_no_stackdev;
-	return dev_name(&ch->st->dev->dev);
+	return ch->st->dev->name;
 };
 EXPORT_SYMBOL(mISDNDevName4ch);
 
diff --git a/drivers/isdn/mISDN/layer1.c b/drivers/isdn/mISDN/layer1.c
index 3fbc170acf9ab..c5a2e9119e868 100644
--- a/drivers/isdn/mISDN/layer1.c
+++ b/drivers/isdn/mISDN/layer1.c
@@ -100,7 +100,7 @@ l1m_debug(struct FsmInst *fi, char *fmt, ...)
 	vaf.fmt = fmt;
 	vaf.va = &va;
 
-	printk(KERN_DEBUG "%s: %pV\n", dev_name(&l1->dch->dev.dev), &vaf);
+	printk(KERN_DEBUG "%s: %pV\n", l1->dch->dev.name, &vaf);
 
 	va_end(va);
 }
diff --git a/drivers/isdn/mISDN/socket.c b/drivers/isdn/mISDN/socket.c
index 42cda5b8bbe16..bce71ae5eb7d4 100644
--- a/drivers/isdn/mISDN/socket.c
+++ b/drivers/isdn/mISDN/socket.c
@@ -499,7 +499,7 @@ data_sock_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
 			memcpy(di.channelmap, dev->channelmap,
 			       sizeof(di.channelmap));
 			di.nrbchan = dev->nrbchan;
-			strscpy(di.name, dev_name(&dev->dev), sizeof(di.name));
+			strscpy(di.name, dev->name, sizeof(di.name));
 			device_unlock(&dev->dev);
 			if (copy_to_user((void __user *)arg, &di, sizeof(di)))
 				err = -EFAULT;
@@ -826,7 +826,7 @@ base_sock_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
 			memcpy(di.channelmap, dev->channelmap,
 			       sizeof(di.channelmap));
 			di.nrbchan = dev->nrbchan;
-			strscpy(di.name, dev_name(&dev->dev), sizeof(di.name));
+			strscpy(di.name, dev->name, sizeof(di.name));
 			device_unlock(&dev->dev);
 			if (copy_to_user((void __user *)arg, &di, sizeof(di)))
 				err = -EFAULT;
@@ -846,10 +846,14 @@ base_sock_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
 		dev = get_mdevice(dn.id);
 		if (dev) {
 			device_lock(&dev->dev);
-			if (!device_is_registered(&dev->dev))
+			if (!device_is_registered(&dev->dev)) {
 				err = -ENODEV;
-			else
+			} else {
 				err = device_rename(&dev->dev, dn.name);
+				if (!err)
+					strscpy(dev->name, dev_name(&dev->dev),
+						sizeof(dev->name));
+			}
 			device_unlock(&dev->dev);
 			put_device(&dev->dev);
 		} else {
diff --git a/drivers/isdn/mISDN/stack.c b/drivers/isdn/mISDN/stack.c
index 4e96684af0aac..9b39ddb4a0944 100644
--- a/drivers/isdn/mISDN/stack.c
+++ b/drivers/isdn/mISDN/stack.c
@@ -165,7 +165,7 @@ send_msg_to_layer(struct mISDNstack *st, struct sk_buff *skb)
 		else
 			printk(KERN_WARNING
 			       "%s: dev(%s) prim(%x) id(%x) no channel\n",
-			       __func__, dev_name(&st->dev->dev), hh->prim,
+			       __func__, st->dev->name, hh->prim,
 			       hh->id);
 	} else if (lm == 0x8) {
 		WARN_ON(lm == 0x8);
@@ -175,12 +175,12 @@ send_msg_to_layer(struct mISDNstack *st, struct sk_buff *skb)
 		else
 			printk(KERN_WARNING
 			       "%s: dev(%s) prim(%x) id(%x) no channel\n",
-			       __func__, dev_name(&st->dev->dev), hh->prim,
+			       __func__, st->dev->name, hh->prim,
 			       hh->id);
 	} else {
 		/* broadcast not handled yet */
 		printk(KERN_WARNING "%s: dev(%s) prim %x not delivered\n",
-		       __func__, dev_name(&st->dev->dev), hh->prim);
+		       __func__, st->dev->name, hh->prim);
 	}
 	return -ESRCH;
 }
@@ -202,7 +202,7 @@ mISDNStackd(void *data)
 	sigfillset(&current->blocked);
 	if (*debug & DEBUG_MSG_THREAD)
 		printk(KERN_DEBUG "mISDNStackd %s started\n",
-		       dev_name(&st->dev->dev));
+		       st->dev->name);
 
 	if (st->notify != NULL) {
 		complete(st->notify);
@@ -238,7 +238,7 @@ mISDNStackd(void *data)
 					printk(KERN_DEBUG
 					       "%s: %s prim(%x) id(%x) "
 					       "send call(%d)\n",
-					       __func__, dev_name(&st->dev->dev),
+					       __func__, st->dev->name,
 					       mISDN_HEAD_PRIM(skb),
 					       mISDN_HEAD_ID(skb), err);
 				dev_kfree_skb(skb);
@@ -281,7 +281,7 @@ mISDNStackd(void *data)
 						     mISDN_STACK_ACTION_MASK));
 		if (*debug & DEBUG_MSG_THREAD)
 			printk(KERN_DEBUG "%s: %s wake status %08lx\n",
-			       __func__, dev_name(&st->dev->dev), st->status);
+			       __func__, st->dev->name, st->status);
 		test_and_set_bit(mISDN_STACK_ACTIVE, &st->status);
 
 		test_and_clear_bit(mISDN_STACK_WAKEUP, &st->status);
@@ -296,17 +296,17 @@ mISDNStackd(void *data)
 #ifdef MISDN_MSG_STATS
 	printk(KERN_DEBUG "mISDNStackd daemon for %s proceed %d "
 	       "msg %d sleep %d stopped\n",
-	       dev_name(&st->dev->dev), st->msg_cnt, st->sleep_cnt,
+	       st->dev->name, st->msg_cnt, st->sleep_cnt,
 	       st->stopped_cnt);
 	task_cputime(st->thread, &utime, &stime);
 	printk(KERN_DEBUG
 	       "mISDNStackd daemon for %s utime(%llu) stime(%llu)\n",
-	       dev_name(&st->dev->dev), utime, stime);
+	       st->dev->name, utime, stime);
 	printk(KERN_DEBUG
 	       "mISDNStackd daemon for %s nvcsw(%ld) nivcsw(%ld)\n",
-	       dev_name(&st->dev->dev), st->thread->nvcsw, st->thread->nivcsw);
+	       st->dev->name, st->thread->nvcsw, st->thread->nivcsw);
 	printk(KERN_DEBUG "mISDNStackd daemon for %s killed now\n",
-	       dev_name(&st->dev->dev));
+	       st->dev->name);
 #endif
 	test_and_set_bit(mISDN_STACK_KILLED, &st->status);
 	test_and_clear_bit(mISDN_STACK_RUNNING, &st->status);
@@ -397,15 +397,15 @@ create_stack(struct mISDNdevice *dev)
 	newst->own.recv = mISDN_queue_message;
 	if (*debug & DEBUG_CORE_FUNC)
 		printk(KERN_DEBUG "%s: st(%s)\n", __func__,
-		       dev_name(&newst->dev->dev));
+		       newst->dev->name);
 	newst->notify = &done;
 	newst->thread = kthread_run(mISDNStackd, (void *)newst, "mISDN_%s",
-				    dev_name(&newst->dev->dev));
+				    newst->dev->name);
 	if (IS_ERR(newst->thread)) {
 		err = PTR_ERR(newst->thread);
 		printk(KERN_ERR
 		       "mISDN:cannot create kernel thread for %s (%d)\n",
-		       dev_name(&newst->dev->dev), err);
+		       newst->dev->name, err);
 		delete_teimanager(dev->teimgr);
 		kfree(newst);
 	} else
@@ -424,7 +424,7 @@ connect_layer1(struct mISDNdevice *dev, struct mISDNchannel *ch,
 
 	if (*debug &  DEBUG_CORE_FUNC)
 		printk(KERN_DEBUG "%s: %s proto(%x) adr(%d %d %d %d)\n",
-		       __func__, dev_name(&dev->dev), protocol, adr->dev,
+		       __func__, dev->name, protocol, adr->dev,
 		       adr->channel, adr->sapi, adr->tei);
 	switch (protocol) {
 	case ISDN_P_NT_S0:
@@ -461,7 +461,7 @@ connect_Bstack(struct mISDNdevice *dev, struct mISDNchannel *ch,
 
 	if (*debug &  DEBUG_CORE_FUNC)
 		printk(KERN_DEBUG "%s: %s proto(%x) adr(%d %d %d %d)\n",
-		       __func__, dev_name(&dev->dev), protocol,
+		       __func__, dev->name, protocol,
 		       adr->dev, adr->channel, adr->sapi,
 		       adr->tei);
 	ch->st = dev->D.st;
@@ -517,7 +517,7 @@ create_l2entity(struct mISDNdevice *dev, struct mISDNchannel *ch,
 
 	if (*debug &  DEBUG_CORE_FUNC)
 		printk(KERN_DEBUG "%s: %s proto(%x) adr(%d %d %d %d)\n",
-		       __func__, dev_name(&dev->dev), protocol,
+		       __func__, dev->name, protocol,
 		       adr->dev, adr->channel, adr->sapi,
 		       adr->tei);
 	rq.protocol = ISDN_P_TE_S0;
@@ -570,7 +570,7 @@ delete_channel(struct mISDNchannel *ch)
 	}
 	if (*debug & DEBUG_CORE_FUNC)
 		printk(KERN_DEBUG "%s: st(%s) protocol(%x)\n", __func__,
-		       dev_name(&ch->st->dev->dev), ch->protocol);
+		       ch->st->dev->name, ch->protocol);
 	if (ch->protocol >= ISDN_P_B_START) {
 		if (ch->peer) {
 			ch->peer->ctrl(ch->peer, CLOSE_CHANNEL, NULL);
@@ -623,7 +623,7 @@ delete_stack(struct mISDNdevice *dev)
 
 	if (*debug & DEBUG_CORE_FUNC)
 		printk(KERN_DEBUG "%s: st(%s)\n", __func__,
-		       dev_name(&st->dev->dev));
+		       st->dev->name);
 	if (dev->teimgr)
 		delete_teimanager(dev->teimgr);
 	if (st->thread) {
diff --git a/drivers/isdn/mISDN/tei.c b/drivers/isdn/mISDN/tei.c
index 2bad3083be901..876c1194920ef 100644
--- a/drivers/isdn/mISDN/tei.c
+++ b/drivers/isdn/mISDN/tei.c
@@ -990,7 +990,7 @@ create_teimgr(struct manager *mgr, struct channel_req *crq)
 
 	if (*debug & DEBUG_L2_TEI)
 		printk(KERN_DEBUG "%s: %s proto(%x) adr(%d %d %d %d)\n",
-		       __func__, dev_name(&mgr->ch.st->dev->dev),
+		       __func__, mgr->ch.st->dev->name,
 		       crq->protocol, crq->adr.dev, crq->adr.channel,
 		       crq->adr.sapi, crq->adr.tei);
 	if (crq->adr.tei > GROUP_TEI)
diff --git a/include/linux/mISDNif.h b/include/linux/mISDNif.h
index ce26d70c1ebfb..79f6d8f218b13 100644
--- a/include/linux/mISDNif.h
+++ b/include/linux/mISDNif.h
@@ -497,6 +497,7 @@ struct mISDNdevice {
 	u_int			Bprotocols;
 	u_int			nrbchan;
 	u_char			channelmap[MISDN_CHMAP_SIZE];
+	char			name[MISDN_MAX_IDLEN];
 	struct list_head	bchannels;
 	struct mISDNchannel	*teimgr;
 	struct completion	released;
-- 
2.50.1 (Apple Git-155)

[PATCH v2 1/3] mISDN: serialize socket teardown against device unregister

[Shuvam Pandey]

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 <shuvampandey1@gmail.com>
---
 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 <linux/mISDNif.h>
+#include <linux/device.h>
 #include <linux/slab.h>
 #include <linux/export.h>
 #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)