From owner-svn-src-stable-11@freebsd.org Tue Aug 20 17:54:18 2019 Return-Path: Delivered-To: svn-src-stable-11@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id CF6BCD743A; Tue, 20 Aug 2019 17:54:18 +0000 (UTC) (envelope-from markj@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 46Cdhp53wzz4ctq; Tue, 20 Aug 2019 17:54:18 +0000 (UTC) (envelope-from markj@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 8F8C26F54; Tue, 20 Aug 2019 17:54:18 +0000 (UTC) (envelope-from markj@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id x7KHsIQv067349; Tue, 20 Aug 2019 17:54:18 GMT (envelope-from markj@FreeBSD.org) Received: (from markj@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id x7KHsImq067348; Tue, 20 Aug 2019 17:54:18 GMT (envelope-from markj@FreeBSD.org) Message-Id: <201908201754.x7KHsImq067348@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: markj set sender to markj@FreeBSD.org using -f From: Mark Johnston Date: Tue, 20 Aug 2019 17:54:18 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-11@freebsd.org Subject: svn commit: r351265 - stable/11/sys/dev/sound/midi X-SVN-Group: stable-11 X-SVN-Commit-Author: markj X-SVN-Commit-Paths: stable/11/sys/dev/sound/midi X-SVN-Commit-Revision: 351265 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-stable-11@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: SVN commit messages for only the 11-stable src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 20 Aug 2019 17:54:18 -0000 Author: markj Date: Tue Aug 20 17:54:18 2019 New Revision: 351265 URL: https://svnweb.freebsd.org/changeset/base/351265 Log: MFC r351262: Use a sleepable lock for midistat functions. Security: CVE-2019-5612 Modified: stable/11/sys/dev/sound/midi/midi.c Directory Properties: stable/11/ (props changed) Modified: stable/11/sys/dev/sound/midi/midi.c ============================================================================== --- stable/11/sys/dev/sound/midi/midi.c Tue Aug 20 17:53:16 2019 (r351264) +++ stable/11/sys/dev/sound/midi/midi.c Tue Aug 20 17:54:18 2019 (r351265) @@ -38,6 +38,7 @@ __FBSDID("$FreeBSD$"); #include +#include #include #include #include @@ -47,10 +48,8 @@ __FBSDID("$FreeBSD$"); #include #include #include -#include #include -#include -#include +#include #include #include #include @@ -185,10 +184,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; /* @@ -287,7 +285,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. @@ -314,13 +312,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); @@ -329,7 +322,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); @@ -354,9 +347,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; @@ -364,14 +356,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), @@ -380,16 +372,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; } @@ -407,7 +402,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)) @@ -426,8 +421,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; } @@ -939,27 +936,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; } @@ -967,40 +959,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; } @@ -1013,7 +1005,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)) { @@ -1376,8 +1368,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")); @@ -1403,8 +1394,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), @@ -1421,7 +1412,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; @@ -1434,20 +1425,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; @@ -1496,13 +1486,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; } @@ -1518,17 +1506,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; }