Index | Thread | Search

From:
Crystal Kolipe <kolipe.c@exoticsilicon.com>
Subject:
Re: softraid: incorrect chunk metadata checksum
To:
Miroslav Cimerman <mc@doas.su>
Cc:
"tech@openbsd.org" <tech@openbsd.org>
Date:
Wed, 23 Apr 2025 03:34:33 -0300

Download raw body.

Thread
Hi,

Please see:

https://marc.info/?l=openbsd-tech&m=164834052628572&w=2

This code has been buggy for a long time.

On Tue, Apr 22, 2025 at 09:01:11PM +0000, Miroslav Cimerman wrote:
> Hello,
> 
> In softraid, chunk metadata is incorrectly initialized.
> The checksum is done only on the first 16 bytes.
> 
> It should be done on whole `struct sr_meta_chunk_invariant',
> and after the coerced size is in place.
> 
> On request I can provide a simple shell script to test this,
> I didn't want to clutter this mail.
> 
> 
> --
> Miroslav Cimerman
> 
> 
> Index: sys/dev/softraid.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/softraid.c,v
> diff -u -p -u -p -r1.433 softraid.c
> --- sys/dev/softraid.c	8 Jan 2025 23:40:40 -0000	1.433
> +++ sys/dev/softraid.c	22 Apr 2025 20:41:46 -0000
> @@ -571,8 +571,6 @@ sr_meta_init(struct sr_discipline *sd, i
>  		    sizeof(scm->scmi.scm_devname));
>  		memcpy(&scm->scmi.scm_uuid, &sm->ssdi.ssd_uuid,
>  		    sizeof(scm->scmi.scm_uuid));
> -		sr_checksum(sc, scm, &scm->scm_checksum,
> -		    sizeof(scm->scm_checksum));
>  
>  		if (min_chunk_sz == 0)
>  			min_chunk_sz = scm->scmi.scm_size;
> @@ -584,9 +582,13 @@ sr_meta_init(struct sr_discipline *sd, i
>  
>  	sm->ssdi.ssd_secsize = secsize;
>  
> -	/* Equalize chunk sizes. */
> -	SLIST_FOREACH(chunk, cl, src_link)
> -		chunk->src_meta.scmi.scm_coerced_size = min_chunk_sz;
> +	/* Equalize chunk sizes and calculate checksum. */
> +	SLIST_FOREACH(chunk, cl, src_link) {
> +		scm = &chunk->src_meta;
> +		scm->scmi.scm_coerced_size = min_chunk_sz;
> +		sr_checksum(sc, scm, &scm->scm_checksum,
> +		    sizeof(struct sr_meta_chunk_invariant));
> +	}
>  
>  	sd->sd_vol.sv_chunk_minsz = min_chunk_sz;
>  	sd->sd_vol.sv_chunk_maxsz = max_chunk_sz;
>