On Mon, May 13, 2019 at 07:25:37PM -0400, Ted Unangst wrote:
> Jan Klemkow wrote:
> > The diff mainly add sizes to free(9) calls. But, while here fix a
> > malloc(9) call with the wrong size in sr_ioctl_installboot().
> > omi->omi_som is allocated with size of struct sr_meta_crypto, but used
> > as struct sr_meta_boot later.
> >
> > One free(9) with size zero left over in sr_discipline_free(). As, the
> > allocated size of sd->sd_vol.sv_chunks is not ascertainable here without
> > larger changes.
> >
> > Build and run the kernel for testing.
>
> The change in installboot for the size is suspicious. I would leave that out.
> Especially if you haven't tested installboot to a crypto softraid. :)
Here is a diff with just the fixed free sizes. Malloc size fix comes
separately.
OK?
Index: dev/softraid.c
===================================================================
RCS file: /cvs/src/sys/dev/softraid.c,v
retrieving revision 1.392
diff -u -p -r1.392 softraid.c
--- dev/softraid.c 2 May 2018 02:24:55 -0000 1.392
+++ dev/softraid.c 14 May 2019 05:13:02 -0000
@@ -1470,28 +1470,29 @@ unwind:
for (bc1 = SLIST_FIRST(&bv1->sbv_chunks); bc1 != NULL;
bc1 = bc2) {
bc2 = SLIST_NEXT(bc1, sbc_link);
- free(bc1->sbc_metadata, M_DEVBUF, 0);
- free(bc1, M_DEVBUF, 0);
+ free(bc1->sbc_metadata, M_DEVBUF,
+ sizeof(*bc1->sbc_metadata));
+ free(bc1, M_DEVBUF, sizeof(*bc1));
}
- free(bv1, M_DEVBUF, 0);
+ free(bv1, M_DEVBUF, sizeof(*bv1));
}
/* Free keydisks chunks. */
for (bc1 = SLIST_FIRST(&kdh); bc1 != NULL; bc1 = bc2) {
bc2 = SLIST_NEXT(bc1, sbc_link);
- free(bc1->sbc_metadata, M_DEVBUF, 0);
- free(bc1, M_DEVBUF, 0);
+ free(bc1->sbc_metadata, M_DEVBUF, sizeof(*bc1->sbc_metadata));
+ free(bc1, M_DEVBUF, sizeof(*bc1));
}
/* Free unallocated chunks. */
for (bc1 = SLIST_FIRST(&bch); bc1 != NULL; bc1 = bc2) {
bc2 = SLIST_NEXT(bc1, sbc_link);
- free(bc1->sbc_metadata, M_DEVBUF, 0);
- free(bc1, M_DEVBUF, 0);
+ free(bc1->sbc_metadata, M_DEVBUF, sizeof(*bc1->sbc_metadata));
+ free(bc1, M_DEVBUF, sizeof(*bc1));
}
while (!SLIST_EMPTY(&sdklist)) {
sdk = SLIST_FIRST(&sdklist);
SLIST_REMOVE_HEAD(&sdklist, sdk_link);
- free(sdk, M_DEVBUF, 0);
+ free(sdk, M_DEVBUF, sizeof(*sdk));
}
free(devs, M_DEVBUF, BIOC_CRMAXLEN * sizeof(dev_t));
@@ -1751,7 +1752,7 @@ sr_hotplug_unregister(struct sr_discipli
if (mhe != NULL) {
SLIST_REMOVE(&sr_hotplug_callbacks, mhe,
sr_hotplug_list, shl_link);
- free(mhe, M_DEVBUF, 0);
+ free(mhe, M_DEVBUF, sizeof(*mhe));
}
}
@@ -1953,7 +1954,7 @@ sr_ccb_free(struct sr_discipline *sd)
while ((ccb = TAILQ_FIRST(&sd->sd_ccb_freeq)) != NULL)
TAILQ_REMOVE(&sd->sd_ccb_freeq, ccb, ccb_link);
- free(sd->sd_ccb, M_DEVBUF, 0);
+ free(sd->sd_ccb, M_DEVBUF, sizeof(*sd->sd_ccb));
}
struct sr_ccb *
@@ -2132,7 +2133,7 @@ sr_wu_free(struct sr_discipline *sd)
while ((wu = TAILQ_FIRST(&sd->sd_wu)) != NULL) {
TAILQ_REMOVE(&sd->sd_wu, wu, swu_next);
- free(wu, M_DEVBUF, 0);
+ free(wu, M_DEVBUF, sizeof(*wu));
}
}
@@ -2966,13 +2967,14 @@ sr_hotspare(struct sr_softc *sc, dev_t d
goto done;
fail:
- free(hotspare, M_DEVBUF, 0);
+ free(hotspare, M_DEVBUF, sizeof(*hotspare));
done:
if (sd)
- free(sd->sd_vol.sv_chunks, M_DEVBUF, 0);
- free(sd, M_DEVBUF, 0);
- free(sm, M_DEVBUF, 0);
+ free(sd->sd_vol.sv_chunks, M_DEVBUF,
+ sizeof(sd->sd_vol.sv_chunks));
+ free(sd, M_DEVBUF, sizeof(*sd));
+ free(sm, M_DEVBUF, sizeof(*sm));
if (open) {
VOP_CLOSE(vn, FREAD | FWRITE, NOCRED, curproc);
vput(vn);
@@ -3079,7 +3081,7 @@ sr_hotspare_rebuild(struct sr_discipline
/* Remove hotspare from available list. */
sc->sc_hotspare_no--;
SLIST_REMOVE(cl, hotspare, sr_chunk, src_link);
- free(hotspare, M_DEVBUF, 0);
+ free(hotspare, M_DEVBUF, sizeof(*hotspare));
}
rw_exit_write(&sc->sc_lock);
@@ -3375,7 +3377,7 @@ sr_ioctl_createraid(struct sr_softc *sc,
&sd->sd_meta->ssdi.ssd_uuid);
sr_error(sc, "disk %s is currently in use; "
"cannot force create", uuid);
- free(uuid, M_DEVBUF, 0);
+ free(uuid, M_DEVBUF, 37);
goto unwind;
}
@@ -3439,7 +3441,7 @@ sr_ioctl_createraid(struct sr_softc *sc,
if (sr_already_assembled(sd)) {
uuid = sr_uuid_format(&sd->sd_meta->ssdi.ssd_uuid);
sr_error(sc, "disk %s already assembled", uuid);
- free(uuid, M_DEVBUF, 0);
+ free(uuid, M_DEVBUF, 37);
goto unwind;
}
@@ -3805,8 +3807,8 @@ sr_ioctl_installboot(struct sr_softc *sc
rv = 0;
done:
- free(bootblk, M_DEVBUF, 0);
- free(bootldr, M_DEVBUF, 0);
+ free(bootblk, M_DEVBUF, bbs);
+ free(bootldr, M_DEVBUF, bls);
return (rv);
}
@@ -3837,7 +3839,7 @@ sr_chunks_unwind(struct sr_softc *sc, st
curproc);
vput(ch_entry->src_vn);
}
- free(ch_entry, M_DEVBUF, 0);
+ free(ch_entry, M_DEVBUF, sizeof(*ch_entry));
}
SLIST_INIT(cl);
}
@@ -3861,14 +3863,14 @@ sr_discipline_free(struct sr_discipline
if (sd->sd_free_resources)
sd->sd_free_resources(sd);
free(sd->sd_vol.sv_chunks, M_DEVBUF, 0);
- free(sd->sd_meta, M_DEVBUF, 0);
- free(sd->sd_meta_foreign, M_DEVBUF, 0);
+ free(sd->sd_meta, M_DEVBUF, SR_META_SIZE * DEV_BSIZE);
+ free(sd->sd_meta_foreign, M_DEVBUF, smd[sd->sd_meta_type].smd_size);
som = &sd->sd_meta_opt;
for (omi = SLIST_FIRST(som); omi != NULL; omi = omi_next) {
omi_next = SLIST_NEXT(omi, omi_link);
free(omi->omi_som, M_DEVBUF, 0);
- free(omi, M_DEVBUF, 0);
+ free(omi, M_DEVBUF, sizeof(*omi));
}
if (sd->sd_target != 0) {
@@ -3884,7 +3886,7 @@ sr_discipline_free(struct sr_discipline
TAILQ_REMOVE(&sc->sc_dis_list, sd, sd_link);
explicit_bzero(sd, sizeof *sd);
- free(sd, M_DEVBUF, 0);
+ free(sd, M_DEVBUF, sizeof(*sd));
}
void
