Skip to content

Commit

Permalink
Fix for broken clients (Linux after 9/2020) walking open FIDs
Browse files Browse the repository at this point in the history
See this note:
https://marc.info/?l=v9fs-developer&m=164327391912025&w=2

Linux, after this commit,
commit 478ba09edc1f2f2ee27180a06150cb2d1a686f9c (HEAD, refs/bisect/new)
Author: Greg Kurz <groug@kaod.org>
Date:   Wed Sep 23 22:11:45 2020 +0800

Allows walking an open FID.

There is some saving grace in that it will always walk to a different
FID.

The proposed fix, implemented by some servers, is to allow walking
open FIDs as long as they are not the same FID.

E.g.:
         /* we can't walk open files */
-       if fid.opened {
+       if fid.opened && tc.Fid == tc.Newfid {
                 req.RespondError(Ebaduse)
                 return
         }

This fix implements that proposal. It moves the test from doWalk to the message handlers, b/c
the value of fid and newfid don't seem to be available in doWalk, and it seems less
messy to just do the test in the two callers.

Signed-off-by: Ronald G. Minnich <rminnich@gmail.com>
  • Loading branch information
rminnich authored and hugelgupf committed Jan 27, 2022
1 parent 6bedfa0 commit ede3efe
Showing 1 changed file with 18 additions and 7 deletions.
25 changes: 18 additions & 7 deletions p9/handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -1033,13 +1033,6 @@ func doWalk(cs *connState, ref *fidRef, names []string, getattr bool) (qids []QI
}
}

// Has it been opened already?
if _, opened := ref.OpenFlags(); opened {
err = linux.EBUSY
return
}

// Is this an empty list? Handle specially. We don't actually need to
// validate anything since this is always permitted.
if len(names) == 0 {
var sf File // Temporary.
Expand Down Expand Up @@ -1141,6 +1134,15 @@ func (t *twalk) handle(cs *connState) message {
}
defer ref.DecRef()

// Has it been opened already?
// That as OK as long as newFID is different.
// Note this violates the spec, but the Linux client
// does too, so we have little choice.
if _, opened := ref.OpenFlags(); opened && t.fid == t.newFID {
return newErr(linux.EBUSY)
}

// Is this an empty list? Handle specially. We don't actually need to
// Do the walk.
qids, newRef, _, _, err := doWalk(cs, ref, t.Names, false)
if err != nil {
Expand All @@ -1162,6 +1164,15 @@ func (t *twalkgetattr) handle(cs *connState) message {
}
defer ref.DecRef()

// Has it been opened already?
// That as OK as long as newFID is different.
// Note this violates the spec, but the Linux client
// does too, so we have little choice.
if _, opened := ref.OpenFlags(); opened && t.fid == t.newFID {
return newErr(linux.EBUSY)
}

// Is this an empty list? Handle specially. We don't actually need to
// Do the walk.
qids, newRef, valid, attr, err := doWalk(cs, ref, t.Names, true)
if err != nil {
Expand Down

0 comments on commit ede3efe

Please sign in to comment.