Skip to content

Commit 16e9064

Browse files
ayushr2gvisor-bot
ayushr2
authored and
gvisor-bot
committed
Add better panic message for when file type changes without ino number changing
This panic message also logs the file path and the inoKey of the impacted dentry in case during revalidation we find out that the inode number is the same but the file type has changed. Also moved directfsInode.updateMetadataFromStatLocked() and lisafsInode.updateMetadataFromStatxLocked() to be in the right files. PiperOrigin-RevId: 803212251
1 parent 1f32215 commit 16e9064

File tree

3 files changed

+82
-76
lines changed

3 files changed

+82
-76
lines changed

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

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -248,13 +248,11 @@ func (i *directfsInode) ensureLisafsControlFD(ctx context.Context, d *dentry) er
248248
panic("unreachable")
249249
}
250250

251-
// Precondition: i.metadataMu must be locked.
252-
//
253251
// +checklocks:i.metadataMu
254252
func (i *directfsInode) updateMetadataLocked(h handle) error {
255253
handleMuRLocked := false
256254
if h.fd < 0 {
257-
// Use open FDs in preferenece to the control FD. Control FDs may be opened
255+
// Use open FDs in preference to the control FD. Control FDs may be opened
258256
// with O_PATH. This may be significantly more efficient in some
259257
// implementations. Prefer a writable FD over a readable one since some
260258
// filesystem implementations may update a writable FD's metadata after
@@ -286,6 +284,31 @@ func (i *directfsInode) updateMetadataLocked(h handle) error {
286284
return i.updateMetadataFromStatLocked(&stat)
287285
}
288286

287+
// updateMetadataFromStatLocked is similar to updateMetadataFromStatxLocked,
288+
// except that it takes a unix.Stat_t argument.
289+
// +checklocks:i.inode.metadataMu
290+
func (i *directfsInode) updateMetadataFromStatLocked(stat *unix.Stat_t) error {
291+
if got, want := stat.Mode&unix.S_IFMT, i.inode.fileType(); got != want {
292+
panic(fmt.Sprintf("directfsInode file type changed from %#o to %#o", want, got))
293+
}
294+
i.inode.mode.Store(stat.Mode)
295+
i.inode.uid.Store(stat.Uid)
296+
i.inode.gid.Store(stat.Gid)
297+
i.inode.blockSize.Store(uint32(stat.Blksize))
298+
// Don't override newer client-defined timestamps with old host-defined
299+
// ones.
300+
if i.inode.atimeDirty.Load() == 0 {
301+
i.inode.atime.Store(dentryTimestampFromUnix(stat.Atim))
302+
}
303+
if i.inode.mtimeDirty.Load() == 0 {
304+
i.inode.mtime.Store(dentryTimestampFromUnix(stat.Mtim))
305+
}
306+
i.inode.ctime.Store(dentryTimestampFromUnix(stat.Ctim))
307+
i.inode.nlink.Store(uint32(stat.Nlink))
308+
i.inode.updateSizeLocked(uint64(stat.Size))
309+
return nil
310+
}
311+
289312
// Precondition: fs.renameMu is locked if d is a socket.
290313
func (i *directfsInode) chmod(ctx context.Context, mode uint16, d *dentry) error {
291314
if i.isSymlink() {
@@ -780,6 +803,11 @@ func doRevalidationDirectfs(ctx context.Context, vfsObj *vfs.VirtualFilesystem,
780803
return nil
781804
}
782805

806+
// Check that the file type has not changed.
807+
if got, want := stat.Mode&unix.S_IFMT, d.inode.fileType(); got != want {
808+
panic(fmt.Sprintf("file type of %q changed from %#o to %#o while inode key (%+v) did not change", genericDebugPathname(d.inode.fs, d), want, got, d.inode.inoKey))
809+
}
810+
783811
// The file at this path hasn't changed. Just update cached metadata.
784812
d.inode.impl.(*directfsInode).updateMetadataFromStatLocked(&stat) // +checklocksforce: i.metadataMu is locked above.
785813
d.inode.metadataMu.Unlock()

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

Lines changed: 0 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -1168,76 +1168,6 @@ func (d *dentry) init() {
11681168
refs.Register(d)
11691169
}
11701170

1171-
// updateMetadataFromStatxLocked is called to update d's metadata after an update
1172-
// from the remote filesystem.
1173-
// Precondition: i.inode.metadataMu must be locked.
1174-
// +checklocks:i.inode.metadataMu
1175-
func (i *lisafsInode) updateMetadataFromStatxLocked(stat *linux.Statx) {
1176-
if stat.Mask&linux.STATX_TYPE != 0 {
1177-
if got, want := stat.Mode&linux.FileTypeMask, i.inode.fileType(); uint32(got) != want {
1178-
panic(fmt.Sprintf("gofer.dentry file type changed from %#o to %#o", want, got))
1179-
}
1180-
}
1181-
if stat.Mask&linux.STATX_MODE != 0 {
1182-
i.inode.mode.Store(uint32(stat.Mode))
1183-
}
1184-
if stat.Mask&linux.STATX_UID != 0 {
1185-
i.inode.uid.Store(dentryUID(lisafs.UID(stat.UID)))
1186-
}
1187-
if stat.Mask&linux.STATX_GID != 0 {
1188-
i.inode.gid.Store(dentryGID(lisafs.GID(stat.GID)))
1189-
}
1190-
if stat.Blksize != 0 {
1191-
i.inode.blockSize.Store(stat.Blksize)
1192-
}
1193-
// Don't override newer client-defined timestamps with old server-defined
1194-
// ones.
1195-
if stat.Mask&linux.STATX_ATIME != 0 && i.inode.atimeDirty.Load() == 0 {
1196-
i.inode.atime.Store(dentryTimestamp(stat.Atime))
1197-
}
1198-
if stat.Mask&linux.STATX_MTIME != 0 && i.inode.mtimeDirty.Load() == 0 {
1199-
i.inode.mtime.Store(dentryTimestamp(stat.Mtime))
1200-
}
1201-
if stat.Mask&linux.STATX_CTIME != 0 {
1202-
i.inode.ctime.Store(dentryTimestamp(stat.Ctime))
1203-
}
1204-
if stat.Mask&linux.STATX_BTIME != 0 {
1205-
i.inode.btime.Store(dentryTimestamp(stat.Btime))
1206-
}
1207-
if stat.Mask&linux.STATX_NLINK != 0 {
1208-
i.inode.nlink.Store(stat.Nlink)
1209-
}
1210-
if stat.Mask&linux.STATX_SIZE != 0 {
1211-
i.updateSizeLocked(stat.Size)
1212-
}
1213-
}
1214-
1215-
// updateMetadataFromStatLocked is similar to updateMetadataFromStatxLocked,
1216-
// except that it takes a unix.Stat_t argument.
1217-
// Precondition: i.inode.metadataMu must be locked.
1218-
// +checklocks:i.inode.metadataMu
1219-
func (i *directfsInode) updateMetadataFromStatLocked(stat *unix.Stat_t) error {
1220-
if got, want := stat.Mode&unix.S_IFMT, i.inode.fileType(); got != want {
1221-
panic(fmt.Sprintf("direct.dentry file type changed from %#o to %#o", want, got))
1222-
}
1223-
i.inode.mode.Store(stat.Mode)
1224-
i.inode.uid.Store(stat.Uid)
1225-
i.inode.gid.Store(stat.Gid)
1226-
i.inode.blockSize.Store(uint32(stat.Blksize))
1227-
// Don't override newer client-defined timestamps with old host-defined
1228-
// ones.
1229-
if i.inode.atimeDirty.Load() == 0 {
1230-
i.inode.atime.Store(dentryTimestampFromUnix(stat.Atim))
1231-
}
1232-
if i.inode.mtimeDirty.Load() == 0 {
1233-
i.inode.mtime.Store(dentryTimestampFromUnix(stat.Mtim))
1234-
}
1235-
i.inode.ctime.Store(dentryTimestampFromUnix(stat.Ctim))
1236-
i.inode.nlink.Store(uint32(stat.Nlink))
1237-
i.inode.updateSizeLocked(uint64(stat.Size))
1238-
return nil
1239-
}
1240-
12411171
// Preconditions: !d.inode.isSynthetic().
12421172
// Preconditions: d.inode.metadataMu is locked.
12431173
// +checklocks:d.inode.metadataMu

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

Lines changed: 51 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -228,8 +228,6 @@ func (i *lisafsInode) updateHandles(ctx context.Context, h handle, readable, wri
228228
}
229229
}
230230

231-
// Precondition: i.metadataMu must be lockei.
232-
//
233231
// +checklocks:i.metadataMu
234232
func (i *lisafsInode) updateMetadataLocked(ctx context.Context, h handle) error {
235233
handleMuRLocked := false
@@ -266,6 +264,49 @@ func (i *lisafsInode) updateMetadataLocked(ctx context.Context, h handle) error
266264
return nil
267265
}
268266

267+
// updateMetadataFromStatxLocked is called to update d's metadata after an update
268+
// from the remote filesystem.
269+
// +checklocks:i.inode.metadataMu
270+
func (i *lisafsInode) updateMetadataFromStatxLocked(stat *linux.Statx) {
271+
if stat.Mask&linux.STATX_TYPE != 0 {
272+
if got, want := stat.Mode&linux.FileTypeMask, i.inode.fileType(); uint32(got) != want {
273+
panic(fmt.Sprintf("lisafsInode file type changed from %#o to %#o", want, got))
274+
}
275+
}
276+
if stat.Mask&linux.STATX_MODE != 0 {
277+
i.inode.mode.Store(uint32(stat.Mode))
278+
}
279+
if stat.Mask&linux.STATX_UID != 0 {
280+
i.inode.uid.Store(dentryUID(lisafs.UID(stat.UID)))
281+
}
282+
if stat.Mask&linux.STATX_GID != 0 {
283+
i.inode.gid.Store(dentryGID(lisafs.GID(stat.GID)))
284+
}
285+
if stat.Blksize != 0 {
286+
i.inode.blockSize.Store(stat.Blksize)
287+
}
288+
// Don't override newer client-defined timestamps with old server-defined
289+
// ones.
290+
if stat.Mask&linux.STATX_ATIME != 0 && i.inode.atimeDirty.Load() == 0 {
291+
i.inode.atime.Store(dentryTimestamp(stat.Atime))
292+
}
293+
if stat.Mask&linux.STATX_MTIME != 0 && i.inode.mtimeDirty.Load() == 0 {
294+
i.inode.mtime.Store(dentryTimestamp(stat.Mtime))
295+
}
296+
if stat.Mask&linux.STATX_CTIME != 0 {
297+
i.inode.ctime.Store(dentryTimestamp(stat.Ctime))
298+
}
299+
if stat.Mask&linux.STATX_BTIME != 0 {
300+
i.inode.btime.Store(dentryTimestamp(stat.Btime))
301+
}
302+
if stat.Mask&linux.STATX_NLINK != 0 {
303+
i.inode.nlink.Store(stat.Nlink)
304+
}
305+
if stat.Mask&linux.STATX_SIZE != 0 {
306+
i.updateSizeLocked(stat.Size)
307+
}
308+
}
309+
269310
func chmod(ctx context.Context, controlFD lisafs.ClientFD, mode uint16) error {
270311
setStat := linux.Statx{
271312
Mask: linux.STATX_MODE,
@@ -652,7 +693,7 @@ func doRevalidationLisafs(ctx context.Context, vfsObj *vfs.VirtualFilesystem, st
652693
if state.refreshStart {
653694
if len(stats) > 0 {
654695
// First dentry is where the search is starting, just update attributes
655-
// since it cannot be replacei.
696+
// since it cannot be replaced.
656697
start.updateMetadataFromStatxLocked(&stats[0]) // +checklocksforce: see above.
657698
stats = stats[1:]
658699
}
@@ -679,6 +720,13 @@ func doRevalidationLisafs(ctx context.Context, vfsObj *vfs.VirtualFilesystem, st
679720
return nil
680721
}
681722

723+
// Check that the file type has not changed.
724+
if stats[i].Mask&linux.STATX_TYPE != 0 {
725+
if got, want := stats[i].Mode&linux.FileTypeMask, d.inode.fileType(); uint32(got) != want {
726+
panic(fmt.Sprintf("file type of %q changed from %#o to %#o while inode key (%+v) did not change", genericDebugPathname(d.inode.fs, d), want, got, d.inode.inoKey))
727+
}
728+
}
729+
682730
// The file at this path hasn't changed. Just update cached metadata.
683731
d.inode.impl.(*lisafsInode).updateMetadataFromStatxLocked(&stats[i]) // +checklocksforce: see above.
684732
d.inode.metadataMu.Unlock()

0 commit comments

Comments
 (0)