[Midnightbsd-cvs] src [12260] trunk: The kernel driver for /dev/midistat implements a handler for read(2).
laffer1 at midnightbsd.org
laffer1 at midnightbsd.org
Thu Aug 22 18:44:46 EDT 2019
Revision: 12260
http://svnweb.midnightbsd.org/src/?rev=12260
Author: laffer1
Date: 2019-08-22 18:44:45 -0400 (Thu, 22 Aug 2019)
Log Message:
-----------
The kernel driver for /dev/midistat implements a handler for read(2).
This handler is not thread-safe, and a multi-threaded program can
exploit races in the handler to cause it to copy out kernel memory
outside the boundaries of midistat's data buffer.
Modified Paths:
--------------
trunk/UPDATING
trunk/sys/dev/sound/midi/midi.c
Modified: trunk/UPDATING
===================================================================
--- trunk/UPDATING 2019-08-22 12:32:48 UTC (rev 12259)
+++ trunk/UPDATING 2019-08-22 22:44:45 UTC (rev 12260)
@@ -1,5 +1,11 @@
Updating Information for MidnightBSD users.
+20190822:
+ The kernel driver for /dev/midistat implements a handler for read(2).
+ This handler is not thread-safe, and a multi-threaded program can
+ exploit races in the handler to cause it to copy out kernel memory
+ outside the boundaries of midistat's data buffer.
+
20190821:
Security patch for CVE-2019-5611.
Modified: trunk/sys/dev/sound/midi/midi.c
===================================================================
--- trunk/sys/dev/sound/midi/midi.c 2019-08-22 12:32:48 UTC (rev 12259)
+++ trunk/sys/dev/sound/midi/midi.c 2019-08-22 22:44:45 UTC (rev 12260)
@@ -39,6 +39,7 @@
__FBSDID("$FreeBSD: stable/10/sys/dev/sound/midi/midi.c 299632 2016-05-13 09:01:20Z ngie $");
#include <sys/param.h>
+#include <sys/systm.h>
#include <sys/queue.h>
#include <sys/kernel.h>
#include <sys/lock.h>
@@ -48,10 +49,8 @@
#include <sys/conf.h>
#include <sys/selinfo.h>
#include <sys/sysctl.h>
-#include <sys/types.h>
#include <sys/malloc.h>
-#include <sys/param.h>
-#include <sys/systm.h>
+#include <sys/sx.h>
#include <sys/proc.h>
#include <sys/fcntl.h>
#include <sys/types.h>
@@ -186,10 +185,9 @@
* /dev/midistat variables and declarations, protected by midistat_lock
*/
-static struct mtx midistat_lock;
+static struct sx midistat_lock;
static int midistat_isopen = 0;
static struct sbuf midistat_sbuf;
-static int midistat_bufptr;
static struct cdev *midistat_dev;
/*
@@ -288,7 +286,7 @@
MIDI_TYPE *buf;
MIDI_DEBUG(1, printf("midiinit: unit %d/%d.\n", unit, channel));
- mtx_lock(&midistat_lock);
+ sx_xlock(&midistat_lock);
/*
* Protect against call with existing unit/channel or auto-allocate a
* new unit number.
@@ -315,13 +313,8 @@
unit = i + 1;
MIDI_DEBUG(1, printf("midiinit #2: unit %d/%d.\n", unit, channel));
- m = malloc(sizeof(*m), M_MIDI, M_NOWAIT | M_ZERO);
- if (m == NULL)
- goto err0;
-
- m->synth = malloc(sizeof(*m->synth), M_MIDI, M_NOWAIT | M_ZERO);
- if (m->synth == NULL)
- goto err1;
+ m = malloc(sizeof(*m), M_MIDI, M_WAITOK | M_ZERO);
+ m->synth = malloc(sizeof(*m->synth), M_MIDI, M_WAITOK | M_ZERO);
kobj_init((kobj_t)m->synth, &midisynth_class);
m->synth->m = m;
kobj_init((kobj_t)m, cls);
@@ -330,7 +323,7 @@
MIDI_DEBUG(1, printf("midiinit queues %d/%d.\n", inqsize, outqsize));
if (!inqsize && !outqsize)
- goto err2;
+ goto err1;
mtx_init(&m->lock, "raw midi", NULL, 0);
mtx_init(&m->qlock, "q raw midi", NULL, 0);
@@ -355,9 +348,8 @@
if ((inqsize && !MIDIQ_BUF(m->inq)) ||
(outqsize && !MIDIQ_BUF(m->outq)))
- goto err3;
+ goto err2;
-
m->busy = 0;
m->flags = 0;
m->unit = unit;
@@ -365,7 +357,7 @@
m->cookie = cookie;
if (MPU_INIT(m, cookie))
- goto err3;
+ goto err2;
mtx_unlock(&m->lock);
mtx_unlock(&m->qlock);
@@ -372,7 +364,7 @@
TAILQ_INSERT_TAIL(&midi_devs, m, link);
- mtx_unlock(&midistat_lock);
+ sx_xunlock(&midistat_lock);
m->dev = make_dev(&midi_cdevsw,
MIDIMKMINOR(unit, MIDI_DEV_RAW, channel),
@@ -381,7 +373,8 @@
return m;
-err3: mtx_destroy(&m->qlock);
+err2:
+ mtx_destroy(&m->qlock);
mtx_destroy(&m->lock);
if (MIDIQ_BUF(m->inq))
@@ -388,9 +381,11 @@
free(MIDIQ_BUF(m->inq), M_MIDI);
if (MIDIQ_BUF(m->outq))
free(MIDIQ_BUF(m->outq), M_MIDI);
-err2: free(m->synth, M_MIDI);
-err1: free(m, M_MIDI);
-err0: mtx_unlock(&midistat_lock);
+err1:
+ free(m->synth, M_MIDI);
+ free(m, M_MIDI);
+err0:
+ sx_xunlock(&midistat_lock);
MIDI_DEBUG(1, printf("midi_init ended in error\n"));
return NULL;
}
@@ -408,7 +403,7 @@
int err;
err = EBUSY;
- mtx_lock(&midistat_lock);
+ sx_xlock(&midistat_lock);
mtx_lock(&m->lock);
if (m->busy) {
if (!(m->rchan || m->wchan))
@@ -427,8 +422,10 @@
if (!err)
goto exit;
-err: mtx_unlock(&m->lock);
-exit: mtx_unlock(&midistat_lock);
+err:
+ mtx_unlock(&m->lock);
+exit:
+ sx_xunlock(&midistat_lock);
return err;
}
@@ -940,27 +937,22 @@
int error;
MIDI_DEBUG(1, printf("midistat_open\n"));
- mtx_lock(&midistat_lock);
+ sx_xlock(&midistat_lock);
if (midistat_isopen) {
- mtx_unlock(&midistat_lock);
+ sx_xunlock(&midistat_lock);
return EBUSY;
}
midistat_isopen = 1;
- mtx_unlock(&midistat_lock);
-
if (sbuf_new(&midistat_sbuf, NULL, 4096, SBUF_AUTOEXTEND) == NULL) {
error = ENXIO;
- mtx_lock(&midistat_lock);
goto out;
}
- mtx_lock(&midistat_lock);
- midistat_bufptr = 0;
error = (midistat_prepare(&midistat_sbuf) > 0) ? 0 : ENOMEM;
-
-out: if (error)
+out:
+ if (error)
midistat_isopen = 0;
- mtx_unlock(&midistat_lock);
+ sx_xunlock(&midistat_lock);
return error;
}
@@ -968,40 +960,40 @@
midistat_close(struct cdev *i_dev, int flags, int mode, struct thread *td)
{
MIDI_DEBUG(1, printf("midistat_close\n"));
- mtx_lock(&midistat_lock);
+ sx_xlock(&midistat_lock);
if (!midistat_isopen) {
- mtx_unlock(&midistat_lock);
+ sx_xunlock(&midistat_lock);
return EBADF;
}
sbuf_delete(&midistat_sbuf);
midistat_isopen = 0;
-
- mtx_unlock(&midistat_lock);
+ sx_xunlock(&midistat_lock);
return 0;
}
static int
-midistat_read(struct cdev *i_dev, struct uio *buf, int flag)
+midistat_read(struct cdev *i_dev, struct uio *uio, int flag)
{
- int l, err;
+ long l;
+ int err;
MIDI_DEBUG(4, printf("midistat_read\n"));
- mtx_lock(&midistat_lock);
+ sx_xlock(&midistat_lock);
if (!midistat_isopen) {
- mtx_unlock(&midistat_lock);
+ sx_xunlock(&midistat_lock);
return EBADF;
}
- l = min(buf->uio_resid, sbuf_len(&midistat_sbuf) - midistat_bufptr);
+ if (uio->uio_offset < 0 || uio->uio_offset > sbuf_len(&midistat_sbuf)) {
+ sx_xunlock(&midistat_lock);
+ return EINVAL;
+ }
err = 0;
+ l = lmin(uio->uio_resid, sbuf_len(&midistat_sbuf) - uio->uio_offset);
if (l > 0) {
- mtx_unlock(&midistat_lock);
- err = uiomove(sbuf_data(&midistat_sbuf) + midistat_bufptr, l,
- buf);
- mtx_lock(&midistat_lock);
- } else
- l = 0;
- midistat_bufptr += l;
- mtx_unlock(&midistat_lock);
+ err = uiomove(sbuf_data(&midistat_sbuf) + uio->uio_offset, l,
+ uio);
+ }
+ sx_xunlock(&midistat_lock);
return err;
}
@@ -1014,7 +1006,7 @@
{
struct snd_midi *m;
- mtx_assert(&midistat_lock, MA_OWNED);
+ sx_assert(&midistat_lock, SA_XLOCKED);
sbuf_printf(s, "FreeBSD Midi Driver (midi2)\n");
if (TAILQ_EMPTY(&midi_devs)) {
@@ -1377,8 +1369,7 @@
static int
midi_destroy(struct snd_midi *m, int midiuninit)
{
-
- mtx_assert(&midistat_lock, MA_OWNED);
+ sx_assert(&midistat_lock, SA_XLOCKED);
mtx_assert(&m->lock, MA_OWNED);
MIDI_DEBUG(3, printf("midi_destroy\n"));
@@ -1404,8 +1395,8 @@
static int
midi_load(void)
{
- mtx_init(&midistat_lock, "midistat lock", NULL, 0);
- TAILQ_INIT(&midi_devs); /* Initialize the queue. */
+ sx_init(&midistat_lock, "midistat lock");
+ TAILQ_INIT(&midi_devs);
midistat_dev = make_dev(&midistat_cdevsw,
MIDIMKMINOR(0, MIDI_DEV_MIDICTL, 0),
@@ -1422,7 +1413,7 @@
MIDI_DEBUG(1, printf("midi_unload()\n"));
retval = EBUSY;
- mtx_lock(&midistat_lock);
+ sx_xlock(&midistat_lock);
if (midistat_isopen)
goto exit0;
@@ -1435,20 +1426,19 @@
if (retval)
goto exit1;
}
+ sx_xunlock(&midistat_lock);
+ destroy_dev(midistat_dev);
- mtx_unlock(&midistat_lock); /* XXX */
-
- destroy_dev(midistat_dev);
/*
* Made it here then unload is complete
*/
- mtx_destroy(&midistat_lock);
+ sx_destroy(&midistat_lock);
return 0;
exit1:
mtx_unlock(&m->lock);
exit0:
- mtx_unlock(&midistat_lock);
+ sx_xunlock(&midistat_lock);
if (retval)
MIDI_DEBUG(2, printf("midi_unload: failed\n"));
return retval;
@@ -1501,13 +1491,11 @@
int retval = 0;
struct snd_midi *m;
- mtx_lock(&midistat_lock);
-
+ sx_xlock(&midistat_lock);
TAILQ_FOREACH(m, &midi_devs, link) {
retval++;
}
-
- mtx_unlock(&midistat_lock);
+ sx_xunlock(&midistat_lock);
return retval;
}
@@ -1523,17 +1511,15 @@
struct snd_midi *m;
int retval = 0;
- mtx_lock(&midistat_lock);
-
+ sx_xlock(&midistat_lock);
TAILQ_FOREACH(m, &midi_devs, link) {
if (unit == retval) {
- mtx_unlock(&midistat_lock);
+ sx_xunlock(&midistat_lock);
return (kobj_t)m->synth;
}
retval++;
}
-
- mtx_unlock(&midistat_lock);
+ sx_xunlock(&midistat_lock);
return NULL;
}
More information about the Midnightbsd-cvs
mailing list