Forward named pipe creation to the gofer

The backing 9p server must allow named pipe creation, which the runsc
fsgofer currently does not.

There are small changes to the overlay here. GetFile may block when
opening a named pipe, which can cause a deadlock:

1. open(O_RDONLY) -> copyMu.Lock() -> GetFile()
2. open(O_WRONLY) -> copyMu.Lock() -> Deadlock

A named pipe usable for writing must already be on the upper filesystem,
but we are still taking copyMu for write when checking for upper. That
can be changed to a read lock to fix the common case.

However, a named pipe on the lower filesystem would still deadlock in
open(O_WRONLY) when it tries to actually perform copy up (which would
simply return EINVAL). Move the copy up type check before taking copyMu
for write to avoid this.

p9 must be modified, as it was incorrectly removing the file mode when
sending messages on the wire.

PiperOrigin-RevId: 249154033
Change-Id: Id6637130e567b03758130eb6c7cdbc976384b7d6
This commit is contained in:
Michael Pratt
2019-05-20 16:53:08 -07:00
committed by Shentubot
parent 6588427451
commit 80cc2c78e5
9 changed files with 60 additions and 36 deletions
+7 -7
View File
@@ -533,18 +533,18 @@ func (c *clientFile) Link(target File, newname string) error {
}
// Mknod implements File.Mknod.
func (c *clientFile) Mknod(name string, permissions FileMode, major uint32, minor uint32, uid UID, gid GID) (QID, error) {
func (c *clientFile) Mknod(name string, mode FileMode, major uint32, minor uint32, uid UID, gid GID) (QID, error) {
if atomic.LoadUint32(&c.closed) != 0 {
return QID{}, syscall.EBADF
}
msg := Tmknod{
Directory: c.fid,
Name: name,
Permissions: permissions,
Major: major,
Minor: minor,
GID: NoGID,
Directory: c.fid,
Name: name,
Mode: mode,
Major: major,
Minor: minor,
GID: NoGID,
}
if versionSupportsTucreation(c.client.version) {
+1 -1
View File
@@ -170,7 +170,7 @@ type File interface {
// Mknod makes a new device node.
//
// On the server, Mknod has a write concurrency guarantee.
Mknod(name string, permissions FileMode, major uint32, minor uint32, uid UID, gid GID) (QID, error)
Mknod(name string, mode FileMode, major uint32, minor uint32, uid UID, gid GID) (QID, error)
// Rename renames the file.
//
+1 -1
View File
@@ -768,7 +768,7 @@ func (t *Tmknod) do(cs *connState, uid UID) (*Rmknod, error) {
}
// Do the mknod.
qid, err = ref.file.Mknod(t.Name, t.Permissions, t.Major, t.Minor, uid, t.GID)
qid, err = ref.file.Mknod(t.Name, t.Mode, t.Major, t.Minor, uid, t.GID)
return err
}); err != nil {
return nil, err
+1 -1
View File
@@ -252,7 +252,7 @@ func (l *local) Link(target p9.File, newname string) error {
// Mknod implements p9.File.Mknod.
//
// Not implemented.
func (l *local) Mknod(name string, permissions p9.FileMode, major uint32, minor uint32, _ p9.UID, _ p9.GID) (p9.QID, error) {
func (l *local) Mknod(name string, mode p9.FileMode, major uint32, minor uint32, _ p9.UID, _ p9.GID) (p9.QID, error) {
return p9.QID{}, syscall.ENOSYS
}
+5 -5
View File
@@ -1163,8 +1163,8 @@ type Tmknod struct {
// Name is the device name.
Name string
// Permissions are the device permissions.
Permissions FileMode
// Mode is the device mode and permissions.
Mode FileMode
// Major is the device major number.
Major uint32
@@ -1180,7 +1180,7 @@ type Tmknod struct {
func (t *Tmknod) Decode(b *buffer) {
t.Directory = b.ReadFID()
t.Name = b.ReadString()
t.Permissions = b.ReadPermissions()
t.Mode = b.ReadFileMode()
t.Major = b.Read32()
t.Minor = b.Read32()
t.GID = b.ReadGID()
@@ -1190,7 +1190,7 @@ func (t *Tmknod) Decode(b *buffer) {
func (t *Tmknod) Encode(b *buffer) {
b.WriteFID(t.Directory)
b.WriteString(t.Name)
b.WritePermissions(t.Permissions)
b.WriteFileMode(t.Mode)
b.Write32(t.Major)
b.Write32(t.Minor)
b.WriteGID(t.GID)
@@ -1203,7 +1203,7 @@ func (*Tmknod) Type() MsgType {
// String implements fmt.Stringer.
func (t *Tmknod) String() string {
return fmt.Sprintf("Tmknod{DirectoryFID: %d, Name: %s, Permissions: 0o%o, Major: %d, Minor: %d, GID: %d}", t.Directory, t.Name, t.Permissions, t.Major, t.Minor, t.GID)
return fmt.Sprintf("Tmknod{DirectoryFID: %d, Name: %s, Mode: 0o%o, Major: %d, Minor: %d, GID: %d}", t.Directory, t.Name, t.Mode, t.Major, t.Minor, t.GID)
}
// Rmknod is a mknod response.
+12 -12
View File
@@ -142,12 +142,12 @@ func TestEncodeDecode(t *testing.T) {
QID: QID{Type: 1},
},
&Tmknod{
Directory: 1,
Name: "a",
Permissions: 2,
Major: 3,
Minor: 4,
GID: 5,
Directory: 1,
Name: "a",
Mode: 2,
Major: 3,
Minor: 4,
GID: 5,
},
&Rmknod{
QID: QID{Type: 1},
@@ -349,12 +349,12 @@ func TestEncodeDecode(t *testing.T) {
},
&Tumknod{
Tmknod: Tmknod{
Directory: 1,
Name: "a",
Permissions: 2,
Major: 3,
Minor: 4,
GID: 5,
Directory: 1,
Name: "a",
Mode: 2,
Major: 3,
Minor: 4,
GID: 5,
},
UID: 6,
},
+14 -4
View File
@@ -113,13 +113,13 @@ func copyUpLockedForRename(ctx context.Context, d *Dirent) error {
// Did we race with another copy up or does there
// already exist something in the upper filesystem
// for d?
d.Inode.overlay.copyMu.Lock()
d.Inode.overlay.copyMu.RLock()
if d.Inode.overlay.upper != nil {
d.Inode.overlay.copyMu.Unlock()
d.Inode.overlay.copyMu.RUnlock()
// Done, d is in the upper filesystem.
return nil
}
d.Inode.overlay.copyMu.Unlock()
d.Inode.overlay.copyMu.RUnlock()
// Find the next component to copy up. We will work our way
// down to the last component of d and finally copy it.
@@ -155,6 +155,14 @@ func findNextCopyUp(ctx context.Context, d *Dirent) *Dirent {
}
func doCopyUp(ctx context.Context, d *Dirent) error {
// Fail fast on Inode types we won't be able to copy up anyways. These
// Inodes may block in GetFile while holding copyMu for reading. If we
// then try to take copyMu for writing here, we'd deadlock.
t := d.Inode.overlay.lower.StableAttr.Type
if t != RegularFile && t != Directory && t != Symlink {
return syserror.EINVAL
}
// Wait to get exclusive access to the upper Inode.
d.Inode.overlay.copyMu.Lock()
defer d.Inode.overlay.copyMu.Unlock()
@@ -177,6 +185,8 @@ func doCopyUp(ctx context.Context, d *Dirent) error {
// - parent.Inode.overlay.upper must be non-nil.
// - next.Inode.overlay.copyMu must be locked writable.
// - next.Inode.overlay.lower must be non-nil.
// - next.Inode.overlay.lower.StableAttr.Type must be RegularFile, Directory,
// or Symlink.
// - upper filesystem must support setting file ownership and timestamps.
func copyUpLocked(ctx context.Context, parent *Dirent, next *Dirent) error {
// Extract the attributes of the file we wish to copy.
@@ -239,7 +249,7 @@ func copyUpLocked(ctx context.Context, parent *Dirent, next *Dirent) error {
childUpperInode = childUpper.Inode
default:
return syserror.EINVAL
panic(fmt.Sprintf("copy up of invalid type %v on %+v", next.Inode.StableAttr.Type, next))
}
// Bring file attributes up to date. This does not include size, which will be
+16 -4
View File
@@ -282,10 +282,22 @@ func (i *inodeOperations) Bind(ctx context.Context, dir *fs.Inode, name string,
return childDir, nil
}
// CreateFifo implements fs.InodeOperations.CreateFifo. Gofer nodes do not support the
// creation of fifos and always returns EPERM.
func (*inodeOperations) CreateFifo(context.Context, *fs.Inode, string, fs.FilePermissions) error {
return syscall.EPERM
// CreateFifo implements fs.InodeOperations.CreateFifo.
func (i *inodeOperations) CreateFifo(ctx context.Context, dir *fs.Inode, name string, perm fs.FilePermissions) error {
if len(name) > maxFilenameLen {
return syserror.ENAMETOOLONG
}
owner := fs.FileOwnerFromContext(ctx)
mode := p9.FileMode(perm.LinuxMode()) | p9.ModeNamedPipe
// N.B. FIFOs use major/minor numbers 0.
if _, err := i.fileState.file.mknod(ctx, name, mode, 0, 0, p9.UID(owner.UID), p9.GID(owner.GID)); err != nil {
return err
}
i.touchModificationAndStatusChangeTime(ctx, dir)
return nil
}
// Remove implements InodeOperations.Remove.
+3 -1
View File
@@ -161,7 +161,9 @@ type InodeOperations interface {
BoundEndpoint(inode *Inode, path string) transport.BoundEndpoint
// GetFile returns a new open File backed by a Dirent and FileFlags.
// It may block as long as it is done with ctx.
//
// Special Inode types may block using ctx.Sleeper. RegularFiles,
// Directories, and Symlinks must not block (see doCopyUp).
//
// The returned File will uniquely back an application fd.
GetFile(ctx context.Context, d *Dirent, flags FileFlags) (*File, error)