NetBSD Problem Report #57856

From www@netbsd.org  Sun Jan 14 18:22:38 2024
Return-Path: <www@netbsd.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits))
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id D1E551A9238
	for <gnats-bugs@gnats.NetBSD.org>; Sun, 14 Jan 2024 18:22:38 +0000 (UTC)
Message-Id: <20240114182237.389BF1A9239@mollari.NetBSD.org>
Date: Sun, 14 Jan 2024 18:22:37 +0000 (UTC)
From: code-netbsd@yoyomail.de
Reply-To: code-netbsd@yoyomail.de
To: gnats-bugs@NetBSD.org
Subject: blkdiscard abort()s when not supplying a partition name
X-Send-Pr-Version: www-1.0

>Number:         57856
>Category:       bin
>Synopsis:       blkdiscard abort()s when not supplying a partition name
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    mrg
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sun Jan 14 18:25:00 +0000 2024
>Closed-Date:    Sat Feb 03 20:38:18 +0000 2024
>Last-Modified:  Sat Feb 03 20:38:18 +0000 2024
>Originator:     M. Boerschig
>Release:        NetBSD 10.0_RC2
>Organization:
>Environment:
NetBSD 10.0_RC2 (GENERIC) amd64
>Description:
Not sure if this is worth a PR, but blkdiscard abort()ed on me during sysinstall. I tried to blkdiscard my nvme (like I'm used to on Linux) before install. However, it took some experimenting to get the arguments right and it aborted.

The patch attached removes the assert() and replaces it with a error message -- not sure if this is helpful. I just don't like Release software to dump core on user input...

Also, discard seems to be not supported on NVME, but I'll have a look separately.
>How-To-Repeat:
$ blkdiscard /dev/ld0
assertion "partchar >= 'a' && partchar <= 'p'" failed: file "/usr/src/sbin/blkdiscard/blkdiscard.c", line 190, function "main"
Abort (core dumped)
>Fix:
diff --git a/sbin/blkdiscard/blkdiscard.c b/sbin/blkdiscard/blkdiscard.c
index 7bccd3f6e128..886181f7b993 100644
--- a/sbin/blkdiscard/blkdiscard.c
+++ b/sbin/blkdiscard/blkdiscard.c
@@ -45,7 +45,6 @@
 #include <unistd.h>
 #include <err.h>
 #include <errno.h>
-#include <assert.h>
 #include <stdbool.h>

 static bool secure = false;
@@ -187,7 +186,10 @@ main(int argc, char *argv[])
                struct disklabel dl;
                if (ioctl(fd, DIOCGDINFO, &dl) != -1) {
                        char partchar = name[strlen(name)-1];
-                       assert(partchar >= 'a' && partchar <= 'p');
+                       if(!(partchar >= 'a' && partchar <= 'p')) {
+                               errx(1, "%s: the partition suffix '%c' is not valid. "
+                                        "Expected range is [a-p].", name, partchar);
+                       }
                        int part = partchar - 'a';

                        size = (uint64_t)dl.d_partitions[part].p_size *

>Release-Note:

>Audit-Trail:
From: matthew green <mrg@eterna23.net>
To: gnats-bugs@netbsd.org, code-netbsd@yoyomail.de
Cc: gnats-admin@netbsd.org, netbsd-bugs@netbsd.org
Subject: re: bin/57856: blkdiscard abort()s when not supplying a partition name
Date: Mon, 15 Jan 2024 06:14:47 +1100

 hi.


 thanks for the PR.  i agree having easy to assert() code like this
 is bad, and i'll fix it.

 can you tell me what actual command you used that triggered this?
 i wonder if the code needs to understand that eg "wd0" means the
 raw partition of "wd0", since /dev/wd0 is now a valid device name
 for the same, but back when i originally wrote this code i don't
 think it was yet common or on my systems yet.

 eg, instead of making this an error, perhaps it should assume
 that kern.rawpartition is the partition, but first let's confirm
 what actual args you used that triggered the problem.

 thanks.


 .mrg.

Responsible-Changed-From-To: bin-bug-people->mrg
Responsible-Changed-By: mrg@NetBSD.org
Responsible-Changed-When: Mon, 15 Jan 2024 00:04:31 +0000
Responsible-Changed-Why:
i have a change in testing that should allow the attempted usage while
also still working properly.


From: "matthew green" <mrg@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/57856 CVS commit: src/sbin/blkdiscard
Date: Thu, 25 Jan 2024 02:42:18 +0000

 Module Name:	src
 Committed By:	mrg
 Date:		Thu Jan 25 02:42:17 UTC 2024

 Modified Files:
 	src/sbin/blkdiscard: blkdiscard.8 blkdiscard.c

 Log Message:
 blkdiscard: avoid asserting when passed a bsd disklabel raw device

 PR#57856 shows when using blkdiscard on eg, /dev/ld0 it asserts because
 '0' is not between 'a' and 'p'.  switch this to using DISKPART() on the
 returned st_rdev, so it works on 'ld0c' or 'ld0' (rawpart=2.)


 To generate a diff of this commit:
 cvs rdiff -u -r1.2 -r1.3 src/sbin/blkdiscard/blkdiscard.8 \
     src/sbin/blkdiscard/blkdiscard.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: "Martin Husemann" <martin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/57856 CVS commit: [netbsd-10] src/sbin/blkdiscard
Date: Sat, 3 Feb 2024 12:09:07 +0000

 Module Name:	src
 Committed By:	martin
 Date:		Sat Feb  3 12:09:07 UTC 2024

 Modified Files:
 	src/sbin/blkdiscard [netbsd-10]: blkdiscard.8 blkdiscard.c

 Log Message:
 Pull up following revision(s) (requested by mrg in ticket #565):

 	sbin/blkdiscard/blkdiscard.c: revision 1.2
 	sbin/blkdiscard/blkdiscard.c: revision 1.3
 	sbin/blkdiscard/blkdiscard.8: revision 1.3

 determine the tty width instead off writing 100 chars before a new line.

 blkdiscard: avoid asserting when passed a bsd disklabel raw device

 PR#57856 shows when using blkdiscard on eg, /dev/ld0 it asserts because
 '0' is not between 'a' and 'p'.  switch this to using DISKPART() on the
 returned st_rdev, so it works on 'ld0c' or 'ld0' (rawpart=2.)


 To generate a diff of this commit:
 cvs rdiff -u -r1.2 -r1.2.2.1 src/sbin/blkdiscard/blkdiscard.8
 cvs rdiff -u -r1.1 -r1.1.2.1 src/sbin/blkdiscard/blkdiscard.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: open->closed
State-Changed-By: mrg@NetBSD.org
State-Changed-When: Sat, 03 Feb 2024 20:38:18 +0000
State-Changed-Why:
fixed and pulled up to netbsd-10.  thanks for the report!


>Unformatted:

Home	PR Database Search