Don't leak the ctrl_data in uvideo_queryctrl, uvideo_s_ctrl and uvideo_g_ctrl.

If one of the usb calls we did here failed we'd return immediately and
not free our buffer.

ok miod@

2 files changed, 49 insertions, 41 deletions

diff --git a/sys/dev/usb/umidi.c b/sys/dev/usb/umidi.c
index 82fb2e1c50a..58a8d5712e3 100644
--- a/sys/dev/usb/umidi.c
+++ b/sys/dev/usb/umidi.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: umidi.c,v 1.29 2011/06/17 07:06:47 mk Exp $	*/
+/*	$OpenBSD: umidi.c,v 1.30 2011/06/23 22:03:43 oga Exp $	*/
 /*	$NetBSD: umidi.c,v 1.16 2002/07/11 21:14:32 augustss Exp $	*/
 /*
  * Copyright (c) 2001 The NetBSD Foundation, Inc.
@@ -84,7 +84,6 @@ static usbd_status bind_jacks_to_mididev(struct umidi_softc *,
 					 struct umidi_jack *,
 					 struct umidi_mididev *);
 static void unbind_jacks_from_mididev(struct umidi_mididev *);
-static void unbind_all_jacks(struct umidi_softc *);
 static usbd_status assign_all_jacks_automatically(struct umidi_softc *);
 static usbd_status open_out_jack(struct umidi_jack *, void *,
 				 void (*)(void *));
@@ -194,8 +193,7 @@ umidi_attach(struct device *parent, struct device *self, void *aux)
 	}
 	err = alloc_all_jacks(sc);
 	if (err!=USBD_NORMAL_COMPLETION) {
-		free_all_endpoints(sc);
-		goto error;
+		goto free_ends;
 	}
 	printf("%s: out=%d, in=%d\n",
 	       sc->sc_dev.dv_xname,
@@ -203,15 +201,12 @@ umidi_attach(struct device *parent, struct device *self, void *aux)
 
 	err = assign_all_jacks_automatically(sc);
 	if (err!=USBD_NORMAL_COMPLETION) {
-		unbind_all_jacks(sc);
-		free_all_jacks(sc);
-		free_all_endpoints(sc);
-		goto error;
+		goto free_jacks;
 	}
 	err = attach_all_mididevs(sc);
 	if (err!=USBD_NORMAL_COMPLETION) {
-		free_all_jacks(sc);
-		free_all_endpoints(sc);
+		/* XXX what about the mididevs that attached? */
+		goto free_mididevs;
 	}
 
 #ifdef UMIDI_DEBUG
@@ -223,6 +218,13 @@ umidi_attach(struct device *parent, struct device *self, void *aux)
 	}
 
 	return;
+
+free_mididevs:
+	free_all_mididevs(sc);
+free_jacks:
+	free_all_jacks(sc);
+free_ends:
+	free_all_endpoints(sc);
 error:
 	printf("%s: disabled.\n", sc->sc_dev.dv_xname);
 	sc->sc_dying = 1;
@@ -408,7 +410,7 @@ alloc_all_endpoints(struct umidi_softc *sc)
 		err = alloc_all_endpoints_genuine(sc);
 	}
 	if (err!=USBD_NORMAL_COMPLETION)
-		return err;
+		free(sc->sc_endpoints, M_USBDEV);
 
 	ep = sc->sc_endpoints;
 	for (i=sc->sc_out_num_endpoints+sc->sc_in_num_endpoints; i>0; i--) {
@@ -842,17 +844,6 @@ unbind_jacks_from_mididev(struct umidi_mididev *mididev)
 	mididev->out_jack = mididev->in_jack = NULL;
 }
 
-static void
-unbind_all_jacks(struct umidi_softc *sc)
-{
-	int i;
-
-	if (sc->sc_mididevs)
-		for (i=0; i<sc->sc_num_mididevs; i++) {
-			unbind_jacks_from_mididev(&sc->sc_mididevs[i]);
-		}
-}
-
 static usbd_status
 assign_all_jacks_automatically(struct umidi_softc *sc)
 {
@@ -994,16 +985,21 @@ static usbd_status
 attach_all_mididevs(struct umidi_softc *sc)
 {
 	usbd_status err;
-	int i;
+	int i, j;
 
 	if (sc->sc_mididevs)
 		for (i=0; i<sc->sc_num_mididevs; i++) {
 			err = attach_mididev(sc, &sc->sc_mididevs[i]);
 			if (err!=USBD_NORMAL_COMPLETION)
-				return err;
+				goto detach;
 		}
 
 	return USBD_NORMAL_COMPLETION;
+detach:
+
+	for (j = 0; j < i; j++)
+		(void)detach_mididev(&sc->sc_mididevs[i], DETACH_QUIET);
+	return err;
 }
 
 static usbd_status
diff --git a/sys/dev/usb/uvideo.c b/sys/dev/usb/uvideo.c
index 3cbe0c76a84..10b5f6e50f1 100644
--- a/sys/dev/usb/uvideo.c
+++ b/sys/dev/usb/uvideo.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: uvideo.c,v 1.162 2011/06/17 07:06:47 mk Exp $ */
+/*	$OpenBSD: uvideo.c,v 1.163 2011/06/23 22:03:43 oga Exp $ */
 
 /*
  * Copyright (c) 2008 Robert Nagy <robert@openbsd.org>
@@ -3227,7 +3227,7 @@ int
 uvideo_queryctrl(void *v, struct v4l2_queryctrl *qctrl)
 {
 	struct uvideo_softc *sc = v;
-	int i;
+	int i, ret = 0;
 	usbd_status error;
 	uint8_t *ctrl_data;
 	uint16_t ctrl_len;
@@ -3258,8 +3258,10 @@ uvideo_queryctrl(void *v, struct v4l2_queryctrl *qctrl)
 	error = uvideo_vc_get_ctrl(sc, ctrl_data, GET_MIN,
 	    sc->sc_desc_vc_pu_cur->bUnitID,
 	    uvideo_ctrls[i].ctrl_selector, uvideo_ctrls[i].ctrl_len);
-	if (error != USBD_NORMAL_COMPLETION)
-		return (EINVAL);
+	if (error != USBD_NORMAL_COMPLETION) {
+		ret = EINVAL;
+		goto out;
+	}
 	switch (ctrl_len) {
 	case 1:
 		qctrl->minimum = uvideo_ctrls[i].sig ?
@@ -3277,8 +3279,10 @@ uvideo_queryctrl(void *v, struct v4l2_queryctrl *qctrl)
 	error = uvideo_vc_get_ctrl(sc, ctrl_data, GET_MAX,
 	    sc->sc_desc_vc_pu_cur->bUnitID,
 	    uvideo_ctrls[i].ctrl_selector, uvideo_ctrls[i].ctrl_len);
-	if (error != USBD_NORMAL_COMPLETION)
-		return (EINVAL);
+	if (error != USBD_NORMAL_COMPLETION) {
+		ret = EINVAL;
+		goto out;
+	}
 	switch(ctrl_len) {
 	case 1:
 		qctrl->maximum = uvideo_ctrls[i].sig ?
@@ -3296,8 +3300,10 @@ uvideo_queryctrl(void *v, struct v4l2_queryctrl *qctrl)
 	error = uvideo_vc_get_ctrl(sc, ctrl_data, GET_RES,
 	    sc->sc_desc_vc_pu_cur->bUnitID,
 	    uvideo_ctrls[i].ctrl_selector, uvideo_ctrls[i].ctrl_len);
-	if (error != USBD_NORMAL_COMPLETION)
-		return (EINVAL);
+	if (error != USBD_NORMAL_COMPLETION) {
+		ret = EINVAL;
+		goto out;
+	}
 	switch(ctrl_len) {
 	case 1:
 		qctrl->step = uvideo_ctrls[i].sig ?
@@ -3315,8 +3321,10 @@ uvideo_queryctrl(void *v, struct v4l2_queryctrl *qctrl)
 	error = uvideo_vc_get_ctrl(sc, ctrl_data, GET_DEF,
 	    sc->sc_desc_vc_pu_cur->bUnitID,
 	    uvideo_ctrls[i].ctrl_selector, uvideo_ctrls[i].ctrl_len);
-	if (error != USBD_NORMAL_COMPLETION)
-		return (EINVAL);
+	if (error != USBD_NORMAL_COMPLETION) {
+		ret = EINVAL;
+		goto out;
+	}
 	switch(ctrl_len) {
 	case 1:
 		qctrl->default_value = uvideo_ctrls[i].sig ?
@@ -3333,16 +3341,17 @@ uvideo_queryctrl(void *v, struct v4l2_queryctrl *qctrl)
 	/* set flags */
 	qctrl->flags = 0;
 
+out:
 	free(ctrl_data, M_USBDEV);
 
-	return (0);
+	return (ret);
 }
 
 int
 uvideo_g_ctrl(void *v, struct v4l2_control *gctrl)
 {
 	struct uvideo_softc *sc = v;
-	int i;
+	int i, ret = 0;
 	usbd_status error;
 	uint8_t *ctrl_data;
 	uint16_t ctrl_len;
@@ -3366,8 +3375,10 @@ uvideo_g_ctrl(void *v, struct v4l2_control *gctrl)
 	error = uvideo_vc_get_ctrl(sc, ctrl_data, GET_CUR,
 	    sc->sc_desc_vc_pu_cur->bUnitID,
 	    uvideo_ctrls[i].ctrl_selector, uvideo_ctrls[i].ctrl_len);
-	if (error != USBD_NORMAL_COMPLETION)
-		return (EINVAL);
+	if (error != USBD_NORMAL_COMPLETION) {
+		ret = EINVAL;
+		goto out;
+	}
 	switch(ctrl_len) {
 	case 1:
 		gctrl->value = uvideo_ctrls[i].sig ?
@@ -3381,6 +3392,7 @@ uvideo_g_ctrl(void *v, struct v4l2_control *gctrl)
 		break;
 	}
 
+out:
 	free(ctrl_data, M_USBDEV);
 
 	return (0);
@@ -3390,7 +3402,7 @@ int
 uvideo_s_ctrl(void *v, struct v4l2_control *sctrl)
 {
 	struct uvideo_softc *sc = v;
-	int i;
+	int i, ret = 0;
 	usbd_status error;
 	uint8_t *ctrl_data;
 	uint16_t ctrl_len;
@@ -3426,11 +3438,11 @@ uvideo_s_ctrl(void *v, struct v4l2_control *sctrl)
 	    sc->sc_desc_vc_pu_cur->bUnitID,
 	    uvideo_ctrls[i].ctrl_selector, uvideo_ctrls[i].ctrl_len);
 	if (error != USBD_NORMAL_COMPLETION)
-		return (EINVAL);
+		ret = EINVAL;
 
 	free(ctrl_data, M_USBDEV);
 
-	return (0);
+	return (ret);
 }
 
 int