summaryrefslogtreecommitdiff
path: root/sys/dev/audio
diff options
context:
space:
mode:
authorisaki <isaki@NetBSD.org>2021-02-09 12:36:34 +0000
committerisaki <isaki@NetBSD.org>2021-02-09 12:36:34 +0000
commitbef9041a7edd54055810d1d596fedb4e1320b43d (patch)
tree40cebd166a10cb8b2b1790a212279d04ed9c0ac3 /sys/dev/audio
parentfdee3101130fc239adc09b14d324eda8290d9082 (diff)
Protect also audioopen() and audiobellopen() from audiodetach() with
psref(9), as well as others(audioread, audiowrite, etc..). - Rename audio_file_enter to audio_sc_acquire_fromfile, audio_file_exit to audio_sc_release, for clarify. These are the reference counter for this sc. - Introduce audio_sc_acquire_foropen for audio{,bell}open. - audio_open needs to examine sc_dying again before inserting it into sc_files, in order to keep sc_files consistency. The race between audiodetach and audioopen is pointed out by riastradh@. Thank you for many advices.
Diffstat (limited to 'sys/dev/audio')
-rw-r--r--sys/dev/audio/audio.c179
1 files changed, 126 insertions, 53 deletions
diff --git a/sys/dev/audio/audio.c b/sys/dev/audio/audio.c
index b9443b8a816..0f348a2a579 100644
--- a/sys/dev/audio/audio.c
+++ b/sys/dev/audio/audio.c
@@ -1,4 +1,4 @@
-/* $NetBSD: audio.c,v 1.89 2021/02/09 05:53:14 isaki Exp $ */
+/* $NetBSD: audio.c,v 1.90 2021/02/09 12:36:34 isaki Exp $ */
/*-
* Copyright (c) 2008 The NetBSD Foundation, Inc.
@@ -138,7 +138,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: audio.c,v 1.89 2021/02/09 05:53:14 isaki Exp $");
+__KERNEL_RCSID(0, "$NetBSD: audio.c,v 1.90 2021/02/09 12:36:34 isaki Exp $");
#ifdef _KERNEL_OPT
#include "audio.h"
@@ -521,8 +521,10 @@ static int audio_exlock_mutex_enter(struct audio_softc *);
static void audio_exlock_mutex_exit(struct audio_softc *);
static int audio_exlock_enter(struct audio_softc *);
static void audio_exlock_exit(struct audio_softc *);
-static struct audio_softc *audio_file_enter(audio_file_t *, struct psref *);
-static void audio_file_exit(struct audio_softc *, struct psref *);
+static void audio_sc_acquire_foropen(struct audio_softc *, struct psref *);
+static struct audio_softc *audio_sc_acquire_fromfile(audio_file_t *,
+ struct psref *);
+static void audio_sc_release(struct audio_softc *, struct psref *);
static int audio_track_waitio(struct audio_softc *, audio_track_t *);
static int audioclose(struct file *);
@@ -1302,7 +1304,10 @@ audiodetach(device_t self, int flags)
if (error)
return error;
- /* delete sysctl nodes */
+ /*
+ * This waits currently running sysctls to finish if exists.
+ * After this, no more new sysctls will come.
+ */
sysctl_teardown(&sc->sc_log);
mutex_enter(sc->sc_lock);
@@ -1531,11 +1536,40 @@ audio_exlock_exit(struct audio_softc *sc)
}
/*
- * Acquire sc from file, and increment the psref count.
+ * Increment reference counter for this sc.
+ * This is intended to be used for open.
+ */
+void
+audio_sc_acquire_foropen(struct audio_softc *sc, struct psref *refp)
+{
+ int s;
+
+ /* psref(9) forbids to migrate CPUs */
+ curlwp_bind();
+
+ /* Block audiodetach while we acquire a reference */
+ s = pserialize_read_enter();
+
+ /*
+ * We don't examine sc_dying here. However, all open methods
+ * call audio_exlock_enter() right after this, so we can examine
+ * sc_dying in it.
+ */
+
+ /* Acquire a reference */
+ psref_acquire(refp, &sc->sc_psref, audio_psref_class);
+
+ /* Now sc won't go away until we drop the reference count */
+ pserialize_read_exit(s);
+}
+
+/*
+ * Get sc from file, and increment reference counter for this sc.
+ * This is intended to be used for methods other than open.
* If successful, returns sc. Otherwise returns NULL.
*/
struct audio_softc *
-audio_file_enter(audio_file_t *file, struct psref *refp)
+audio_sc_acquire_fromfile(audio_file_t *file, struct psref *refp)
{
int s;
bool dying;
@@ -1563,10 +1597,10 @@ audio_file_enter(audio_file_t *file, struct psref *refp)
}
/*
- * Decrement the psref count.
+ * Decrement reference counter for this sc.
*/
void
-audio_file_exit(struct audio_softc *sc, struct psref *refp)
+audio_sc_release(struct audio_softc *sc, struct psref *refp)
{
psref_release(refp, &sc->sc_psref, audio_psref_class);
@@ -1644,6 +1678,7 @@ static int
audioopen(dev_t dev, int flags, int ifmt, struct lwp *l)
{
struct audio_softc *sc;
+ struct psref sc_ref;
int error;
/* Find the device */
@@ -1651,9 +1686,11 @@ audioopen(dev_t dev, int flags, int ifmt, struct lwp *l)
if (sc == NULL || sc->hw_if == NULL)
return ENXIO;
+ audio_sc_acquire_foropen(sc, &sc_ref);
+
error = audio_exlock_enter(sc);
if (error)
- return error;
+ goto done;
device_active(sc->sc_dev, DVA_SYSTEM);
switch (AUDIODEV(dev)) {
@@ -1673,6 +1710,8 @@ audioopen(dev_t dev, int flags, int ifmt, struct lwp *l)
}
audio_exlock_exit(sc);
+done:
+ audio_sc_release(sc, &sc_ref);
return error;
}
@@ -1697,7 +1736,7 @@ audioclose(struct file *fp)
* - free all memory objects, regardless of sc.
*/
- sc = audio_file_enter(file, &sc_ref);
+ sc = audio_sc_acquire_fromfile(file, &sc_ref);
if (sc) {
switch (AUDIODEV(dev)) {
case SOUND_DEVICE:
@@ -1715,7 +1754,7 @@ audioclose(struct file *fp)
break;
}
- audio_file_exit(sc, &sc_ref);
+ audio_sc_release(sc, &sc_ref);
}
/* Free memory objects anyway */
@@ -1744,7 +1783,7 @@ audioread(struct file *fp, off_t *offp, struct uio *uio, kauth_cred_t cred,
file = fp->f_audioctx;
dev = file->dev;
- sc = audio_file_enter(file, &sc_ref);
+ sc = audio_sc_acquire_fromfile(file, &sc_ref);
if (sc == NULL)
return EIO;
@@ -1765,7 +1804,7 @@ audioread(struct file *fp, off_t *offp, struct uio *uio, kauth_cred_t cred,
break;
}
- audio_file_exit(sc, &sc_ref);
+ audio_sc_release(sc, &sc_ref);
return error;
}
@@ -1783,7 +1822,7 @@ audiowrite(struct file *fp, off_t *offp, struct uio *uio, kauth_cred_t cred,
file = fp->f_audioctx;
dev = file->dev;
- sc = audio_file_enter(file, &sc_ref);
+ sc = audio_sc_acquire_fromfile(file, &sc_ref);
if (sc == NULL)
return EIO;
@@ -1804,7 +1843,7 @@ audiowrite(struct file *fp, off_t *offp, struct uio *uio, kauth_cred_t cred,
break;
}
- audio_file_exit(sc, &sc_ref);
+ audio_sc_release(sc, &sc_ref);
return error;
}
@@ -1822,7 +1861,7 @@ audioioctl(struct file *fp, u_long cmd, void *addr)
file = fp->f_audioctx;
dev = file->dev;
- sc = audio_file_enter(file, &sc_ref);
+ sc = audio_sc_acquire_fromfile(file, &sc_ref);
if (sc == NULL)
return EIO;
@@ -1847,7 +1886,7 @@ audioioctl(struct file *fp, u_long cmd, void *addr)
break;
}
- audio_file_exit(sc, &sc_ref);
+ audio_sc_release(sc, &sc_ref);
return error;
}
@@ -1861,7 +1900,7 @@ audiostat(struct file *fp, struct stat *st)
KASSERT(fp->f_audioctx);
file = fp->f_audioctx;
- sc = audio_file_enter(file, &sc_ref);
+ sc = audio_sc_acquire_fromfile(file, &sc_ref);
if (sc == NULL)
return EIO;
@@ -1872,7 +1911,7 @@ audiostat(struct file *fp, struct stat *st)
st->st_gid = kauth_cred_getegid(fp->f_cred);
st->st_mode = S_IFCHR;
- audio_file_exit(sc, &sc_ref);
+ audio_sc_release(sc, &sc_ref);
return 0;
}
@@ -1890,7 +1929,7 @@ audiopoll(struct file *fp, int events)
file = fp->f_audioctx;
dev = file->dev;
- sc = audio_file_enter(file, &sc_ref);
+ sc = audio_sc_acquire_fromfile(file, &sc_ref);
if (sc == NULL)
return POLLERR;
@@ -1908,7 +1947,7 @@ audiopoll(struct file *fp, int events)
break;
}
- audio_file_exit(sc, &sc_ref);
+ audio_sc_release(sc, &sc_ref);
return revents;
}
@@ -1925,7 +1964,7 @@ audiokqfilter(struct file *fp, struct knote *kn)
file = fp->f_audioctx;
dev = file->dev;
- sc = audio_file_enter(file, &sc_ref);
+ sc = audio_sc_acquire_fromfile(file, &sc_ref);
if (sc == NULL)
return EIO;
@@ -1943,7 +1982,7 @@ audiokqfilter(struct file *fp, struct knote *kn)
break;
}
- audio_file_exit(sc, &sc_ref);
+ audio_sc_release(sc, &sc_ref);
return error;
}
@@ -1961,7 +2000,7 @@ audiommap(struct file *fp, off_t *offp, size_t len, int prot, int *flagsp,
file = fp->f_audioctx;
dev = file->dev;
- sc = audio_file_enter(file, &sc_ref);
+ sc = audio_sc_acquire_fromfile(file, &sc_ref);
if (sc == NULL)
return EIO;
@@ -1982,7 +2021,7 @@ audiommap(struct file *fp, off_t *offp, size_t len, int prot, int *flagsp,
break;
}
- audio_file_exit(sc, &sc_ref);
+ audio_sc_release(sc, &sc_ref);
return error;
}
@@ -1998,6 +2037,7 @@ int
audiobellopen(dev_t dev, audio_file_t **filep)
{
struct audio_softc *sc;
+ struct psref sc_ref;
int error;
/* Find the device */
@@ -2005,14 +2045,18 @@ audiobellopen(dev_t dev, audio_file_t **filep)
if (sc == NULL || sc->hw_if == NULL)
return ENXIO;
+ audio_sc_acquire_foropen(sc, &sc_ref);
+
error = audio_exlock_enter(sc);
if (error)
- return error;
+ goto done;
device_active(sc->sc_dev, DVA_SYSTEM);
error = audio_open(dev, sc, FWRITE, 0, curlwp, filep);
audio_exlock_exit(sc);
+done:
+ audio_sc_release(sc, &sc_ref);
return error;
}
@@ -2024,14 +2068,19 @@ audiobellclose(audio_file_t *file)
struct psref sc_ref;
int error;
- sc = audio_file_enter(file, &sc_ref);
- if (sc == NULL)
- return EIO;
-
- error = audio_close(sc, file);
-
- audio_file_exit(sc, &sc_ref);
+ error = 0;
+ /*
+ * audiobellclose() must
+ * - unplug track from the trackmixer if sc exist.
+ * - free all memory objects, regardless of sc.
+ */
+ sc = audio_sc_acquire_fromfile(file, &sc_ref);
+ if (sc) {
+ error = audio_close(sc, file);
+ audio_sc_release(sc, &sc_ref);
+ }
+ /* Free memory objects anyway */
KASSERT(file->ptrack);
audio_track_destroy(file->ptrack);
KASSERT(file->rtrack == NULL);
@@ -2048,7 +2097,7 @@ audiobellsetrate(audio_file_t *file, u_int sample_rate)
struct audio_info ai;
int error;
- sc = audio_file_enter(file, &sc_ref);
+ sc = audio_sc_acquire_fromfile(file, &sc_ref);
if (sc == NULL)
return EIO;
@@ -2062,7 +2111,7 @@ audiobellsetrate(audio_file_t *file, u_int sample_rate)
audio_exlock_exit(sc);
done:
- audio_file_exit(sc, &sc_ref);
+ audio_sc_release(sc, &sc_ref);
return error;
}
@@ -2074,13 +2123,13 @@ audiobellwrite(audio_file_t *file, struct uio *uio)
struct psref sc_ref;
int error;
- sc = audio_file_enter(file, &sc_ref);
+ sc = audio_sc_acquire_fromfile(file, &sc_ref);
if (sc == NULL)
return EIO;
error = audio_write(sc, uio, 0, file);
- audio_file_exit(sc, &sc_ref);
+ audio_sc_release(sc, &sc_ref);
return error;
}
@@ -2104,6 +2153,7 @@ audio_open(dev_t dev, struct audio_softc *sc, int flags, int ifmt,
bool cred_held;
bool hw_opened;
bool rmixer_started;
+ bool inserted;
int fd;
int error;
@@ -2118,6 +2168,7 @@ audio_open(dev_t dev, struct audio_softc *sc, int flags, int ifmt,
cred_held = false;
hw_opened = false;
rmixer_started = false;
+ inserted = false;
af = kmem_zalloc(sizeof(audio_file_t), KM_SLEEP);
af->sc = sc;
@@ -2321,22 +2372,22 @@ audio_open(dev_t dev, struct audio_softc *sc, int flags, int ifmt,
rmixer_started = true;
}
- if (bellfile) {
- *bellfile = af;
- } else {
- error = fd_allocfile(&fp, &fd);
- if (error)
- goto bad;
-
- error = fd_clone(fp, fd, flags, &audio_fileops, af);
- KASSERTMSG(error == EMOVEFD, "error=%d", error);
- }
-
/*
- * Count up finally.
- * Don't fail from here.
+ * This is the last sc_lock section in the function, so we have to
+ * examine sc_dying again before starting the rest tasks. Because
+ * audiodeatch() may have been invoked (and it would set sc_dying)
+ * from the time audioopen() was executed until now. If it happens,
+ * audiodetach() may already have set file->dying for all sc_files
+ * that exist at that point, so that audioopen() must abort without
+ * inserting af to sc_files, in order to keep consistency.
*/
mutex_enter(sc->sc_lock);
+ if (sc->sc_dying) {
+ mutex_exit(sc->sc_lock);
+ goto bad;
+ }
+
+ /* Count up finally */
if (af->ptrack)
sc->sc_popens++;
if (af->rtrack)
@@ -2345,13 +2396,35 @@ audio_open(dev_t dev, struct audio_softc *sc, int flags, int ifmt,
SLIST_INSERT_HEAD(&sc->sc_files, af, entry);
mutex_exit(sc->sc_intr_lock);
mutex_exit(sc->sc_lock);
+ inserted = true;
+
+ if (bellfile) {
+ *bellfile = af;
+ } else {
+ error = fd_allocfile(&fp, &fd);
+ if (error)
+ goto bad;
+
+ error = fd_clone(fp, fd, flags, &audio_fileops, af);
+ KASSERTMSG(error == EMOVEFD, "error=%d", error);
+ }
+
+ /* Be nothing else after fd_clone */
TRACEF(3, af, "done");
return error;
bad:
- if (fp) {
- fd_abort(curproc, fp, fd);
+ if (inserted) {
+ mutex_enter(sc->sc_lock);
+ mutex_enter(sc->sc_intr_lock);
+ SLIST_REMOVE(&sc->sc_files, af, audio_file, entry);
+ mutex_exit(sc->sc_intr_lock);
+ if (af->ptrack)
+ sc->sc_popens--;
+ if (af->rtrack)
+ sc->sc_ropens--;
+ mutex_exit(sc->sc_lock);
}
if (rmixer_started) {