[src/trunk]: src/sys/dev/dkwedge dk(4): Make it clearer that dkopen EROFS bra...

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

details:   https://anonhg.NetBSD.org/src/rev/0b13c772941b
branches:  trunk
changeset: 374727:0b13c772941b
user:      riastradh <riastradh%NetBSD.org@localhost>
date:      Wed May 10 23:08:24 2023 +0000

description:
dk(4): Make it clearer that dkopen EROFS branch doesn't leak.

It looked like we may need to sometimes call dklastclose in error
branch for the case of (flags & ~sc->sc_mode & FWRITE) != 0, but it
is not actually possible to reach that case: if the caller requested
read/write, and the parent is read-only, and it is the first time
we've opened the parent, then dkfirstopen will fail with EROFS so we
never get there.

But this is confusing and it looked like the error branch is wrong,
so let's rearrange the conditional to make it clearer that we cannot
goto out after dkfirstopen has succeeded.  And then assert that the
case cannot happen when we do call dkfirstopen.

diffstat:

 sys/dev/dkwedge/dk.c |  20 +++++++++++++++-----
 1 files changed, 15 insertions(+), 5 deletions(-)

diffs (44 lines):

diff -r 1942f65ee3f8 -r 0b13c772941b sys/dev/dkwedge/dk.c
--- a/sys/dev/dkwedge/dk.c      Wed May 10 22:14:54 2023 +0000
+++ b/sys/dev/dkwedge/dk.c      Wed May 10 23:08:24 2023 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: dk.c,v 1.156 2023/05/09 13:14:14 riastradh Exp $       */
+/*     $NetBSD: dk.c,v 1.157 2023/05/10 23:08:24 riastradh Exp $       */
 
 /*-
  * Copyright (c) 2004, 2005, 2006, 2007 The NetBSD Foundation, Inc.
@@ -30,7 +30,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: dk.c,v 1.156 2023/05/09 13:14:14 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: dk.c,v 1.157 2023/05/10 23:08:24 riastradh Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_dkwedge.h"
@@ -1266,12 +1266,22 @@ dkopen(dev_t dev, int flags, int fmt, st
                error = dkfirstopen(sc, flags);
                if (error)
                        goto out;
-       }
-       KASSERT(sc->sc_mode != 0);
-       if (flags & ~sc->sc_mode & FWRITE) {
+       } else if (flags & ~sc->sc_mode & FWRITE) {
+               /*
+                * The parent is already open, but the previous attempt
+                * to open it read/write failed and fell back to
+                * read-only.  In that case, we assume the medium is
+                * read-only and fail to open the wedge read/write.
+                */
                error = EROFS;
                goto out;
        }
+       KASSERT(sc->sc_mode != 0);
+       KASSERTMSG(sc->sc_mode & FREAD, "%s: sc_mode=%x",
+           device_xname(sc->sc_dev), sc->sc_mode);
+       KASSERTMSG((flags & FWRITE) ? (sc->sc_mode & FWRITE) : 1,
+           "%s: flags=%x sc_mode=%x",
+           device_xname(sc->sc_dev), flags, sc->sc_mode);
        if (fmt == S_IFCHR)
                sc->sc_dk.dk_copenmask |= 1;
        else