From: Crystal Kolipe Subject: Re: softraid: incorrect chunk metadata checksum To: Miroslav Cimerman Cc: "tech@openbsd.org" Date: Wed, 23 Apr 2025 03:34:33 -0300 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; >