Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit 83c538e

Browse files
tranji-cloudgvisor-bot
authored andcommittedJun 5, 2025·
Hard links Support Phase 1: Gofer Client dentry/inode refactor
This commit does the following: - Decouples dentry fields and the file's metadata (inode fields) - Embeds the inode pointer into dentry struct {} - Removed impl from dentry struct {} and extended inode struct with back-end implementation - Refactor Gofer client fs to access the newly created inode pointer - Refactor dentry_impl.go to inode_impl.go - Refactor {directfs/lisafs}_dentry to {directfs/lisafs}_inode PiperOrigin-RevId: 750748327
1 parent f2d2ff7 commit 83c538e

File tree

18 files changed

+1626
-1480
lines changed

18 files changed

+1626
-1480
lines changed
 

‎pkg/sentry/fsimpl/gofer/BUILD

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,17 @@ package(default_applicable_licenses = ["//:license"])
55

66
licenses(["notice"])
77

8+
go_template_instance(
9+
name = "inode_refs",
10+
out = "inode_refs.go",
11+
package = "gofer",
12+
prefix = "inode",
13+
template = "//pkg/refs:refs_template",
14+
types = {
15+
"T": "inode",
16+
},
17+
)
18+
819
go_template_instance(
920
name = "string_list",
1021
out = "string_list.go",
@@ -56,16 +67,17 @@ go_template_instance(
5667
go_library(
5768
name = "gofer",
5869
srcs = [
59-
"dentry_impl.go",
6070
"dentry_list.go",
61-
"directfs_dentry.go",
71+
"directfs_inode.go",
6272
"directory.go",
6373
"filesystem.go",
6474
"fstree.go",
6575
"gofer.go",
6676
"handle.go",
6777
"host_named_pipe.go",
68-
"lisafs_dentry.go",
78+
"inode_impl.go",
79+
"inode_refs.go",
80+
"lisafs_inode.go",
6981
"regular_file.go",
7082
"revalidate.go",
7183
"save_restore.go",
@@ -112,6 +124,7 @@ go_library(
112124
"//pkg/sentry/socket/unix/transport",
113125
"//pkg/sentry/usage",
114126
"//pkg/sentry/vfs",
127+
"//pkg/sentry/vfs/memxattr",
115128
"//pkg/sync",
116129
"//pkg/syserr",
117130
"//pkg/unet",

‎pkg/sentry/fsimpl/gofer/directfs_dentry.go renamed to ‎pkg/sentry/fsimpl/gofer/directfs_inode.go

Lines changed: 186 additions & 172 deletions
Large diffs are not rendered by default.

‎pkg/sentry/fsimpl/gofer/directory.go

Lines changed: 45 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ import (
3030
)
3131

3232
func (d *dentry) isDir() bool {
33-
return d.fileType() == linux.S_IFDIR
33+
return d.inode.fileType() == linux.S_IFDIR
3434
}
3535

3636
// cacheNewChildLocked will cache the new child dentry, and will panic if a
@@ -51,7 +51,7 @@ func (d *dentry) isDir() bool {
5151
// +checklocks:d.childrenMu
5252
func (d *dentry) cacheNewChildLocked(child *dentry, name string) {
5353
d.IncRef() // reference held by child on its parent
54-
genericSetParentAndName(d.fs, child, d, name)
54+
genericSetParentAndName(d.inode.fs, child, d, name)
5555
if d.children == nil {
5656
d.children = make(map[string]*dentry)
5757
} else if c, ok := d.children[name]; ok {
@@ -74,10 +74,10 @@ func (d *dentry) cacheNewChildLocked(child *dentry, name string) {
7474
// +checklocks:d.childrenMu
7575
func (d *dentry) cacheNegativeLookupLocked(name string) {
7676
// Don't cache negative lookups if InteropModeShared is in effect (since
77-
// this makes remote lookup unavoidable), or if d.isSynthetic() (in which
77+
// this makes remote lookup unavoidable), or if d.inode.isSynthetic() (in which
7878
// case the only files in the directory are those for which a dentry exists
7979
// in d.children). Instead, just delete any previously-cached dentry.
80-
if d.fs.opts.interop == InteropModeShared || d.isSynthetic() {
80+
if d.inode.fs.opts.interop == InteropModeShared || d.inode.isSynthetic() {
8181
delete(d.children, name)
8282
return
8383
}
@@ -124,34 +124,41 @@ type createSyntheticOpts struct {
124124
// newSyntheticDentry creates a synthetic file with the given name.
125125
func (fs *filesystem) newSyntheticDentry(opts *createSyntheticOpts) *dentry {
126126
now := fs.clock.Now().Nanoseconds()
127+
ino := fs.nextIno()
128+
inodePtr := fs.findInode(ino)
129+
if inodePtr == nil {
130+
inodePtr = new(inode)
131+
inodePtr.init(fs, ino, nil)
132+
}
133+
127134
child := &dentry{
128-
refs: atomicbitops.FromInt64(1), // held by parent.
129-
fs: fs,
130-
ino: fs.nextIno(),
131-
mode: atomicbitops.FromUint32(uint32(opts.mode)),
132-
uid: atomicbitops.FromUint32(uint32(opts.kuid)),
133-
gid: atomicbitops.FromUint32(uint32(opts.kgid)),
134-
blockSize: atomicbitops.FromUint32(hostarch.PageSize), // arbitrary
135-
atime: atomicbitops.FromInt64(now),
136-
mtime: atomicbitops.FromInt64(now),
137-
ctime: atomicbitops.FromInt64(now),
138-
btime: atomicbitops.FromInt64(now),
139-
readFD: atomicbitops.FromInt32(-1),
140-
writeFD: atomicbitops.FromInt32(-1),
141-
mmapFD: atomicbitops.FromInt32(-1),
142-
nlink: atomicbitops.FromUint32(2),
135+
refs: atomicbitops.FromInt64(1), // held by parent.
136+
inode: inodePtr,
143137
}
138+
139+
child.inode.mode.Store(uint32(opts.mode))
140+
child.inode.uid.Store(uint32(opts.kuid))
141+
child.inode.gid.Store(uint32(opts.kgid))
142+
child.inode.blockSize.Store(hostarch.PageSize)
143+
child.inode.atime.Store(now)
144+
child.inode.mtime.Store(now)
145+
child.inode.ctime.Store(now)
146+
child.inode.btime.Store(now)
147+
child.inode.nlink.Store(2)
148+
child.inode.readFD.Store(-1)
149+
child.inode.writeFD.Store(-1)
150+
child.inode.mmapFD.Store(-1)
144151
switch opts.mode.FileType() {
145152
case linux.S_IFDIR:
146153
// Nothing else needs to be done.
147154
case linux.S_IFSOCK:
148-
child.endpoint = opts.endpoint
155+
child.inode.endpoint = opts.endpoint
149156
case linux.S_IFIFO:
150-
child.pipe = opts.pipe
157+
child.inode.pipe = opts.pipe
151158
default:
152159
panic(fmt.Sprintf("failed to create synthetic file of unrecognized type: %v", opts.mode.FileType()))
153160
}
154-
child.init(nil /* impl */)
161+
child.init()
155162
return child
156163
}
157164

@@ -192,7 +199,7 @@ func (fd *directoryFD) IterDirents(ctx context.Context, cb vfs.IterDirentsCallba
192199
fd.dirents = ds
193200
}
194201

195-
if d.cachedMetadataAuthoritative() {
202+
if d.inode.cachedMetadataAuthoritative() {
196203
d.touchAtime(fd.vfsfd.Mount())
197204
}
198205

@@ -224,8 +231,8 @@ func (d *dentry) getDirents(ctx context.Context) ([]vfs.Dirent, error) {
224231

225232
// filesystem.renameMu is needed for d.parent, and must be locked before
226233
// d.opMu.
227-
d.fs.renameMu.RLock()
228-
defer d.fs.renameMu.RUnlock()
234+
d.inode.fs.renameMu.RLock()
235+
defer d.inode.fs.renameMu.RUnlock()
229236
d.opMu.RLock()
230237
defer d.opMu.RUnlock()
231238

@@ -248,33 +255,33 @@ func (d *dentry) getDirents(ctx context.Context) ([]vfs.Dirent, error) {
248255
{
249256
Name: ".",
250257
Type: linux.DT_DIR,
251-
Ino: uint64(d.ino),
258+
Ino: uint64(d.inode.ino),
252259
NextOff: 1,
253260
},
254261
{
255262
Name: "..",
256-
Type: uint8(parent.mode.Load() >> 12),
257-
Ino: uint64(parent.ino),
263+
Type: uint8(parent.inode.mode.Load() >> 12),
264+
Ino: uint64(parent.inode.ino),
258265
NextOff: 2,
259266
},
260267
}
261268
var realChildren map[string]struct{}
262-
if !d.isSynthetic() {
263-
if d.syntheticChildren != 0 && d.fs.opts.interop == InteropModeShared {
269+
if !d.inode.isSynthetic() {
270+
if d.syntheticChildren != 0 && d.inode.fs.opts.interop == InteropModeShared {
264271
// Record the set of children d actually has so that we don't emit
265272
// duplicate entries for synthetic children.
266273
realChildren = make(map[string]struct{})
267274
}
268-
d.handleMu.RLock()
269-
if !d.isReadHandleOk() {
275+
d.inode.handleMu.RLock()
276+
if !d.inode.isReadHandleOk() {
270277
// This should not be possible because a readable handle should
271278
// have been opened when the calling directoryFD was opened.
272279
panic("gofer.dentry.getDirents called without a readable handle")
273280
}
274281
err := d.getDirentsLocked(ctx, func(name string, key inoKey, dType uint8) {
275282
dirent := vfs.Dirent{
276283
Name: name,
277-
Ino: d.fs.inoFromKey(key),
284+
Ino: d.inode.fs.inoFromKey(key),
278285
NextOff: int64(len(dirents) + 1),
279286
Type: dType,
280287
}
@@ -283,7 +290,7 @@ func (d *dentry) getDirents(ctx context.Context) ([]vfs.Dirent, error) {
283290
realChildren[name] = struct{}{}
284291
}
285292
})
286-
d.handleMu.RUnlock()
293+
d.inode.handleMu.RUnlock()
287294
if err != nil {
288295
return nil, err
289296
}
@@ -292,22 +299,22 @@ func (d *dentry) getDirents(ctx context.Context) ([]vfs.Dirent, error) {
292299
// Emit entries for synthetic children.
293300
if d.syntheticChildren != 0 {
294301
for _, child := range d.children {
295-
if child == nil || !child.isSynthetic() {
302+
if child == nil || !child.inode.isSynthetic() {
296303
continue
297304
}
298305
if _, ok := realChildren[child.name]; ok {
299306
continue
300307
}
301308
dirents = append(dirents, vfs.Dirent{
302309
Name: child.name,
303-
Type: uint8(child.mode.Load() >> 12),
304-
Ino: uint64(child.ino),
310+
Type: uint8(child.inode.mode.Load() >> 12),
311+
Ino: uint64(child.inode.ino),
305312
NextOff: int64(len(dirents) + 1),
306313
})
307314
}
308315
}
309316
// Cache dirents for future directoryFDs if permitted.
310-
if d.cachedMetadataAuthoritative() {
317+
if d.inode.cachedMetadataAuthoritative() {
311318
d.dirents = dirents
312319
d.childrenSet = make(map[string]struct{}, len(dirents))
313320
for _, dirent := range d.dirents {

‎pkg/sentry/fsimpl/gofer/filesystem.go

Lines changed: 99 additions & 96 deletions
Large diffs are not rendered by default.

‎pkg/sentry/fsimpl/gofer/gofer.go

Lines changed: 524 additions & 466 deletions
Large diffs are not rendered by default.

‎pkg/sentry/fsimpl/gofer/gofer_test.go

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -27,37 +27,47 @@ import (
2727
func TestDestroyIdempotent(t *testing.T) {
2828
ctx := contexttest.Context(t)
2929
fs := filesystem{
30-
mf: pgalloc.MemoryFileFromContext(ctx),
31-
inoByKey: make(map[inoKey]uint64),
32-
clock: ktime.RealtimeClockFromContext(ctx),
30+
mf: pgalloc.MemoryFileFromContext(ctx),
31+
inoByKey: make(map[inoKey]uint64),
32+
inodeByIno: make(map[uint64]*inode),
33+
clock: ktime.RealtimeClockFromContext(ctx),
3334
// Test relies on no dentry being held in the cache.
3435
dentryCache: &dentryCache{maxCachedDentries: 0},
3536
client: &lisafs.Client{},
3637
}
3738

38-
parentInode := lisafs.Inode{
39+
parentRemoteInode := lisafs.Inode{
3940
ControlFD: 1,
4041
Stat: linux.Statx{
4142
Mask: linux.STATX_TYPE | linux.STATX_MODE,
4243
Mode: linux.S_IFDIR | 0666,
4344
},
4445
}
45-
parent, err := fs.newLisafsDentry(ctx, &parentInode)
46+
parentInode, err := fs.newLisafsInode(ctx, &parentRemoteInode)
4647
if err != nil {
47-
t.Fatalf("fs.newLisafsDentry(): %v", err)
48+
t.Fatalf("fs.newLisafsInode(): %v", err)
49+
}
50+
parent, err := fs.newDentry(parentInode)
51+
if err != nil {
52+
t.Fatalf("fs.newDentry(): %v", err)
4853
}
4954

50-
childInode := lisafs.Inode{
55+
childRemoteInode := lisafs.Inode{
5156
ControlFD: 2,
5257
Stat: linux.Statx{
5358
Mask: linux.STATX_TYPE | linux.STATX_MODE | linux.STATX_SIZE,
5459
Mode: linux.S_IFREG | 0666,
5560
Size: 0,
5661
},
5762
}
58-
child, err := fs.newLisafsDentry(ctx, &childInode)
63+
childInode, err := fs.newLisafsInode(ctx, &childRemoteInode)
64+
if err != nil {
65+
t.Fatalf("fs.newLisafsInode(): %v", err)
66+
}
67+
68+
child, err := fs.newDentry(childInode)
5969
if err != nil {
60-
t.Fatalf("fs.newLisafsDentry(): %v", err)
70+
t.Fatalf("fs.newDentry(): %v", err)
6171
}
6272
parent.opMu.Lock()
6373
parent.childrenMu.Lock()

‎pkg/sentry/fsimpl/gofer/dentry_impl.go renamed to ‎pkg/sentry/fsimpl/gofer/inode_impl.go

Lines changed: 229 additions & 229 deletions
Large diffs are not rendered by default.

‎pkg/sentry/fsimpl/gofer/lisafs_dentry.go renamed to ‎pkg/sentry/fsimpl/gofer/lisafs_inode.go

Lines changed: 166 additions & 142 deletions
Large diffs are not rendered by default.

‎pkg/sentry/fsimpl/gofer/regular_file.go

Lines changed: 160 additions & 160 deletions
Large diffs are not rendered by default.

‎pkg/sentry/fsimpl/gofer/revalidate.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ func (fs *filesystem) revalidateOne(ctx context.Context, vfsObj *vfs.VirtualFile
6767
// Skip revalidation for interop mode different than InteropModeShared or
6868
// if the parent is synthetic (child must be synthetic too, but it cannot be
6969
// replaced without first replacing the parent).
70-
if parent.cachedMetadataAuthoritative() {
70+
if parent.inode.cachedMetadataAuthoritative() {
7171
return nil
7272
}
7373

@@ -184,7 +184,7 @@ func (fs *filesystem) revalidateStep(ctx context.Context, rp resolvingPath, d *d
184184
state.add(child)
185185

186186
// Symlink must be resolved before continuing with revalidation.
187-
if child.isSymlink() {
187+
if child.inode.isSymlink() {
188188
return nil, errRevalidationStepDone{}
189189
}
190190

@@ -211,7 +211,7 @@ func (d *dentry) invalidate(ctx context.Context, vfsObj *vfs.VirtualFilesystem,
211211
if child := parent.children[d.name]; child == d {
212212
// Invalidate dentry so it gets reloaded next time it's accessed.
213213
delete(parent.children, d.name)
214-
if d.isSynthetic() {
214+
if d.inode.isSynthetic() {
215215
// Normally we don't mark invalidated dentries as deleted since
216216
// they may still exist (but at a different path), and also for
217217
// consistency with Linux. However, synthetic files are
@@ -247,7 +247,7 @@ func (d *dentry) invalidate(ctx context.Context, vfsObj *vfs.VirtualFilesystem,
247247
// now. (The same would apply to racy replacement by
248248
// filesystem.RenameAt(), but we can't race with rename since renameMu
249249
// has been locked since entering filesystem.revalidatePath().)
250-
if removed && (d.isSynthetic() || d.endpoint != nil) {
250+
if removed && (d.inode.isSynthetic() || d.inode.endpoint != nil) {
251251
d.decRefNoCaching()
252252
}
253253

@@ -282,7 +282,7 @@ func (d *dentry) disownAllChildrenForInvalidation(ctx context.Context, vfsObj *v
282282
for name, child := range d.children {
283283
children = append(children, child)
284284
delete(d.children, name)
285-
if child.isSynthetic() {
285+
if child.inode.isSynthetic() {
286286
child.deleteSynthetic(d, ds)
287287
}
288288
}

‎pkg/sentry/fsimpl/gofer/save_restore.go

Lines changed: 77 additions & 75 deletions
Large diffs are not rendered by default.

‎pkg/sentry/fsimpl/gofer/socket.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,8 @@ import (
2424
"gvisor.dev/gvisor/pkg/waiter"
2525
)
2626

27-
func (d *dentry) isSocket() bool {
28-
return d.fileType() == linux.S_IFSOCK
27+
func (i *inode) isSocket() bool {
28+
return i.fileType() == linux.S_IFSOCK
2929
}
3030

3131
func isSocketTypeSupported(sockType linux.SockType) bool {
@@ -103,9 +103,9 @@ func (e *endpoint) UnidirectionalConnect(ctx context.Context, opts transport.Uni
103103
}
104104

105105
func (e *endpoint) newConnectedEndpoint(ctx context.Context, sockType linux.SockType, queue *waiter.Queue, opts transport.UnixSocketOpts) (*transport.SCMConnectedEndpoint, *syserr.Error) {
106-
e.dentry.fs.renameMu.RLock()
106+
e.dentry.inode.fs.renameMu.RLock()
107107
hostSockFD, err := e.dentry.connect(ctx, sockType)
108-
e.dentry.fs.renameMu.RUnlock()
108+
e.dentry.inode.fs.renameMu.RUnlock()
109109
if err != nil {
110110
return nil, syserr.ErrConnectionRefused
111111
}

‎pkg/sentry/fsimpl/gofer/special_file.go

Lines changed: 35 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ type specialFileFD struct {
9292
}
9393

9494
func newSpecialFileFD(h handle, mnt *vfs.Mount, d *dentry, flags uint32) (*specialFileFD, error) {
95-
ftype := d.fileType()
95+
ftype := d.inode.fileType()
9696
seekable := ftype == linux.S_IFREG || ftype == linux.S_IFCHR || ftype == linux.S_IFBLK
9797
haveQueue := (ftype == linux.S_IFIFO || ftype == linux.S_IFSOCK || ftype == linux.S_IFCHR) && h.fd >= 0
9898
fd := &specialFileFD{
@@ -101,7 +101,7 @@ func newSpecialFileFD(h handle, mnt *vfs.Mount, d *dentry, flags uint32) (*speci
101101
seekable: seekable,
102102
haveQueue: haveQueue,
103103
}
104-
fd.LockFD.Init(&d.locks)
104+
fd.LockFD.Init(&d.inode.locks)
105105
if haveQueue {
106106
if err := fdnotifier.AddFD(h.fd, &fd.queue); err != nil {
107107
return nil, err
@@ -117,10 +117,10 @@ func newSpecialFileFD(h handle, mnt *vfs.Mount, d *dentry, flags uint32) (*speci
117117
}
118118
return nil, err
119119
}
120-
d.fs.syncMu.Lock()
121-
d.fs.specialFileFDs.PushBack(fd)
122-
d.fs.syncMu.Unlock()
123-
if fd.vfsfd.IsWritable() && (d.mode.Load()&0111 != 0) {
120+
d.inode.fs.syncMu.Lock()
121+
d.inode.fs.specialFileFDs.PushBack(fd)
122+
d.inode.fs.syncMu.Unlock()
123+
if fd.vfsfd.IsWritable() && (d.inode.mode.Load()&0111 != 0) {
124124
metric.SuspiciousOperationsMetric.Increment(&metric.SuspiciousOperationsTypeOpenedWriteExecuteFile)
125125
}
126126
if h.fd >= 0 {
@@ -229,7 +229,7 @@ func (fd *specialFileFD) PRead(ctx context.Context, dst usermem.IOSequence, offs
229229
return 0, linuxerr.EOPNOTSUPP
230230
}
231231

232-
if d := fd.dentry(); d.cachedMetadataAuthoritative() {
232+
if d := fd.dentry(); d.inode.cachedMetadataAuthoritative() {
233233
d.touchAtime(fd.vfsfd.Mount())
234234
}
235235

@@ -304,20 +304,20 @@ func (fd *specialFileFD) pwrite(ctx context.Context, src usermem.IOSequence, off
304304
// If the regular file fd was opened with O_APPEND, make sure the file
305305
// size is updated. There is a possible race here if size is modified
306306
// externally after metadata cache is updated.
307-
if fd.vfsfd.StatusFlags()&linux.O_APPEND != 0 && !d.cachedMetadataAuthoritative() {
308-
if err := d.updateMetadata(ctx); err != nil {
307+
if fd.vfsfd.StatusFlags()&linux.O_APPEND != 0 && !d.inode.cachedMetadataAuthoritative() {
308+
if err := d.inode.updateMetadata(ctx); err != nil {
309309
return 0, offset, err
310310
}
311311
}
312312

313313
// We need to hold the metadataMu *while* writing to a regular file.
314-
d.metadataMu.Lock()
315-
defer d.metadataMu.Unlock()
314+
d.inode.metadataMu.Lock()
315+
defer d.inode.metadataMu.Unlock()
316316

317317
// Set offset to file size if the regular file was opened with O_APPEND.
318318
if fd.vfsfd.StatusFlags()&linux.O_APPEND != 0 {
319-
// Holding d.metadataMu is sufficient for reading d.size.
320-
offset = int64(d.size.RacyLoad())
319+
// Holding d.inode.metadataMu is sufficient for reading d.inode.size.
320+
offset = int64(d.inode.size.RacyLoad())
321321
}
322322
limit, err := vfs.CheckLimit(ctx, offset, src.NumBytes())
323323
if err != nil {
@@ -326,7 +326,7 @@ func (fd *specialFileFD) pwrite(ctx context.Context, src usermem.IOSequence, off
326326
src = src.TakeFirst64(limit)
327327
}
328328

329-
if d.cachedMetadataAuthoritative() {
329+
if d.inode.cachedMetadataAuthoritative() {
330330
if fd.isRegularFile {
331331
d.touchCMtimeLocked()
332332
} else {
@@ -335,17 +335,17 @@ func (fd *specialFileFD) pwrite(ctx context.Context, src usermem.IOSequence, off
335335
}
336336

337337
// handleReadWriter always writes to the remote file. So O_DIRECT is
338-
// effectively always set. Invalidate pages in d.mappings that have been
338+
// effectively always set. Invalidate pages in d.inode.mappings that have been
339339
// written to.
340340
pgstart := hostarch.PageRoundDown(uint64(offset))
341341
pgend, ok := hostarch.PageRoundUp(uint64(offset + src.NumBytes()))
342342
if !ok {
343343
return 0, offset, linuxerr.EINVAL
344344
}
345345
mr := memmap.MappableRange{pgstart, pgend}
346-
d.mapsMu.Lock()
347-
d.mappings.Invalidate(mr, memmap.InvalidateOpts{})
348-
d.mapsMu.Unlock()
346+
d.inode.mapsMu.Lock()
347+
d.inode.mappings.Invalidate(mr, memmap.InvalidateOpts{})
348+
d.inode.mapsMu.Unlock()
349349

350350
rw := getHandleReadWriter(ctx, &fd.handle, offset)
351351
n, err := src.CopyInTo(ctx, rw)
@@ -369,11 +369,11 @@ func (fd *specialFileFD) pwrite(ctx context.Context, src usermem.IOSequence, off
369369
}
370370
// Update file size for regular files.
371371
if fd.isRegularFile {
372-
// d.metadataMu is already locked at this point.
373-
if uint64(offset) > d.size.RacyLoad() {
374-
d.dataMu.Lock()
375-
defer d.dataMu.Unlock()
376-
d.size.Store(uint64(offset))
372+
// d.inode.metadataMu is already locked at this point.
373+
if uint64(offset) > d.inode.size.RacyLoad() {
374+
d.inode.dataMu.Lock()
375+
defer d.inode.dataMu.Unlock()
376+
d.inode.size.Store(uint64(offset))
377377
}
378378
}
379379
return int64(n), offset, err
@@ -444,19 +444,19 @@ func (fd *specialFileFD) ConfigureMMap(ctx context.Context, opts *memmap.MMapOpt
444444
// AddMapping implements memmap.Mappable.AddMapping.
445445
func (fd *specialFileFD) AddMapping(ctx context.Context, ms memmap.MappingSpace, ar hostarch.AddrRange, offset uint64, writable bool) error {
446446
d := fd.dentry()
447-
d.mapsMu.Lock()
448-
defer d.mapsMu.Unlock()
449-
d.mappings.AddMapping(ms, ar, offset, writable)
447+
d.inode.mapsMu.Lock()
448+
defer d.inode.mapsMu.Unlock()
449+
d.inode.mappings.AddMapping(ms, ar, offset, writable)
450450
fd.hostFileMapper.IncRefOn(memmap.MappableRange{offset, offset + uint64(ar.Length())})
451451
return nil
452452
}
453453

454454
// RemoveMapping implements memmap.Mappable.RemoveMapping.
455455
func (fd *specialFileFD) RemoveMapping(ctx context.Context, ms memmap.MappingSpace, ar hostarch.AddrRange, offset uint64, writable bool) {
456456
d := fd.dentry()
457-
d.mapsMu.Lock()
458-
defer d.mapsMu.Unlock()
459-
d.mappings.RemoveMapping(ms, ar, offset, writable)
457+
d.inode.mapsMu.Lock()
458+
defer d.inode.mapsMu.Unlock()
459+
d.inode.mappings.RemoveMapping(ms, ar, offset, writable)
460460
fd.hostFileMapper.DecRefOn(memmap.MappableRange{offset, offset + uint64(ar.Length())})
461461
}
462462

@@ -484,9 +484,9 @@ func (fd *specialFileFD) Translate(ctx context.Context, required, optional memma
484484
// InvalidateUnsavable implements memmap.Mappable.InvalidateUnsavable.
485485
func (fd *specialFileFD) InvalidateUnsavable(ctx context.Context) error {
486486
d := fd.dentry()
487-
d.mapsMu.Lock()
488-
defer d.mapsMu.Unlock()
489-
d.mappings.InvalidateAll(memmap.InvalidateOpts{})
487+
d.inode.mapsMu.Lock()
488+
defer d.inode.mapsMu.Unlock()
489+
d.inode.mappings.InvalidateAll(memmap.InvalidateOpts{})
490490
return nil
491491
}
492492

@@ -532,7 +532,7 @@ func (fd *specialFileFD) requireHostFD() {
532532

533533
func (fd *specialFileFD) updateMetadata(ctx context.Context) error {
534534
d := fd.dentry()
535-
d.metadataMu.Lock()
536-
defer d.metadataMu.Unlock()
537-
return d.updateMetadataLocked(ctx, fd.handle)
535+
d.inode.metadataMu.Lock()
536+
defer d.inode.metadataMu.Unlock()
537+
return d.inode.updateMetadataLocked(ctx, fd.handle)
538538
}

‎pkg/sentry/fsimpl/gofer/symlink.go

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -20,28 +20,28 @@ import (
2020
"gvisor.dev/gvisor/pkg/sentry/vfs"
2121
)
2222

23-
func (d *dentry) isSymlink() bool {
24-
return d.fileType() == linux.S_IFLNK
23+
func (i *inode) isSymlink() bool {
24+
return i.fileType() == linux.S_IFLNK
2525
}
2626

2727
// Precondition: d.isSymlink().
2828
func (d *dentry) readlink(ctx context.Context, mnt *vfs.Mount) (string, error) {
29-
if d.fs.opts.interop != InteropModeShared {
29+
if d.inode.fs.opts.interop != InteropModeShared {
3030
d.touchAtime(mnt)
31-
d.dataMu.Lock()
32-
if d.haveTarget {
33-
target := d.target
34-
d.dataMu.Unlock()
31+
d.inode.dataMu.Lock()
32+
if d.inode.haveTarget {
33+
target := d.inode.target
34+
d.inode.dataMu.Unlock()
3535
return target, nil
3636
}
3737
}
38-
target, err := d.readlinkImpl(ctx)
39-
if d.fs.opts.interop != InteropModeShared {
38+
target, err := d.inode.readlinkImpl(ctx)
39+
if d.inode.fs.opts.interop != InteropModeShared {
4040
if err == nil {
41-
d.haveTarget = true
42-
d.target = target
41+
d.inode.haveTarget = true
42+
d.inode.target = target
4343
}
44-
d.dataMu.Unlock() // +checklocksforce: guaranteed locked from above.
44+
d.inode.dataMu.Unlock() // +checklocksforce: guaranteed locked from above.
4545
}
4646
return target, err
4747
}

‎pkg/sentry/fsimpl/gofer/time.go

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -36,56 +36,56 @@ func (d *dentry) touchAtime(mnt *vfs.Mount) {
3636
if err := mnt.CheckBeginWrite(); err != nil {
3737
return
3838
}
39-
now := d.fs.clock.Now().Nanoseconds()
40-
d.metadataMu.Lock()
41-
d.atime.Store(now)
42-
d.atimeDirty.Store(1)
43-
d.metadataMu.Unlock()
39+
now := d.inode.fs.clock.Now().Nanoseconds()
40+
d.inode.metadataMu.Lock()
41+
d.inode.atime.Store(now)
42+
d.inode.atimeDirty.Store(1)
43+
d.inode.metadataMu.Unlock()
4444
mnt.EndWrite()
4545
}
4646

47-
// Preconditions: d.metadataMu is locked. d.cachedMetadataAuthoritative() == true.
47+
// Preconditions: d.inode.metadataMu is locked. d.cachedMetadataAuthoritative() == true.
4848
func (d *dentry) touchAtimeLocked(mnt *vfs.Mount) {
4949
if opts := mnt.Options(); opts.Flags.NoATime || opts.ReadOnly {
5050
return
5151
}
5252
if err := mnt.CheckBeginWrite(); err != nil {
5353
return
5454
}
55-
now := d.fs.clock.Now().Nanoseconds()
56-
d.atime.Store(now)
57-
d.atimeDirty.Store(1)
55+
now := d.inode.fs.clock.Now().Nanoseconds()
56+
d.inode.atime.Store(now)
57+
d.inode.atimeDirty.Store(1)
5858
mnt.EndWrite()
5959
}
6060

6161
// Preconditions:
6262
// - d.cachedMetadataAuthoritative() == true.
6363
// - The caller has successfully called vfs.Mount.CheckBeginWrite().
6464
func (d *dentry) touchCtime() {
65-
now := d.fs.clock.Now().Nanoseconds()
66-
d.metadataMu.Lock()
67-
d.ctime.Store(now)
68-
d.metadataMu.Unlock()
65+
now := d.inode.fs.clock.Now().Nanoseconds()
66+
d.inode.metadataMu.Lock()
67+
d.inode.ctime.Store(now)
68+
d.inode.metadataMu.Unlock()
6969
}
7070

7171
// Preconditions:
7272
// - d.cachedMetadataAuthoritative() == true.
7373
// - The caller has successfully called vfs.Mount.CheckBeginWrite().
7474
func (d *dentry) touchCMtime() {
75-
now := d.fs.clock.Now().Nanoseconds()
76-
d.metadataMu.Lock()
77-
d.mtime.Store(now)
78-
d.ctime.Store(now)
79-
d.mtimeDirty.Store(1)
80-
d.metadataMu.Unlock()
75+
now := d.inode.fs.clock.Now().Nanoseconds()
76+
d.inode.metadataMu.Lock()
77+
d.inode.mtime.Store(now)
78+
d.inode.ctime.Store(now)
79+
d.inode.mtimeDirty.Store(1)
80+
d.inode.metadataMu.Unlock()
8181
}
8282

8383
// Preconditions:
8484
// - d.cachedMetadataAuthoritative() == true.
85-
// - The caller has locked d.metadataMu.
85+
// - The caller has locked d.inode.metadataMu.
8686
func (d *dentry) touchCMtimeLocked() {
87-
now := d.fs.clock.Now().Nanoseconds()
88-
d.mtime.Store(now)
89-
d.ctime.Store(now)
90-
d.mtimeDirty.Store(1)
87+
now := d.inode.fs.clock.Now().Nanoseconds()
88+
d.inode.mtime.Store(now)
89+
d.inode.ctime.Store(now)
90+
d.inode.mtimeDirty.Store(1)
9191
}

‎test/syscalls/BUILD

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -344,9 +344,6 @@ syscall_test(
344344
add_fusefs = True,
345345
add_overlay = True,
346346
test = "//test/syscalls/linux:link_test",
347-
# TODO(gvisor.dev/issue/6739): Remove use_tmpfs=True once gofer filesystem
348-
# supports hard links correctly.
349-
use_tmpfs = True,
350347
)
351348

352349
syscall_test(

‎test/syscalls/linux/link.cc

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,13 @@
2121

2222
#include <string>
2323

24+
#include "gmock/gmock.h"
2425
#include "gtest/gtest.h"
2526
#include "absl/flags/flag.h"
2627
#include "absl/strings/str_cat.h"
27-
#include "test/util/capability_util.h"
2828
#include "test/util/file_descriptor.h"
2929
#include "test/util/fs_util.h"
30+
#include "test/util/linux_capability_util.h"
3031
#include "test/util/posix_error.h"
3132
#include "test/util/temp_path.h"
3233
#include "test/util/test_util.h"
@@ -51,7 +52,6 @@ bool IsSameFile(const std::string& f1, const std::string& f2) {
5152
}
5253

5354
// TODO(b/178640646): Add test for linkat with AT_EMPTY_PATH
54-
5555
TEST(LinkTest, CanCreateLinkFile) {
5656
auto oldfile = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFile());
5757
const std::string newname = NewTempAbsPath();
@@ -76,6 +76,21 @@ TEST(LinkTest, CanCreateLinkFile) {
7676
IsPosixErrorOkAndHolds(initial_link_count));
7777
}
7878

79+
TEST(LinkTest, HardlinkChangeMode) {
80+
auto oldfile = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFile());
81+
const std::string newname = NewTempAbsPath();
82+
struct stat stat1 = {};
83+
84+
ASSERT_THAT(link(oldfile.path().c_str(), newname.c_str()), SyscallSucceeds());
85+
86+
EXPECT_TRUE(IsSameFile(oldfile.path(), newname));
87+
88+
EXPECT_THAT(chmod(oldfile.path().c_str(), S_IRUSR), SyscallSucceeds());
89+
EXPECT_THAT(lstat(newname.c_str(), &stat1), SyscallSucceeds());
90+
EXPECT_EQ(S_IRUSR, (stat1.st_mode & S_IRWXU));
91+
EXPECT_THAT(unlink(newname.c_str()), SyscallSucceeds());
92+
}
93+
7994
TEST(LinkTest, PermissionDenied) {
8095
SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(HaveCapability(CAP_FOWNER)));
8196

‎test/util/test_util.cc

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
#include <sys/utsname.h>
2424
#include <unistd.h>
2525

26+
#include <cstdint>
2627
#include <ctime>
2728
#include <iostream>
2829
#include <vector>
@@ -182,12 +183,14 @@ PosixErrorOr<std::vector<OpenFd>> GetOpenFDs() {
182183
return ret_fds;
183184
}
184185

186+
PosixErrorOr<uint64_t> Permissions(const std::string& path) {
187+
ASSIGN_OR_RETURN_ERRNO(auto stat_result, Stat(path));
188+
return static_cast<uint64_t>(stat_result.st_mode);
189+
}
190+
185191
PosixErrorOr<uint64_t> Links(const std::string& path) {
186-
struct stat st;
187-
if (stat(path.c_str(), &st)) {
188-
return PosixError(errno, absl::StrCat("Failed to stat ", path));
189-
}
190-
return static_cast<uint64_t>(st.st_nlink);
192+
ASSIGN_OR_RETURN_ERRNO(auto stat_result, Stat(path));
193+
return static_cast<uint64_t>(stat_result.st_nlink);
191194
}
192195

193196
void RandomizeBuffer(char* buffer, size_t len) {

0 commit comments

Comments
 (0)
Please sign in to comment.