Prevent a double free by assigning the new keymap and corresponding size
authoranton <anton@openbsd.org>
Thu, 30 Dec 2021 06:55:11 +0000 (06:55 +0000)
committeranton <anton@openbsd.org>
Thu, 30 Dec 2021 06:55:11 +0000 (06:55 +0000)
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
sys/dev/wscons/wskbd.c
sys/dev/wscons/wskbdutil.c
sys/dev/wscons/wsksymvar.h

index ea2cbb2..5693a5b 100644 (file)
@@ -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;
+}
index 9cca6ee..504c105 100644 (file)
@@ -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];
index 7ab9eee..4056992 100644 (file)
@@ -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 *);