From: anton <anton@openbsd.org>
Date: Thu, 30 Dec 2021 06:55:11 +0000 (+0000)
Subject: Prevent a double free by assigning the new keymap and corresponding size
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=61c8c0f0fd3e5e422eda433673a010dd0da6b046;p=openbsd

Prevent a double free by assigning the new keymap and corresponding size
after the allocation and initialization is done. Otherwise, a race is
possible if malloc ends up sleeping.

ok sashan@

Reported-by: syzbot+7f8224e9f1a3487caf25@syzkaller.appspotmail.com
---

diff --git a/sys/dev/wscons/wskbd.c b/sys/dev/wscons/wskbd.c
index ea2cbb21e28..5693a5b614e 100644
--- a/sys/dev/wscons/wskbd.c
+++ b/sys/dev/wscons/wskbd.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: wskbd.c,v 1.109 2021/10/22 04:59:31 anton Exp $ */
+/* $OpenBSD: wskbd.c,v 1.110 2021/12/30 06:55:11 anton Exp $ */
 /* $NetBSD: wskbd.c,v 1.80 2005/05/04 01:52:16 augustss Exp $ */
 
 /*
@@ -233,6 +233,8 @@ int	wskbd_mux_close(struct wsevsrc *);
 int	wskbd_do_open(struct wskbd_softc *, struct wseventvar *);
 int	wskbd_do_ioctl(struct device *, u_long, caddr_t, int, struct proc *);
 
+void	wskbd_set_keymap(struct wskbd_softc *, struct wscons_keymap *, int);
+
 int	(*wskbd_get_backlight)(struct wskbd_backlight *);
 int	(*wskbd_set_backlight)(struct wskbd_backlight *);
 
@@ -414,9 +416,14 @@ wskbd_attach(struct device *parent, struct device *self, void *aux)
 	}
 #endif
 	for (;;) {
-		if (wskbd_load_keymap(&sc->id->t_keymap, layout, &sc->sc_map,
-		    &sc->sc_maplen) == 0)
+		struct wscons_keymap *map;
+		int maplen;
+
+		if (wskbd_load_keymap(&sc->id->t_keymap, layout, &map,
+		    &maplen) == 0) {
+			wskbd_set_keymap(sc, map, maplen);
 			break;
+		}
 #if NWSMUX > 0
 		if (layout == sc->id->t_keymap.layout)
 			panic("cannot load keymap");
@@ -1131,9 +1138,11 @@ getkeyrepeat:
 
 		error = copyin(umdp->map, buf, len);
 		if (error == 0) {
-			wskbd_init_keymap(umdp->maplen,
-					  &sc->sc_map, &sc->sc_maplen);
-			memcpy(sc->sc_map, buf, len);
+			struct wscons_keymap *map;
+
+			map = wskbd_init_keymap(umdp->maplen);
+			memcpy(map, buf, len);
+			wskbd_set_keymap(sc, map, umdp->maplen);
 			/* drop the variant bits handled by the map */
 			enc = KB_USER | (KB_VARIANT(sc->id->t_keymap.layout) &
 			    KB_HANDLEDBYWSKBD);
@@ -1169,10 +1178,14 @@ getkeyrepeat:
 		} else if (sc->id->t_keymap.layout & KB_NOENCODING) {
 			return (0);
 		} else {
+			struct wscons_keymap *map;
+			int maplen;
+
 			error = wskbd_load_keymap(&sc->id->t_keymap, enc,
-			    &sc->sc_map, &sc->sc_maplen);
+			    &map, &maplen);
 			if (error)
 				return (error);
+			wskbd_set_keymap(sc, map, maplen);
 		}
 		wskbd_update_layout(sc->id, enc);
 #if NWSMUX > 0
@@ -1858,3 +1871,11 @@ wskbd_debugger(struct wskbd_softc *sc)
 	}
 #endif
 }
+
+void
+wskbd_set_keymap(struct wskbd_softc *sc, struct wscons_keymap *map, int maplen)
+{
+	free(sc->sc_map, M_DEVBUF, sc->sc_maplen * sizeof(*sc->sc_map));
+	sc->sc_map = map;
+	sc->sc_maplen = maplen;
+}
diff --git a/sys/dev/wscons/wskbdutil.c b/sys/dev/wscons/wskbdutil.c
index 9cca6eea7a9..504c105325f 100644
--- a/sys/dev/wscons/wskbdutil.c
+++ b/sys/dev/wscons/wskbdutil.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: wskbdutil.c,v 1.18 2019/01/31 18:01:14 millert Exp $	*/
+/*	$OpenBSD: wskbdutil.c,v 1.19 2021/12/30 06:55:11 anton Exp $	*/
 /*	$NetBSD: wskbdutil.c,v 1.7 1999/12/21 11:59:13 drochner Exp $	*/
 
 /*-
@@ -375,27 +375,21 @@ wskbd_get_mapentry(const struct wskbd_mapdata *mapdata, int kc,
 	}
 }
 
-void
-wskbd_init_keymap(int newlen, struct wscons_keymap **map, int *maplen)
+struct wscons_keymap *
+wskbd_init_keymap(int maplen)
 {
+	struct wscons_keymap *map;
 	int i;
 
-	if (newlen != *maplen) {
-		if (*maplen > 0)
-			free(*map, M_DEVBUF,
-			    *maplen * sizeof(struct wscons_keymap));
-		*maplen = newlen;
-		*map = mallocarray(newlen, sizeof(struct wscons_keymap),
-		    M_DEVBUF, M_WAITOK);
-	}
-
-	for (i = 0; i < *maplen; i++) {
-		(*map)[i].command = KS_voidSymbol;
-		(*map)[i].group1[0] = KS_voidSymbol;
-		(*map)[i].group1[1] = KS_voidSymbol;
-		(*map)[i].group2[0] = KS_voidSymbol;
-		(*map)[i].group2[1] = KS_voidSymbol;
+	map = mallocarray(maplen, sizeof(*map), M_DEVBUF, M_WAITOK);
+	for (i = 0; i < maplen; i++) {
+		map[i].command = KS_voidSymbol;
+		map[i].group1[0] = KS_voidSymbol;
+		map[i].group1[1] = KS_voidSymbol;
+		map[i].group2[0] = KS_voidSymbol;
+		map[i].group2[1] = KS_voidSymbol;
 	}
+	return map;
 }
 
 int
@@ -437,7 +431,8 @@ wskbd_load_keymap(const struct wskbd_mapdata *mapdata, kbd_t layout,
 		}
 	}
 
-	wskbd_init_keymap(i + 1, map, maplen);
+	*map = wskbd_init_keymap(i + 1);
+	*maplen = i + 1;
 
 	for (s = stack_ptr - 1; s >= 0; s--) {
 		mp = stack[s];
diff --git a/sys/dev/wscons/wsksymvar.h b/sys/dev/wscons/wsksymvar.h
index 7ab9eee81fc..405699273f9 100644
--- a/sys/dev/wscons/wsksymvar.h
+++ b/sys/dev/wscons/wsksymvar.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: wsksymvar.h,v 1.9 2021/09/20 17:32:39 anton Exp $	*/
+/*	$OpenBSD: wsksymvar.h,v 1.10 2021/12/30 06:55:11 anton Exp $	*/
 /*	$NetBSD: wsksymvar.h,v 1.8.4.1 2000/07/07 09:50:21 hannken Exp $ */
 
 /*-
@@ -67,7 +67,7 @@ struct wskbd_mapdata {
  */
 void	wskbd_get_mapentry(const struct wskbd_mapdata *, int,
                                 struct wscons_keymap *);
-void	wskbd_init_keymap(int, struct wscons_keymap **, int *);
+struct wscons_keymap	*wskbd_init_keymap(int);
 int	wskbd_load_keymap(const struct wskbd_mapdata *, kbd_t,
                                struct wscons_keymap **, int *);
 keysym_t wskbd_compose_value(keysym_t *);