Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

Re: Use after free in ami(4)

1 view
Skip to first unread message

Jonathan Gray

unread,
May 6, 2013, 10:09:22 PM5/6/13
to
On Sun, May 05, 2013 at 01:30:05PM +0100, Federico Schwindt wrote:
>
> 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.-

Well there is one more case and then after that sc->sc_cfg is set
to cfg so we shouldn't leak anything else?

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 7 May 2013 02:00:01 -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);
@@ -1480,8 +1482,10 @@ mfi_bio_getitall(struct mfi_softc *sc)
cfg = malloc(size, M_DEVBUF, M_NOWAIT | M_ZERO);
if (cfg == NULL)
goto done;
- if (mfi_mgmt(sc, MR_DCMD_CONF_GET, MFI_DATA_IN, size, cfg, NULL))
+ if (mfi_mgmt(sc, MR_DCMD_CONF_GET, MFI_DATA_IN, size, cfg, NULL)) {
+ free(cfg, M_DEVBUF);
goto done;
+ }

/* replace current pointer with enw one */
if (sc->sc_cfg)

Mark Kettenis

unread,
May 7, 2013, 3:38:34 AM5/7/13
to
> Date: Tue, 7 May 2013 12:09:22 +1000
> From: Jonathan Gray <j...@jsg.id.au>
>
> On Sun, May 05, 2013 at 01:30:05PM +0100, Federico Schwindt wrote:
> >
> > 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.-
>
> Well there is one more case and then after that sc->sc_cfg is set
> to cfg so we shouldn't leak anything else?

ok kettenis@

Kenneth R Westerback

unread,
May 7, 2013, 6:37:00 AM5/7/13
to
On Tue, May 07, 2013 at 12:09:22PM +1000, Jonathan Gray wrote:
> On Sun, May 05, 2013 at 01:30:05PM +0100, Federico Schwindt wrote:
> >
> > 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.-
>
> Well there is one more case and then after that sc->sc_cfg is set
> to cfg so we shouldn't leak anything else?
>
ok krw@. You missed a typo in the comment! 'enw one'. :-)

.... Ken

0 new messages