Bug 266862 - devctl getpath cause panic invoking with root0
Summary: devctl getpath cause panic invoking with root0
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: Any Any
: Normal Affects Many People
Assignee: Warner Losh
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-10-06 06:17 UTC by Takanori Watanabe
Modified: 2022-10-20 04:41 UTC (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Takanori Watanabe freebsd_committer freebsd_triage 2022-10-06 06:17:48 UTC
Invoking 
% devctl devpath FreeBSD root0
cause kernel panic

This may because parent of "root0" device is NULL and invoking 
BUS_GET_DEVICE_PATH(device_get_parent(dev), dev, locator, sb);
references NULL pointer.
So check before invoking it.

Following code is not tested yet.

diff --git a/sys/kern/subr_bus.c b/sys/kern/subr_bus.c
index 041e77259313..92447f825643 100644
--- a/sys/kern/subr_bus.c
+++ b/sys/kern/subr_bus.c
@@ -5310,9 +5310,13 @@ device_get_path(device_t dev, const char *locator)
        ssize_t len;
        char *rv = NULL;
        int error;
+       device_t parent = device_get_parent(dev);
+
+       if(parent == NULL)
+               return EINVAL;
 
        sb = sbuf_new(NULL, NULL, 0, SBUF_AUTOEXTEND | SBUF_INCLUDENUL);
-       error = BUS_GET_DEVICE_PATH(device_get_parent(dev), dev, locator, sb);
+       error = BUS_GET_DEVICE_PATH(parent, dev, locator, sb);
        sbuf_finish(sb);        /* Note: errors checked with sbuf_len() below */
        if (error != 0)
                goto out;
Comment 1 Takanori Watanabe freebsd_committer freebsd_triage 2022-10-06 10:49:25 UTC
The code was not be compiled. Correct code is as follows. 
With this patch, 
% devctl devpath FreeBSD root0
does not cause panic and show error message, as I expected.
devctl: Failed to get path via FreeBSD to root0: Cannot allocate memory


diff --git a/sys/kern/subr_bus.c b/sys/kern/subr_bus.c
index 487e573451ed..041e77259313 100644
--- a/sys/kern/subr_bus.c
+++ b/sys/kern/subr_bus.c
@@ -5310,13 +5310,9 @@ device_get_path(device_t dev, const char *locator)
        ssize_t len;
        char *rv = NULL;
        int error;
-       device_t parent = device_get_parent(dev);
-
-       if(parent == NULL)
-               return rv;
 
        sb = sbuf_new(NULL, NULL, 0, SBUF_AUTOEXTEND | SBUF_INCLUDENUL);
-       error = BUS_GET_DEVICE_PATH(parent, dev, locator, sb);
+       error = BUS_GET_DEVICE_PATH(device_get_parent(dev), dev, locator, sb);
        sbuf_finish(sb);        /* Note: errors checked with sbuf_len() below */
        if (error != 0)
                goto out;
Comment 2 Takanori Watanabe freebsd_committer freebsd_triage 2022-10-06 10:50:07 UTC
(In reply to Takanori Watanabe from comment #1)
reverse patch ...
diff --git a/sys/kern/subr_bus.c b/sys/kern/subr_bus.c
index 041e77259313..487e573451ed 100644
--- a/sys/kern/subr_bus.c
+++ b/sys/kern/subr_bus.c
@@ -5310,9 +5310,13 @@ device_get_path(device_t dev, const char *locator)
        ssize_t len;
        char *rv = NULL;
        int error;
+       device_t parent = device_get_parent(dev);
+
+       if(parent == NULL)
+               return rv;
 
        sb = sbuf_new(NULL, NULL, 0, SBUF_AUTOEXTEND | SBUF_INCLUDENUL);
-       error = BUS_GET_DEVICE_PATH(device_get_parent(dev), dev, locator, sb);
+       error = BUS_GET_DEVICE_PATH(parent, dev, locator, sb);
        sbuf_finish(sb);        /* Note: errors checked with sbuf_len() below */
        if (error != 0)
                goto out;
Comment 3 Konstantin Belousov freebsd_committer freebsd_triage 2022-10-07 01:34:00 UTC
This is mostluy right, but I do not see why should we avoid handling nexus
in the DEV_GET_PATH.  Also there is a mess with the error propagation, so
we get ENOMEM in the situation.

I believe I wrote the proper patch, please see
https://reviews.freebsd.org/D36899
Comment 4 commit-hook freebsd_committer freebsd_triage 2022-10-19 16:40:16 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=8cf783bde35352eb6105cefa9fcb586c01b77179

commit 8cf783bde35352eb6105cefa9fcb586c01b77179
Author:     Konstantin Belousov <kib@FreeBSD.org>
AuthorDate: 2022-10-06 19:46:30 +0000
Commit:     Konstantin Belousov <kib@FreeBSD.org>
CommitDate: 2022-10-19 16:39:33 +0000

    device_get_path(): handle case when dev is root

    PR:     266862
    Based on submission by: takawata
    Reviewed by:    jhb, takawata
    Disscussed with:        imp
    Sponsored by:   The FreeBSD Foundation
    MFC after:      1 week
    Differential revision:  https://reviews.freebsd.org/D36899

 sys/kern/subr_bus.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)