Pull up following revision(s) (requested by isaki in ticket #1156):

sys/dev/audio/audio.c: revision 1.80
	sys/dev/audio/audio.c: revision 1.81

Fix that audio_open() didn't halt the recording mixer correctly
if fd_allocfile() failed, since rev 1.65.

Will fix PR kern/55848.

 -

Rewrite error handling on audio_open().
This also fixes a few resource leaks on error case.
This commit is contained in:
martin 2020-12-19 13:48:27 +00:00
parent 5726130b5c
commit b4c6f2e09f
1 changed files with 57 additions and 28 deletions

View File

@ -1,4 +1,4 @@
/* $NetBSD: audio.c,v 1.28.2.16 2020/06/07 19:04:00 martin Exp $ */
/* $NetBSD: audio.c,v 1.28.2.17 2020/12/19 13:48:27 martin Exp $ */
/*-
* Copyright (c) 2008 The NetBSD Foundation, Inc.
@ -138,7 +138,7 @@
*/
#include <sys/cdefs.h>
__KERNEL_RCSID(0, "$NetBSD: audio.c,v 1.28.2.16 2020/06/07 19:04:00 martin Exp $");
__KERNEL_RCSID(0, "$NetBSD: audio.c,v 1.28.2.17 2020/12/19 13:48:27 martin Exp $");
#ifdef _KERNEL_OPT
#include "audio.h"
@ -2075,6 +2075,9 @@ audio_open(dev_t dev, struct audio_softc *sc, int flags, int ifmt,
audio_file_t *af;
audio_ring_t *hwbuf;
bool fullduplex;
bool cred_held;
bool hw_opened;
bool rmixer_started;
int fd;
int error;
@ -2085,6 +2088,11 @@ audio_open(dev_t dev, struct audio_softc *sc, int flags, int ifmt,
ISDEVSOUND(dev) ? "sound" : "audio",
flags, sc->sc_popens, sc->sc_ropens);
fp = NULL;
cred_held = false;
hw_opened = false;
rmixer_started = false;
af = kmem_zalloc(sizeof(audio_file_t), KM_SLEEP);
af->sc = sc;
af->dev = dev;
@ -2094,7 +2102,7 @@ audio_open(dev_t dev, struct audio_softc *sc, int flags, int ifmt,
af->mode |= AUMODE_RECORD;
if (af->mode == 0) {
error = ENXIO;
goto bad1;
goto bad;
}
fullduplex = (sc->sc_props & AUDIO_PROP_FULLDUPLEX);
@ -2110,7 +2118,7 @@ audio_open(dev_t dev, struct audio_softc *sc, int flags, int ifmt,
if (sc->sc_ropens != 0) {
TRACE(1, "record track already exists");
error = ENODEV;
goto bad1;
goto bad;
}
/* Play takes precedence */
af->mode &= ~AUMODE_RECORD;
@ -2119,7 +2127,7 @@ audio_open(dev_t dev, struct audio_softc *sc, int flags, int ifmt,
if (sc->sc_popens != 0) {
TRACE(1, "play track already exists");
error = ENODEV;
goto bad1;
goto bad;
}
}
}
@ -2166,13 +2174,14 @@ audio_open(dev_t dev, struct audio_softc *sc, int flags, int ifmt,
}
error = audio_file_setinfo(sc, af, &ai);
if (error)
goto bad2;
goto bad;
if (sc->sc_popens + sc->sc_ropens == 0) {
/* First open */
sc->sc_cred = kauth_cred_get();
kauth_cred_hold(sc->sc_cred);
cred_held = true;
if (sc->hw_if->open) {
int hwflags;
@ -2205,8 +2214,16 @@ audio_open(dev_t dev, struct audio_softc *sc, int flags, int ifmt,
mutex_exit(sc->sc_intr_lock);
mutex_exit(sc->sc_lock);
if (error)
goto bad2;
goto bad;
}
/*
* Regardless of whether we called hw_if->open (whether
* hw_if->open exists) or not, we move to the Opened phase
* here. Therefore from this point, we have to call
* hw_if->close (if exists) whenever abort.
* Note that both of hw_if->{open,close} are optional.
*/
hw_opened = true;
/*
* Set speaker mode when a half duplex.
@ -2226,14 +2243,14 @@ audio_open(dev_t dev, struct audio_softc *sc, int flags, int ifmt,
mutex_exit(sc->sc_intr_lock);
mutex_exit(sc->sc_lock);
if (error)
goto bad3;
goto bad;
}
}
} else if (sc->sc_multiuser == false) {
uid_t euid = kauth_cred_geteuid(kauth_cred_get());
if (euid != 0 && euid != kauth_cred_geteuid(sc->sc_cred)) {
error = EPERM;
goto bad2;
goto bad;
}
}
@ -2250,7 +2267,7 @@ audio_open(dev_t dev, struct audio_softc *sc, int flags, int ifmt,
mutex_exit(sc->sc_intr_lock);
mutex_exit(sc->sc_lock);
if (error)
goto bad3;
goto bad;
}
}
/*
@ -2269,18 +2286,24 @@ audio_open(dev_t dev, struct audio_softc *sc, int flags, int ifmt,
mutex_exit(sc->sc_intr_lock);
mutex_exit(sc->sc_lock);
if (error)
goto bad3;
goto bad;
}
mutex_enter(sc->sc_lock);
audio_rmixer_start(sc);
mutex_exit(sc->sc_lock);
rmixer_started = true;
}
if (bellfile == NULL) {
if (bellfile) {
*bellfile = af;
} else {
error = fd_allocfile(&fp, &fd);
if (error)
goto bad3;
goto bad;
error = fd_clone(fp, fd, flags, &audio_fileops, af);
KASSERTMSG(error == EMOVEFD, "error=%d", error);
}
/*
@ -2297,22 +2320,21 @@ audio_open(dev_t dev, struct audio_softc *sc, int flags, int ifmt,
mutex_exit(sc->sc_intr_lock);
mutex_exit(sc->sc_lock);
if (bellfile) {
*bellfile = af;
} else {
error = fd_clone(fp, fd, flags, &audio_fileops, af);
KASSERTMSG(error == EMOVEFD, "error=%d", error);
}
TRACEF(3, af, "done");
return error;
/*
* Since track here is not yet linked to sc_files,
* you can call track_destroy() without sc_intr_lock.
*/
bad3:
if (sc->sc_popens + sc->sc_ropens == 0) {
bad:
if (fp) {
fd_abort(curproc, fp, fd);
}
if (rmixer_started) {
mutex_enter(sc->sc_lock);
audio_rmixer_halt(sc);
mutex_exit(sc->sc_lock);
}
if (hw_opened) {
if (sc->hw_if->close) {
mutex_enter(sc->sc_lock);
mutex_enter(sc->sc_intr_lock);
@ -2321,7 +2343,14 @@ bad3:
mutex_exit(sc->sc_lock);
}
}
bad2:
if (cred_held) {
kauth_cred_free(sc->sc_cred);
}
/*
* Since track here is not yet linked to sc_files,
* you can call track_destroy() without sc_intr_lock.
*/
if (af->rtrack) {
audio_track_destroy(af->rtrack);
af->rtrack = NULL;
@ -2330,7 +2359,7 @@ bad2:
audio_track_destroy(af->ptrack);
af->ptrack = NULL;
}
bad1:
kmem_free(af, sizeof(*af));
return error;
}