Hi again,

Sorry for the mangled whitespace in my previous reply.

I just noticed that your code from [1] doesn't initialize
the checksums correctly, you don't assign scm in the foreach.

Copied from my previous reply:
Well, are there any issues that could arise from fixing it?
It's not actually validated anywhere, hotspares and new chunks
for rebuild are initialized correctly.


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

--
Miroslav Cimerman


Crystal Kolipe <kolipe.c@exoticsilicon.com> wrote:
> 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;
> >