Module Name: src
Committed By: martin
Date: Sat Dec 19 13:48:27 UTC 2020
Modified Files:
src/sys/dev/audio [netbsd-9]: audio.c
Log Message:
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.
To generate a diff of this commit:
cvs rdiff -u -r1.28.2.16 -r1.28.2.17 src/sys/dev/audio/audio.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/sys/dev/audio/audio.c
diff -u src/sys/dev/audio/audio.c:1.28.2.16 src/sys/dev/audio/audio.c:1.28.2.17
--- src/sys/dev/audio/audio.c:1.28.2.16 Sun Jun 7 19:04:00 2020
+++ src/sys/dev/audio/audio.c Sat Dec 19 13:48:27 2020
@@ -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
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
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
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
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
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
}
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
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
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
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
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
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;
}
