On Sun, May 5, 2013 at 2:46 AM, Kenneth R Westerback <[email protected]
> wrote:
> On Sun, May 05, 2013 at 11:12:56AM +1000, Jonathan Gray wrote:
> > On Sat, May 04, 2013 at 07:04:59PM -0400, Kenneth R Westerback wrote:
> > > On Sat, May 04, 2013 at 08:25:18PM +0300, Arto Jonsson wrote:
> > > > Spotted by LLVM static analyser.
> > > >
> > > > In /usr/src/sys/dev/ic/ami.c line 499 ami_freemem(sc, am) is called.
> In
> > > > the next block ami_alloc_ccbs(...) is called. If the result != 0
> control
> > > > jumps to free_mbox in line 614. This falls through to call
> > > > ami_freemem(sc, am) again.
> > > >
> > >
> > > Is LLMV happy with this diff?
> > >
> > > .... Ken
> > >
> > > Index: ami.c
> > > ===================================================================
> > > RCS file: /cvs/src/sys/dev/ic/ami.c,v
> > > retrieving revision 1.223
> > > diff -u -p -r1.223 ami.c
> > > --- ami.c 9 Jan 2012 18:50:44 -0000 1.223
> > > +++ ami.c 4 May 2013 23:02:16 -0000
> > > @@ -497,6 +497,7 @@ ami_attach(struct ami_softc *sc)
> > > }
> > >
> > > ami_freemem(sc, am);
> > > + am = NULL;
> > >
> > > if (ami_alloc_ccbs(sc, AMI_MAXCMDS + 1) != 0) {
> > > /* error already printed */
> > > @@ -614,7 +615,8 @@ ami_attach(struct ami_softc *sc)
> > > free_mbox:
> > > ami_freemem(sc, sc->sc_mbox_am);
> > > free_idata:
> > > - ami_freemem(sc, am);
> > > + if (am)
> > > + ami_freemem(sc, am);
> > >
> > > return (1);
> > > }
> >
> > here is the diff I had for ami, the mfi one included here as a bonus
>
> Looks better to me. ok krw@ for both.
>
> .... Ken
>
> >
> > Index: ami.c
> > ===================================================================
> > RCS file: /cvs/src/sys/dev/ic/ami.c,v
> > retrieving revision 1.223
> > diff -u -p -r1.223 ami.c
> > --- ami.c 9 Jan 2012 18:50:44 -0000 1.223
> > +++ ami.c 5 May 2013 01:06:08 -0000
> > @@ -496,12 +496,12 @@ ami_attach(struct ami_softc *sc)
> > sc->sc_link.openings = sc->sc_maxcmds;
> > }
> >
> > - ami_freemem(sc, am);
> > -
> > if (ami_alloc_ccbs(sc, AMI_MAXCMDS + 1) != 0) {
> > /* error already printed */
> > goto free_mbox;
> > }
> > +
> > + ami_freemem(sc, am);
> >
> > /* hack for hp netraid version encoding */
> > if ('A' <= sc->sc_fwver[2] && sc->sc_fwver[2] <= 'Z' &&
> > Index: mfi.c
> > ===================================================================
> > RCS file: /cvs/src/sys/dev/ic/mfi.c,v
> > retrieving revision 1.144
> > diff -u -p -r1.144 mfi.c
> > --- mfi.c 3 May 2013 02:46:28 -0000 1.144
> > +++ mfi.c 3 May 2013 07:20:03 -0000
> > @@ -1470,8 +1470,10 @@ mfi_bio_getitall(struct mfi_softc *sc)
> > if (cfg == NULL)
> > goto done;
> > if (mfi_mgmt(sc, MR_DCMD_CONF_GET, MFI_DATA_IN, sizeof *cfg, cfg,
> > - NULL))
> > + NULL)) {
> > + free(cfg, M_DEVBUF);
> > goto done;
> > + }
> >
> > size = cfg->mfc_size;
> > free(cfg, M_DEVBUF);
>
>
ok to both.
there are more cases in mfi_bio_getitall() where the second allocated cfg
(ld_det as well) might not be freed if something goes wrong.
f.-
