Bug #1866

[PATCH] ucom - destroy device node on detach

Added by qhwt.dfly almost 4 years ago. Updated almost 4 years ago.

Status:ClosedStart date:
Priority:NormalDue date:
Assignee:qhwt.dfly% Done:

0%

Category:-
Target version:-

Description

Issuing `ls /dev' after unloading the driver triggered a panic because
of the stale device node.
---
sys/dev/usbmisc/ucom/ucom.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/sys/dev/usbmisc/ucom/ucom.c b/sys/dev/usbmisc/ucom/ucom.c
index f85c3b7..a0a7260 100644
--- a/sys/dev/usbmisc/ucom/ucom.c
+++ b/sys/dev/usbmisc/ucom/ucom.c
@@ -177,7 +177,7 @@ ucom_attach(struct ucom_softc *sc)

DPRINTF(("ucom_attach: make_dev: ucom%d\n", unit));

- dev = make_dev(&ucom_ops, unit | UCOM_CALLOUT_MASK,
+ sc->dev = dev = make_dev(&ucom_ops, unit | UCOM_CALLOUT_MASK,
UID_UUCP, GID_DIALER, 0660,
"ucom%d", unit);
dev->si_tty = tp;
@@ -194,6 +194,7 @@ ucom_detach(struct ucom_softc *sc)

DPRINTF(("ucom_detach: sc = %p, tp = %p\n", sc, sc->sc_tty));

+ destroy_dev(sc->dev);
lwkt_gettoken(&tty_token);
sc->sc_dying = 1;

History

#1 Updated by eocallaghan almost 4 years ago

Hey,

Just for readability, I think it would be better not to have two = on the same
line here.

Cheers,
Edward.

#2 Updated by qhwt.dfly almost 4 years ago

> Hey,

Hi,

> Just for readability, I think it would be better not to have two = on the same
> line here.

Well, it's common in C for assigning the same value to multiple variables
that way, but I don't insist on that point here :)
I'm going to commit this in a few days if no objections raised.

Cheers.

diff --git a/sys/dev/usbmisc/ucom/ucom.c b/sys/dev/usbmisc/ucom/ucom.c
index f85c3b7..66ccc53 100644
--- a/sys/dev/usbmisc/ucom/ucom.c
+++ b/sys/dev/usbmisc/ucom/ucom.c
@@ -181,6 +181,7 @@ ucom_attach(struct ucom_softc *sc)
UID_UUCP, GID_DIALER, 0660,
"ucom%d", unit);
dev->si_tty = tp;
+ sc->dev = dev;
lwkt_reltoken(&tty_token);

return (0);
@@ -194,6 +195,7 @@ ucom_detach(struct ucom_softc *sc)

DPRINTF(("ucom_detach: sc = %p, tp = %p\n", sc, sc->sc_tty));

+ destroy_dev(sc->dev);
lwkt_gettoken(&tty_token);
sc->sc_dying = 1;

#3 Updated by alexh almost 4 years ago

Patch looks good to me, but you could move the destroy_dev() out of
under the tty_token; the tty_token in drivers is mainly intended to
serialize the tty structure access and such.

Cheers,
Alex

On 07/10/10 03:27, YONETANI Tomokazu wrote:
> Issuing `ls /dev' after unloading the driver triggered a panic because
> of the stale device node.
> ---
> sys/dev/usbmisc/ucom/ucom.c | 3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/sys/dev/usbmisc/ucom/ucom.c b/sys/dev/usbmisc/ucom/ucom.c
> index f85c3b7..a0a7260 100644
> --- a/sys/dev/usbmisc/ucom/ucom.c
> +++ b/sys/dev/usbmisc/ucom/ucom.c
> @@ -177,7 +177,7 @@ ucom_attach(struct ucom_softc *sc)
>
> DPRINTF(("ucom_attach: make_dev: ucom%d\n", unit));
>
> - dev = make_dev(&ucom_ops, unit | UCOM_CALLOUT_MASK,
> + sc->dev = dev = make_dev(&ucom_ops, unit | UCOM_CALLOUT_MASK,
> UID_UUCP, GID_DIALER, 0660,
> "ucom%d", unit);
> dev->si_tty = tp;
> @@ -194,6 +194,7 @@ ucom_detach(struct ucom_softc *sc)
>
> DPRINTF(("ucom_detach: sc = %p, tp = %p\n", sc, sc->sc_tty));
>
> + destroy_dev(sc->dev);
> lwkt_gettoken(&tty_token);
> sc->sc_dying = 1;
>

#4 Updated by alexh almost 4 years ago

Disregard my comment... I read 'reltoken' instead of 'gettoken'. It is
already outside the token-protected section.

#5 Updated by qhwt+dfly almost 4 years ago

committed as f2e9314ab6dfc3209d1c09aa31927aa4cc2729c3

Also available in: Atom PDF