[src/netbsd-9]: src/sys/dev/audio Pull up following revision(s) (requested by...

[martin <martin%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/9589d0aa0fcb
branches:  netbsd-9
changeset: 964228:9589d0aa0fcb
user:      martin <martin%NetBSD.org@localhost>
date:      Sat Dec 19 13:48:27 2020 +0000

description:
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.

diffstat:

 sys/dev/audio/audio.c |  91 +++++++++++++++++++++++++++++++++-----------------
 1 files changed, 60 insertions(+), 31 deletions(-)

diffs (218 lines):

diff -r b4fcf4b35a4a -r 9589d0aa0fcb sys/dev/audio/audio.c
--- a/sys/dev/audio/audio.c     Sat Dec 19 13:34:41 2020 +0000
+++ b/sys/dev/audio/audio.c     Sat Dec 19 13:48:27 2020 +0000
@@ -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_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 @@
            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 @@
                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 @@
                        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 @@
                        if (sc->sc_popens != 0) {
                                TRACE(1, "play track already exists");
                                error = ENODEV;
-                               goto bad1;
+                               goto bad;
                        }
                }
        }
@@ -2166,13 +2174,14 @@
        }
        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 @@
                        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 @@
                                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 @@
                        mutex_exit(sc->sc_intr_lock);
                        mutex_exit(sc->sc_lock);
                        if (error)
-                               goto bad3;
+                               goto bad;
                }
        }
        /*
@@ -2269,18 +2286,24 @@
                        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);
-       }
-
-       if (bellfile == NULL) {
+               rmixer_started = true;
+       }
+
+       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 @@
        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 @@
                        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 @@
                audio_track_destroy(af->ptrack);
                af->ptrack = NULL;
        }
-bad1:
+
        kmem_free(af, sizeof(*af));
        return error;
 }