diff options
| author | isaki <isaki@NetBSD.org> | 2021-02-09 12:36:34 +0000 |
|---|---|---|
| committer | isaki <isaki@NetBSD.org> | 2021-02-09 12:36:34 +0000 |
| commit | bef9041a7edd54055810d1d596fedb4e1320b43d (patch) | |
| tree | 40cebd166a10cb8b2b1790a212279d04ed9c0ac3 /sys/dev/audio | |
| parent | fdee3101130fc239adc09b14d324eda8290d9082 (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.c | 179 |
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) { |
