Bug 269190 - bugs in stat(1) and devname(3), wrong report on disk names
Summary: bugs in stat(1) and devname(3), wrong report on disk names
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: 13.1-STABLE
Hardware: Any Any
: --- Affects Many People
Assignee: freebsd-bugs (Nobody)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-01-27 20:51 UTC by alexander naumochkin
Modified: 2023-02-03 00:27 UTC (History)
2 users (show)

See Also:


Attachments
patches lib/libc/gen/devname.c and usr.bin/stat/stat.c (1.58 KB, text/plain)
2023-01-27 20:51 UTC, alexander naumochkin
no flags Details
patches lib/libc/gen/devname.c and usr.bin/stat/stat.c (1.62 KB, patch)
2023-01-28 16:09 UTC, alexander naumochkin
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description alexander naumochkin 2023-01-27 20:51:53 UTC
Created attachment 239747 [details]
patches lib/libc/gen/devname.c and usr.bin/stat/stat.c

If I want to know the name of the partition a file is on I run stat(1) with the -f "%Sd %Sr" option and it always reports #NODEV #NODEV. I checked out the stat(1) source code and found that it always calls devname(3) for dev_t with S_ISBLK mode, which is gone since FreeBSD 6.x AFAIR.

Also during my investigation I figured out that devname(3) has 10 years old bug: when it tries to check that the `type' is one of S_IFCHR or S_IFBLK, it applies S_ISBLK macro to `dev' instead of `type'.


patches attached
Comment 1 Konstantin Belousov freebsd_committer freebsd_triage 2023-01-28 13:38:05 UTC
WRT the stat(1) patch, why do you _not_ pass S_IFBLK when the device is the
block device?  IMO it is more correct to do the following:

 #if HAVE_DEVNAME
-    sdata = (what == SHOW_st_dev) ?
-        devname(st->st_dev, S_IFBLK) :
-        devname(st->st_rdev, 
-        S_ISCHR(st->st_mode) ? S_IFCHR :
-        S_ISBLK(st->st_mode) ? S_IFBLK :
-        0U);
-    if (sdata == NULL)
-       sdata = "???";
+     sdata = devname(what == SHOW_st_dev ? st->st_dev :
+         st->st_rdev, S_ISCHR(st->st_mode) ? S_IFCHR :
+         (S_ISBLK(st->st_mode) ? S_IFBLK : 0));
 #endif /* HAVE_DEVNAME */
Comment 2 alexander naumochkin 2023-01-28 16:06:06 UTC
In order to generalize the code it should be passed, you're right Konstantin.

Although it has no mean on FreeBSD, right?



patchpack.shar updated
Comment 3 alexander naumochkin 2023-01-28 16:09:49 UTC
Created attachment 239765 [details]
patches lib/libc/gen/devname.c and usr.bin/stat/stat.c

patch for stat.c updated with @kib's advices
Comment 4 Konstantin Belousov freebsd_committer freebsd_triage 2023-01-28 17:19:34 UTC
(In reply to alexander naumochkin from comment #2)
FreeBSD still could have block special nodes, e.g. on UFS, but not on devfs.
They do not function as special nodes, same as character devices on UFS are
non-functional.
Comment 5 commit-hook freebsd_committer freebsd_triage 2023-01-28 18:05:19 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=0311fe403d35f6241fa94104c4b92c9d73214cbc

commit 0311fe403d35f6241fa94104c4b92c9d73214cbc
Author:     Alexander Naumochkin <alexander.naumochkin@gmail.com>
AuthorDate: 2023-01-28 13:22:50 +0000
Commit:     Konstantin Belousov <kib@FreeBSD.org>
CommitDate: 2023-01-28 18:04:35 +0000

    devname(3): apply S_ISBLK() to type, not dev_t

    PR:     269190
    Reviewed by:    kib
    MFC after:      1 week

 lib/libc/gen/devname.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Comment 6 commit-hook freebsd_committer freebsd_triage 2023-01-28 18:05:21 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=050e4bca94e907c046e9ed63f90b5dd0e49fb405

commit 050e4bca94e907c046e9ed63f90b5dd0e49fb405
Author:     Alexander Naumochkin <alexander.naumochkin@gmail.com>
AuthorDate: 2023-01-28 13:31:39 +0000
Commit:     Konstantin Belousov <kib@FreeBSD.org>
CommitDate: 2023-01-28 18:04:43 +0000

    stat(1): Do not pass S_IFBLK to devname unconditionally

    PR:     269190
    Reviewed by:    kib
    MFC after:      1 week

 usr.bin/stat/stat.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)
Comment 7 commit-hook freebsd_committer freebsd_triage 2023-02-03 00:27:38 UTC
A commit in branch stable/13 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=75aa61400cba6a67ff0ad392d8a5cc2d3855bd3b

commit 75aa61400cba6a67ff0ad392d8a5cc2d3855bd3b
Author:     Alexander Naumochkin <alexander.naumochkin@gmail.com>
AuthorDate: 2023-01-28 13:31:39 +0000
Commit:     Konstantin Belousov <kib@FreeBSD.org>
CommitDate: 2023-02-03 00:26:12 +0000

    stat(1): Do not pass S_IFBLK to devname unconditionally

    PR:     269190

    (cherry picked from commit 050e4bca94e907c046e9ed63f90b5dd0e49fb405)

 usr.bin/stat/stat.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)
Comment 8 commit-hook freebsd_committer freebsd_triage 2023-02-03 00:27:40 UTC
A commit in branch stable/13 references this bug:

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

commit dfd1dea884d771f763f1a40efe8b018ffcc04897
Author:     Alexander Naumochkin <alexander.naumochkin@gmail.com>
AuthorDate: 2023-01-28 13:22:50 +0000
Commit:     Konstantin Belousov <kib@FreeBSD.org>
CommitDate: 2023-02-03 00:26:12 +0000

    devname(3): apply S_ISBLK() to type, not dev_t

    PR:     269190

    (cherry picked from commit 0311fe403d35f6241fa94104c4b92c9d73214cbc)

 lib/libc/gen/devname.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)