Allow 9ns clones of chans opened with O_PATH
authorBarret Rhoden <brho@cs.berkeley.edu>
Wed, 16 Sep 2015 16:19:54 +0000 (12:19 -0400)
committerBarret Rhoden <brho@cs.berkeley.edu>
Mon, 28 Sep 2015 19:14:00 +0000 (15:14 -0400)
I've heard it's a bad idea to allow clones of opened chans.  I'd like to
do it for O_PATH (no I/O allowed) chans.

I don't know the reasons for not allowing the clone.  One thing is that
it appears that devices up their refcnts or otherwise keep objects
around based on an open - which stands to reason: even after you unlink,
open objects stay around.  When we devclone, we might be messing up
something where the device does not know about the cloned chan.
However, the new chan is not COPEN, so it's like one of what I call the
"kernel internal" chans.

This relates to the decision to have O_PATH actually call a device's
open.  The choices seem to be:

1) Don't call open.  The user has an FD that points to a chan that may
or may not point to an actual object (not super comfortable with that,
but it might be fine.  This is my original SYS_walk).  Devclone doesn't
allow any opened chans to be cloned (and therefore walked from), and no
O_PATHs are actually opened.  You can only openat() from one of these
O_PATHs.  FD taps (e.g. ipfdtap) will need to grab references to keep
the conversation alive (and there may be races/issues of tapping old
conversations).

2) Call open.  The user will only ever have FDs that point to opened,
possibly-refcnted objects (this might be unnecessary).  The device is
involved in an O_PATH open.  openat() can be called on any FD (we may
allow from non-dirs).  The downside is that we're cloning an open chan,
which may have bad side effects.

For now, I'll go with option 2.

kern/src/ns/dev.c

index 7c7b324..4f6b013 100644 (file)
@@ -118,11 +118,23 @@ struct chan *devclone(struct chan *c)
 {
        struct chan *nc;
 
-       if (c->flag & COPEN)
-               panic("clone of open file type %s\n", devtab[c->type].name);
+       /* In plan 9, you couldn't clone an open chan.  We're allowing it, possibly
+        * foolishly.  The new chan is a non-open, "kernel internal" chan.  Note
+        * that c->flag isn't set, for instance.  c->mode is, which might be a
+        * problem.  The newchan should eventually have a device's open called on
+        * it, at which point it upgrades from a kernel internal chan to one that
+        * can refer to an object in the device (e.g. grab a refcnt on a
+        * conversation in #ip).
+        *
+        * Either we allow devclones of open chans, or O_PATH walks do not open a
+        * file.  It's nice to allow the device to do something for O_PATH, but
+        * perhaps that is not critical.  However, if we can't clone an opened chan,
+        * then we can *only* openat from an FD that is O_PATH, which is not the
+        * spec (and not as useful). */
+       if ((c->flag & COPEN) && !(c->flag & O_PATH))
+               panic("clone of non-O_PATH open file type %s\n", devtab[c->type].name);
 
        nc = newchan();
-
        nc->type = c->type;
        nc->dev = c->dev;
        nc->mode = c->mode;