Date: Tue, 20 Aug 2019 17:53:16 +0000 (UTC) From: Mark Johnston <markj@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-12@freebsd.org Subject: svn commit: r351264 - stable/12/sys/dev/sound/midi Message-ID: <201908201753.x7KHrGpx067243@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: markj Date: Tue Aug 20 17:53:16 2019 New Revision: 351264 URL: https://svnweb.freebsd.org/changeset/base/351264 Log: MFC r351262: Use a sleepable lock for midistat functions. Security: CVE-2019-5612 Modified: stable/12/sys/dev/sound/midi/midi.c Directory Properties: stable/12/ (props changed) Modified: stable/12/sys/dev/sound/midi/midi.c ============================================================================== --- stable/12/sys/dev/sound/midi/midi.c Tue Aug 20 17:52:28 2019 (r351263) +++ stable/12/sys/dev/sound/midi/midi.c Tue Aug 20 17:53:16 2019 (r351264) @@ -40,6 +40,7 @@ __FBSDID("$FreeBSD$"); #include <sys/param.h> +#include <sys/systm.h> #include <sys/queue.h> #include <sys/kernel.h> #include <sys/lock.h> @@ -49,10 +50,8 @@ __FBSDID("$FreeBSD$"); #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> @@ -187,10 +186,9 @@ TAILQ_HEAD(, snd_midi) midi_devs; * /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; /* @@ -289,7 +287,7 @@ midi_init(kobj_class_t cls, int unit, int channel, voi 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. @@ -316,13 +314,8 @@ midi_init(kobj_class_t cls, int unit, int channel, voi 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); @@ -331,7 +324,7 @@ midi_init(kobj_class_t cls, int unit, int channel, voi 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); @@ -356,9 +349,8 @@ midi_init(kobj_class_t cls, int unit, int channel, voi if ((inqsize && !MIDIQ_BUF(m->inq)) || (outqsize && !MIDIQ_BUF(m->outq))) - goto err3; + goto err2; - m->busy = 0; m->flags = 0; m->unit = unit; @@ -366,14 +358,14 @@ midi_init(kobj_class_t cls, int unit, int channel, voi m->cookie = cookie; if (MPU_INIT(m, cookie)) - goto err3; + goto err2; mtx_unlock(&m->lock); mtx_unlock(&m->qlock); 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), @@ -382,16 +374,19 @@ midi_init(kobj_class_t cls, int unit, int channel, voi return m; -err3: mtx_destroy(&m->qlock); +err2: + mtx_destroy(&m->qlock); mtx_destroy(&m->lock); if (MIDIQ_BUF(m->inq)) 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; } @@ -409,7 +404,7 @@ midi_uninit(struct snd_midi *m) int err; err = EBUSY; - mtx_lock(&midistat_lock); + sx_xlock(&midistat_lock); mtx_lock(&m->lock); if (m->busy) { if (!(m->rchan || m->wchan)) @@ -428,8 +423,10 @@ midi_uninit(struct snd_midi *m) 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; } @@ -941,27 +938,22 @@ midistat_open(struct cdev *i_dev, int flags, int mode, 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; } @@ -969,40 +961,40 @@ static int 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; } @@ -1015,7 +1007,7 @@ midistat_prepare(struct sbuf *s) { 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)) { @@ -1378,8 +1370,7 @@ midisynth_bender(void *n, uint8_t chn, uint16_t val) 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")); @@ -1405,8 +1396,8 @@ midi_destroy(struct snd_midi *m, int midiuninit) 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), @@ -1423,7 +1414,7 @@ midi_unload(void) MIDI_DEBUG(1, printf("midi_unload()\n")); retval = EBUSY; - mtx_lock(&midistat_lock); + sx_xlock(&midistat_lock); if (midistat_isopen) goto exit0; @@ -1436,20 +1427,19 @@ midi_unload(void) if (retval) goto exit1; } - - mtx_unlock(&midistat_lock); /* XXX */ - + sx_xunlock(&midistat_lock); 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; @@ -1498,13 +1488,11 @@ midimapper_open(void *arg1, void **cookie) 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; } @@ -1520,17 +1508,15 @@ midimapper_fetch_synth(void *arg, void *cookie, int un 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; }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201908201753.x7KHrGpx067243>