On Tue, Apr 30, 2024 at 12:03:04PM GMT, Alexander Klimov wrote:
> Hello everyone!
> 
> Actually I was working on a way to create a degraded RAID.
> As the ioctl create RAID syscall takes a list of dev_t,
> I tried NODEV for Not yet Online DEVice. ;-)
> I expected the kernel to complain. But instead it crashed.

This is not a bug report, please follow https://www.openbsd.org/report.html

> 
> How to reproduce:
> 
> 1) Apply the diff below.
> 2) Build (just) sbin/bioctl.
> 3) Run: bioctl -c 1 -l XdYZ,OFFLINE softraid0 # XdYZ at your choice
> 4) The system crashes.

Your diffs are mangled and don't apply.
I cannot reproduce a panic inside a fresh snaphot install with either diff:

        OpenBSD 7.5-current (GENERIC) #35: Sun Apr 28 08:53:53 MDT 2024
            dera...@amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/GENERIC

for d in a b; do
        vmctl create -s 100m $d.img
        vnd=$(vnconfig $d.img)
        echo 'raid *' | disklabel -wAT- $vnd
done
./obj/bioctl -c1 -lvnd0a,vnd1a softraid0
bioctl -d sd1
./obj/bioctl -c1 -lvnd0a,OFFLINE softraid0
softraid0: trying to bring up sd1 degraded
sd1 at scsibus3 targ 1 lun 0: <OPENBSD, SR RAID 1, 006>
sd1: 99MB, 512 bytes/sector, 204272 sectors
softraid0: trying to bring up sd1 degraded
softraid0: RAID 1 volume attached as sd1

> 
> 
> Short version:
> 
> 
> --- sbin/bioctl/bioctl.c.old  Fri Apr 26 07:45:28 2024
> +++ sbin/bioctl/bioctl.c      Tue Jan  2 00:14:59 2024
> @@ -1015,16 +1026,20 @@
>                       /* got one */
>                       sz = e - s + 1;
>                       strlcpy(dev, s, sz + 1);
> -                     fd = opendev(dev, O_RDONLY, OPENDEV_BLCK, NULL);
> -                     if (fd == -1)
> -                             err(1, "could not open %s", dev);
> -                     if (fstat(fd, &sb) == -1) {
> -                             int saved_errno = errno;
> +                     if (strcmp(dev, "OFFLINE")) {
> +                             fd = opendev(dev, O_RDONLY, OPENDEV_BLCK, NULL);
> +                             if (fd == -1)
> +                                     err(1, "could not open %s", dev);
> +                             if (fstat(fd, &sb) == -1) {
> +                                     int saved_errno = errno;
> +                                     close(fd);
> +                                     errc(1, saved_errno, "could not stat 
> %s", dev);
> +                             }
>                               close(fd);
> -                             errc(1, saved_errno, "could not stat %s", dev);
> +                             dt[no_dev] = sb.st_rdev;
> +                     } else {
> +                             dt[no_dev] = NODEV;
>                       }
> -                     close(fd);
> -                     dt[no_dev] = sb.st_rdev;
>                       no_dev++;
>                       if (no_dev > (int)(BIOC_CRMAXLEN / sizeof(dev_t)))
>                               errx(1, "too many devices on device list");
> 
> 
> Long version:
> 
> 
> --- sbin/bioctl/bioctl.c.old  Fri Apr 26 07:45:28 2024
> +++ sbin/bioctl/bioctl.c      Tue Jan  2 00:14:59 2024
> @@ -833,9 +833,9 @@
>       struct sr_crypto_kdfinfo kdfinfo;
>       struct sr_crypto_pbkdf  kdfhint;
>       struct stat             sb;
> -     int                     rv, no_dev, fd;
> +     int                     rv, no_dev, online = 0, fd, i;
>       dev_t                   *dt;
> -     u_int16_t               min_disks = 0;
> +     u_int16_t               min_disks = 0, min_online;
> 
>       if (!dev_list)
>               errx(1, "no devices specified");
> @@ -845,6 +845,7 @@
>               err(1, "not enough memory for dev_t list");
> 
>       no_dev = bio_parse_devlist(dev_list, dt);
> +     min_online = no_dev;
> 
>       switch (level) {
>       case 0:
> @@ -852,12 +853,15 @@
>               break;
>       case 1:
>               min_disks = 2;
> +             min_online = 1;
>               break;
>       case 5:
>               min_disks = 3;
> +             min_online = no_dev - 1;
>               break;
> -     case 'C':
>       case 0x1C:
> +             min_online = 1;
> +     case 'C':
>               min_disks = 1;
>               break;
>       case 'c':
> @@ -870,6 +874,13 @@
>       if (no_dev < min_disks)
>               errx(1, "not enough disks");
> 
> +     for (i = 0; i < no_dev; i++)
> +             if (dt[i] != NODEV)
> +                     online++;
> +
> +     if (online < min_online)
> +             errx(1, "not enough disks online");
> +
>       /* for crypto raid we only allow one single chunk */
>       if (level == 'C' && no_dev != min_disks)
>               errx(1, "not exactly one partition");
> @@ -1015,16 +1026,20 @@
>                       /* got one */
>                       sz = e - s + 1;
>                       strlcpy(dev, s, sz + 1);
> -                     fd = opendev(dev, O_RDONLY, OPENDEV_BLCK, NULL);
> -                     if (fd == -1)
> -                             err(1, "could not open %s", dev);
> -                     if (fstat(fd, &sb) == -1) {
> -                             int saved_errno = errno;
> +                     if (strcmp(dev, "OFFLINE")) {
> +                             fd = opendev(dev, O_RDONLY, OPENDEV_BLCK, NULL);
> +                             if (fd == -1)
> +                                     err(1, "could not open %s", dev);
> +                             if (fstat(fd, &sb) == -1) {
> +                                     int saved_errno = errno;
> +                                     close(fd);
> +                                     errc(1, saved_errno, "could not stat 
> %s", dev);
> +                             }
>                               close(fd);
> -                             errc(1, saved_errno, "could not stat %s", dev);
> +                             dt[no_dev] = sb.st_rdev;
> +                     } else {
> +                             dt[no_dev] = NODEV;
>                       }
> -                     close(fd);
> -                     dt[no_dev] = sb.st_rdev;
>                       no_dev++;
>                       if (no_dev > (int)(BIOC_CRMAXLEN / sizeof(dev_t)))
>                               errx(1, "too many devices on device list");
> @@ -1034,7 +1049,7 @@
> 
>       for (i = 0; i < no_dev; i++)
>               for (x = 0; x < no_dev; x++)
> -                     if (dt[i] == dt[x] && x != i)
> +                     if (dt[i] == dt[x] && x != i && dt[i] != NODEV)
>                               errx(1, "duplicate device in list");
> 
>       return (no_dev);
> 

Reply via email to