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:27:33 +0900

On Thu, 12 Feb 2026 00:22:06 +0900 (JST)
YASUOKA Masahiko <yasuoka@openbsd.org> wrote:
> 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.
(snip)
> 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.

Sorry, the last sentence must be:

Keep the comment, because the comment is very correct and dev *MUST*
be closed without 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);
>> 
>> 
>