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

Reply via email to