Index | Thread | Search

From:
Stefan Sperling <stsp@stsp.name>
Subject:
unchecked malloc failures in hidmt_setup()
To:
tech@openbsd.org
Cc:
Thomas Dettbarn <dettus@dettus.net>, bru@openbsd.org
Date:
Mon, 16 Jun 2025 17:59:26 +0200

Download raw body.

Thread
  • Stefan Sperling:

    unchecked malloc failures in hidmt_setup()

Two M_NOWAIT malloc calls in hidmt_setup() lack NULL return checks.
The chance of hitting this in practice is low because this code
usally only runs during autoconf at boot time.

Fixing this exposed some minor memory leaks:
The code leaks the report buffer allocation in all cases, and it does
not free input capability report buffers on failure.

Tested on the Z13.
Noticed while reading the imt(4) patch sent by Thomas Dettbarn.


M  sys/dev/hid/hidmt.c  |  25+  2-

1 file changed, 25 insertions(+), 2 deletions(-)

path + /usr/src
commit - 2b4e04f3782d5ff8d215bda9dc807a2e742c53de
blob - 62b500a4f446e0fa1696cba1f22f13b9c4a4efe4
file + sys/dev/hid/hidmt.c
--- sys/dev/hid/hidmt.c
+++ sys/dev/hid/hidmt.c
@@ -125,6 +125,11 @@ hidmt_setup(struct device *self, struct hidmt *mt, voi
 
 	capsize = hid_report_size(desc, dlen, hid_feature, mt->sc_rep_cap);
 	rep = malloc(capsize, M_DEVBUF, M_NOWAIT | M_ZERO);
+	if (rep == NULL) {
+		printf("\n%s: could not allocate capability report buffer\n",
+		    self->dv_xname);
+		return 1;
+	}
 
 	if (mt->hidev_report_type_conv == NULL)
 		panic("no report type conversion function");
@@ -134,6 +139,7 @@ hidmt_setup(struct device *self, struct hidmt *mt, voi
 	    rep, capsize)) {
 		printf("\n%s: failed getting capability report\n",
 		    self->dv_xname);
+		free(rep, M_DEVBUF, capsize);
 		return 1;
 	}
 
@@ -211,6 +217,12 @@ hidmt_setup(struct device *self, struct hidmt *mt, voi
 		}
 
 		input = malloc(sizeof(*input), M_DEVBUF, M_NOWAIT | M_ZERO);
+		if (input == NULL) {
+			printf("\n%s: could not allocate input report buffer\n",
+			    self->dv_xname);
+			hid_end_parse(hd);
+			goto fail;
+		}
 		memcpy(&input->loc, &h.loc, sizeof(struct hid_location));
 		input->usage = h.usage;
 
@@ -221,16 +233,27 @@ hidmt_setup(struct device *self, struct hidmt *mt, voi
 	if (mt->sc_maxx <= 0 || mt->sc_maxy <= 0) {
 		printf("\n%s: invalid max X/Y %d/%d\n", self->dv_xname,
 		    mt->sc_maxx, mt->sc_maxy);
-		return 1;
+		goto fail;
 	}
 
 	if (hidmt_set_input_mode(mt, HIDMT_INPUT_MODE_MT_TOUCHPAD)) {
 		printf("\n%s: switch to multitouch mode failed\n",
 		    self->dv_xname);
-		return 1;
+		goto fail;
 	}
 
+	free(rep, M_DEVBUF, capsize);
 	return 0;
+
+fail:
+	while (!SIMPLEQ_EMPTY(&mt->sc_inputs)) {
+		struct hidmt_data *input = SIMPLEQ_FIRST(&mt->sc_inputs);
+		SIMPLEQ_REMOVE_HEAD(&mt->sc_inputs, entry);
+		free(input, M_DEVBUF, sizeof(*input));
+	}
+
+	free(rep, M_DEVBUF, capsize);
+	return 1;
 }
 
 void