Index | Thread | Search

From:
YASUOKA Masahiko <yasuoka@openbsd.org>
Subject:
Re: Close the existing keydisk.img.
To:
takeasou.masato@gmail.com
Cc:
tech@openbsd.org
Date:
Thu, 12 Feb 2026 00:22:06 +0900

Download raw body.

Thread
Hi

On Mon, 9 Feb 2026 16:07:29 +0900
ASOU Masato <takeasou.masato@gmail.com> wrote:
> When I run the make command in regress/usr.sbin/install for the
> second time, vnd2 fails to be unconfigured.  The problem does not
> occur if the make clean command is run (to delete keydisk.img) before
> the second time.
> 
> If keydisk.img was generatad with the dd command during the first
> run, softraid_crypto.c: sr_crypto_create_key_disk() execute
> VOP_CLOSE("vnd2").  However, if keydisk.img from the previous run
> still exists, softraid_crypto.c: sr_crypto_read_key_disk() does not
> execute VOP_CLOSE("vnd2").
> 
> The patch is included at the end of this email.
> 
> 
> The below is first time and vnd2 success to be unconfigured.
> 
>     $ cd /usr/src/regress/usr.sbin/installboot
>     $ doas make
>     doas (asou@obsd78-vnconfig.example.co.jp) password:
>     ==== create-root ====
>     dd if=/dev/zero  of=disk1.img bs=1m count=0 seek=768 status=none
>     vnconfig -- disk1.img 1>diskdev1.txt
>     fdisk -gy -b 532480 -- "$(<diskdev1.txt)" 1>/dev/null
>     echo 'RAID *' |  disklabel -wAT- -- "$(<diskdev1.txt)" 1>/dev/null
>     dd if=/dev/zero  of=disk2.img bs=1m count=0 seek=768 status=none
>     vnconfig -- disk2.img 1>diskdev2.txt
>     fdisk -gy -b 532480 -- "$(<diskdev2.txt)" 1>/dev/null
>     echo 'RAID *' |  disklabel -wAT- -- "$(<diskdev2.txt)" 1>/dev/null
>     dd if=/dev/zero  of=keydisk.img bs=1m count=0 seek=768 status=none
>     vnconfig -- keydisk.img 1>keydev.txt
>     fdisk -gy -b 532480 -- "$(<keydev.txt)" 1>/dev/null
>     echo 'RAID *' |  disklabel -wAT- -- "$(<keydev.txt)" 1>/dev/null
>     bioctl -l"$(sed -- s/$/a/ diskdev1.txt diskdev2.txt | paste -sd,
> -- -)"  -c1C -k"$(<keydev.txt)"a -- softraid0 |  grep -o --
> 'sd[0-9]*$' 1>rootdev.txt
>     fdisk -gy -b 532480 -- "$(<rootdev.txt)" 1>/dev/null
>     disklabel -Aw -- "$(<rootdev.txt)"
>     newfs -q -- "$(<rootdev.txt)"a 1>/dev/null
>     newfs: reduced number of fragments per cylinder group from 31696
> to 31432 to enlarge last cylinder group
>     mkdir -p -- mnt
>     mount -- /dev/"$(<rootdev.txt)"a mnt
>     mkdir -- mnt/usr
>     cp -r -- /usr/mdec mnt/usr/
> 
>     ==== prepare ====
>     /usr/sbin/installboot -p -- "$(<rootdev.txt)"
> 
>     ==== dry-prepare ====
>     /usr/sbin/installboot -n -p -- "$(<rootdev.txt)"
> 
>     ==== dry-default ====
>     /usr/sbin/installboot -n -- "$(<rootdev.txt)"
> 
>     ==== dry-root ====
>     /usr/sbin/installboot -n -r/ -- "$(<rootdev.txt)"
> 
>     ==== root ====
>     /usr/sbin/installboot -r mnt -- "$(<rootdev.txt)"
> 
>     ==== root-stages ====
>     /usr/sbin/installboot -- "$(<rootdev.txt)" /usr/mdec/biosboot /usr/mdec/boot
> 
>     ==== dry-prepare-root ====
>     /usr/sbin/installboot -n -p -r/ -- "$(<rootdev.txt)" 2>/dev/null
>     *** Error 1 in . (Makefile:142 'dry-prepare-root')
>     EXPECTED_FAIL
> 
>     ==== dry-prepare-stages ====
>     /usr/sbin/installboot -n -p -- "$(<rootdev.txt)"
> /usr/mdec/biosboot /usr/mdec/boot 2>/dev/null
>     *** Error 1 in . (Makefile:144 'dry-prepare-stages')
>     EXPECTED_FAIL
> 
>     ==== dry-nodisk-stages ====
>     /usr/sbin/installboot -n -- /usr/mdec/biosboot /usr/mdec/boot 2>/dev/null
>     *** Error 1 in . (Makefile:147 'dry-nodisk-stages')
>     EXPECTED_FAIL
> 
>     ==== dry-toofew ====
>     /usr/sbin/installboot -n -- 2>/dev/null
>     *** Error 1 in . (Makefile:150 'dry-toofew')
>     EXPECTED_FAIL
> 
>     ==== dry-toomany ====
>     /usr/sbin/installboot -n -- disk stage1 stage2 too many 2>/dev/null
>     *** Error 1 in . (Makefile:152 'dry-toomany')
>     EXPECTED_FAIL
> 
>     ==== cleanup ====
>     umount -- mnt 2>/dev/null
>     rmdir -- mnt 2>/dev/null
>     bioctl -d -- "$(<rootdev.txt)" 2>/dev/null
>     vnconfig -u -- "$(<diskdev1.txt)" 2>/dev/null
>     vnconfig -u -- "$(<diskdev2.txt)" 2>/dev/null
>     vnconfig -u -- "$(<keydev.txt)" 2>/dev/null
> 
>     $ doas vnconfig -l
>     vnd0: not in use
>     vnd1: not in use
>     vnd2: not in use
>     vnd3: not in use
>     $
> 
> The above seems fine.
> 
> However, when I run the make command again, vnd2 fails to be
> unconfigured.
> 
>     $ doas make
>     ==== create-root ====
>     dd if=/dev/zero  of=disk1.img bs=1m count=0 seek=768 status=none
>     vnconfig -- disk1.img 1>diskdev1.txt
>     fdisk -gy -b 532480 -- "$(<diskdev1.txt)" 1>/dev/null
>     echo 'RAID *' |  disklabel -wAT- -- "$(<diskdev1.txt)" 1>/dev/null
>     dd if=/dev/zero  of=disk2.img bs=1m count=0 seek=768 status=none
>     vnconfig -- disk2.img 1>diskdev2.txt
>     fdisk -gy -b 532480 -- "$(<diskdev2.txt)" 1>/dev/null
>     echo 'RAID *' |  disklabel -wAT- -- "$(<diskdev2.txt)" 1>/dev/null
>     dd if=/dev/zero  of=keydisk.img bs=1m count=0 seek=768 status=none
>     vnconfig -- keydisk.img 1>keydev.txt
>     fdisk -gy -b 532480 -- "$(<keydev.txt)" 1>/dev/null
>     echo 'RAID *' |  disklabel -wAT- -- "$(<keydev.txt)" 1>/dev/null
>     bioctl -l"$(sed -- s/$/a/ diskdev1.txt diskdev2.txt | paste -sd,
> -- -)"  -c1C -k"$(<keydev.txt)"a -- softraid0 |  grep -o --
> 'sd[0-9]*$' 1>rootdev.txt
>     fdisk -gy -b 532480 -- "$(<rootdev.txt)" 1>/dev/null
>     disklabel -Aw -- "$(<rootdev.txt)"
>     newfs -q -- "$(<rootdev.txt)"a 1>/dev/null
>     newfs: reduced number of fragments per cylinder group from 31696
> to 31432 to enlarge last cylinder group
>     mkdir -p -- mnt
>     mount -- /dev/"$(<rootdev.txt)"a mnt
>     mkdir -- mnt/usr
>     cp -r -- /usr/mdec mnt/usr/
> 
>     ==== prepare ====
>     /usr/sbin/installboot -p -- "$(<rootdev.txt)"
> 
>     ==== dry-prepare ====
>     /usr/sbin/installboot -n -p -- "$(<rootdev.txt)"
> 
>     ==== dry-default ====
>     /usr/sbin/installboot -n -- "$(<rootdev.txt)"
> 
>     ==== dry-root ====
>     /usr/sbin/installboot -n -r/ -- "$(<rootdev.txt)"
> 
>     ==== root ====
>     /usr/sbin/installboot -r mnt -- "$(<rootdev.txt)"
> 
>     ==== root-stages ====
>     /usr/sbin/installboot -- "$(<rootdev.txt)" /usr/mdec/biosboot /usr/mdec/boot
> 
>     ==== dry-prepare-root ====
>     /usr/sbin/installboot -n -p -r/ -- "$(<rootdev.txt)" 2>/dev/null
>     *** Error 1 in . (Makefile:142 'dry-prepare-root')
>     EXPECTED_FAIL
> 
>     ==== dry-prepare-stages ====
>     /usr/sbin/installboot -n -p -- "$(<rootdev.txt)"
> /usr/mdec/biosboot /usr/mdec/boot 2>/dev/null
>     *** Error 1 in . (Makefile:144 'dry-prepare-stages')
>     EXPECTED_FAIL
> 
>     ==== dry-nodisk-stages ====
>     /usr/sbin/installboot -n -- /usr/mdec/biosboot /usr/mdec/boot 2>/dev/null
>     *** Error 1 in . (Makefile:147 'dry-nodisk-stages')
>     EXPECTED_FAIL
> 
>     ==== dry-toofew ====
>     /usr/sbin/installboot -n -- 2>/dev/null
>     *** Error 1 in . (Makefile:150 'dry-toofew')
>     EXPECTED_FAIL
> 
>     ==== dry-toomany ====
>     /usr/sbin/installboot -n -- disk stage1 stage2 too many 2>/dev/null
>     *** Error 1 in . (Makefile:152 'dry-toomany')
>     EXPECTED_FAIL
> 
>     ==== cleanup ====
>     umount -- mnt 2>/dev/null
>     rmdir -- mnt 2>/dev/null
>     bioctl -d -- "$(<rootdev.txt)" 2>/dev/null
>     vnconfig -u -- "$(<diskdev1.txt)" 2>/dev/null
>     vnconfig -u -- "$(<diskdev2.txt)" 2>/dev/null
>     vnconfig -u -- "$(<keydev.txt)" 2>/dev/null
>     *** Error 1 in target 'cleanup' (ignored)
> 
>     $ doas vnconfig -l
>     vnd0: not in use
>     vnd1: not in use
>     vnd2: covering keydisk.img on sd0f, inode 77924
>     vnd3: not in use
>     $

Let me make the test simple,

- Prepare
  dd if=/dev/zero of=dsk.img bs=1m seek=10 count=0
  dd if=/dev/zero of=key.img bs=1m seek=10 count=0
  vnconfig vnd0 dsk.img
  vnconfig vnd1 key.img
  fdisk -iy vnd0
  fdisk -iy vnd1
  disklabel -E vnd0  # create 'a' parition fstype=RAID
  disklabel -E vnd1  # create 'a' parition fstype=RAID

- Test (if the key disk is busy after bioctl)
  repeat twice {
    bioctl -c C -l /dev/vnd0a -k /dev/vnd1a softraid0
    vnconfig -u vnd1	      # must success without error
    bioctl -d sd1
    vnconfig vnd1 key.img
  } 

Test success at when loop #1 but it fails when loop #2.  keydisk is
created when #1 and read aferwards.

==
loop#1
# bioctl -c C -l /dev/vnd0a -k /dev/vnd1a softraid0
sd1 at scsibus3 targ 1 lun 0: <OPENBSD, SR CRYPTO, 006>
sd1: 9MB, 512 bytes/sector, 19824 sectors
softraid0: CRYPTO volume attached as sd1
# vnconfig -u vnd1
# bioctl -d d1                                                                 
bioctl: Can't locate d1 device via /dev/bio
# bioctl -d sd1
sd1 detached
# vnconfig vnd1 key.img

loop#2
# bioctl -c C -l /dev/vnd0a -k /dev/vnd1a softraid0
sd1 at scsibus3 targ 1 lun 0: <OPENBSD, SR CRYPTO, 006>
sd1: 9MB, 512 bytes/sector, 19824 sectors
softraid0: CRYPTO volume attached as sd1
# vnconfig -u vnd1
vnconfig: VNDIOCCLR: Device busy
==

> comment? ok?

Basically your diff seems ok, but see the below

> diff --git a/sys/dev/softraid_crypto.c b/sys/dev/softraid_crypto.c
> index 6e24f2ff4f5..8e52701e5f9 100644
> --- a/sys/dev/softraid_crypto.c
> +++ b/sys/dev/softraid_crypto.c
> @@ -837,7 +837,7 @@ sr_crypto_read_key_disk(struct sr_discipline *sd,
> struct sr_crypto *mdd_crypto,
>                 vput(vn);
>                 goto done;
>         }
> -       open = 1; /* close dev on error */
> +       open = 1;

Keep the comment, Becase the comment is very correct and dev must not
be closed when not error.

> 
>         /* Get partition details. */
>         label = malloc(sizeof(*label), M_DEVBUF, M_WAITOK);
> @@ -900,8 +900,6 @@ sr_crypto_read_key_disk(struct sr_discipline *sd,
> struct sr_crypto *mdd_crypto,
>                 }
>         }
> 
> -       open = 0;
> -

Yes.

loop #1 is ok, because sr_crypto_create_key_disk() doesn't have the
lines like this.

>  done:
>         for (omi = SLIST_FIRST(&som); omi != NULL; omi = omi_next) {
>                 omi_next = SLIST_NEXT(omi, omi_link);
> 
>