netbird/gvisor
0
0
Fork 0
mirror of https://github.com/netbirdio/gvisor.git synced 2026-05-22 17:12:49 -07:00
Code Issues Packages Projects Releases Wiki Activity

gofer: Fix bug when casting auth.K{U/G}ID to int.

Browse Source 
auth.K{U/G}ID has type uint32. When uid = auth.NoID = math.MaxUint32, which is
intended to represent -1, and it is casted to int, it becomes 4294967295 on
64-bit systems as int is 8 bytes in size. So the == -1 check fails.

Fixed the bug by explicitly setting syscall args to -1 when uid.Ok() == false.
Similarly, fix the bug in runsc/fsgofer.

PiperOrigin-RevId: 728258705
This commit is contained in:
Ayush Ranjan
2025-02-18 10:16:08 -08:00
committed by gVisor bot
parent 17563a8af9
commit 84670a4fc6
3 changed files with 60 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/lisafs/message.go
+2 -2
View File
@@ -179,10 +179,10 @@ const (
)
const (
// NoUID is a sentinel used to indicate no valid UID.
// NoUID is a sentinel used to indicate no valid UID. See auth.NoID.
NoUID UID = math.MaxUint32
// NoGID is a sentinel used to indicate no valid GID.
// NoGID is a sentinel used to indicate no valid GID. See auth.NoID.
NoGID GID = math.MaxUint32
)
pkg/sentry/fsimpl/gofer/directfs_dentry.go
+31 -23
View File
@@ -381,15 +381,13 @@ func (d *directfsDentry) setStatLocked(ctx context.Context, stat *linux.Statx) (
}
if stat.Mask&(unix.STATX_UID|unix.STATX_GID) != 0 {
// "If the owner or group is specified as -1, then that ID is not changed"
// - chown(2)
uid := -1
uid := auth.KUID(auth.NoID)
if stat.Mask&unix.STATX_UID != 0 {
uid = int(stat.UID)
uid = auth.KUID(stat.UID)
}
gid := -1
gid := auth.KGID(auth.NoID)
if stat.Mask&unix.STATX_GID != 0 {
gid = int(stat.GID)
gid = auth.KGID(stat.GID)
}
if err := fchown(d.controlFD, uid, gid); err != nil {
failureMask |= stat.Mask & (unix.STATX_UID | unix.STATX_GID)
@@ -399,8 +397,21 @@ func (d *directfsDentry) setStatLocked(ctx context.Context, stat *linux.Statx) (
return
}
func fchown(fd, uid, gid int) error {
return unix.Fchownat(fd, "", uid, gid, unix.AT_EMPTY_PATH|unix.AT_SYMLINK_NOFOLLOW)
func fchown(fd int, uid auth.KUID, gid auth.KGID) error {
// "If the owner or group is specified as -1, then that ID is not changed"
// - chown(2). Only bother making the syscall if the owner is changing.
if !uid.Ok() && !gid.Ok() {
return nil
}
u := -1
g := -1
if uid.Ok() {
u = int(uid)
}
if gid.Ok() {
g = int(gid)
}
return unix.Fchownat(fd, "", u, g, unix.AT_EMPTY_PATH|unix.AT_SYMLINK_NOFOLLOW)
}
// Precondition: d.handleMu must be locked.
@@ -442,7 +453,7 @@ func (d *directfsDentry) getXattr(ctx context.Context, name string, size uint64)
// getCreatedChild opens the newly created child, sets its uid/gid, constructs
// a disconnected dentry and returns it.
func (d *directfsDentry) getCreatedChild(name string, uid, gid int, isDir bool) (*dentry, error) {
func (d *directfsDentry) getCreatedChild(name string, uid auth.KUID, gid auth.KGID, isDir bool) (*dentry, error) {
unlinkFlags := 0
extraOpenFlags := 0
if isDir {
@@ -464,15 +475,12 @@ func (d *directfsDentry) getCreatedChild(name string, uid, gid int, isDir bool)
return nil, err
}
// "If the owner or group is specified as -1, then that ID is not changed"
// - chown(2). Only bother making the syscall if the owner is changing.
if uid != -1 || gid != -1 {
if err := fchown(childFD, uid, gid); err != nil {
deleteChild()
_ = unix.Close(childFD)
return nil, err
}
if err := fchown(childFD, uid, gid); err != nil {
deleteChild()
_ = unix.Close(childFD)
return nil, err
}
child, err := d.fs.newDirectfsDentry(childFD)
if err != nil {
// Ownership of childFD was passed to newDirectDentry(), so no need to
@@ -498,7 +506,7 @@ func (d *directfsDentry) mknod(ctx context.Context, name string, creds *auth.Cre
if err := unix.Mknodat(d.controlFD, name, uint32(opts.Mode), 0); err != nil {
return nil, err
}
return d.getCreatedChild(name, int(creds.EffectiveKUID), int(creds.EffectiveKGID), false /* isDir */)
return d.getCreatedChild(name, creds.EffectiveKUID, creds.EffectiveKGID, false /* isDir */)
}
// Precondition: opts.Endpoint != nil and is transport.HostBoundEndpoint type.
@@ -523,7 +531,7 @@ func (d *directfsDentry) bindAt(ctx context.Context, name string, creds *auth.Cr
return nil, err
}
// Socket already has the right UID/GID set, so use uid = gid = -1.
child, err := d.getCreatedChild(name, -1 /* uid */, -1 /* gid */, false /* isDir */)
child, err := d.getCreatedChild(name, auth.NoID /* uid */, auth.NoID /* gid */, false /* isDir */)
if err != nil {
hbep.ResetBoundSocketFD(ctx)
return nil, err
@@ -551,21 +559,21 @@ func (d *directfsDentry) link(target *directfsDentry, name string) (*dentry, err
// link. The original file already has the right owner.
// TODO(gvisor.dev/issue/6739): Hard linked dentries should share the same
// inode fields.
return d.getCreatedChild(name, -1 /* uid */, -1 /* gid */, false /* isDir */)
return d.getCreatedChild(name, auth.NoID /* uid */, auth.NoID /* gid */, false /* isDir */)
}
func (d *directfsDentry) mkdir(name string, mode linux.FileMode, uid auth.KUID, gid auth.KGID) (*dentry, error) {
if err := unix.Mkdirat(d.controlFD, name, uint32(mode)); err != nil {
return nil, err
}
return d.getCreatedChild(name, int(uid), int(gid), true /* isDir */)
return d.getCreatedChild(name, uid, gid, true /* isDir */)
}
func (d *directfsDentry) symlink(name, target string, creds *auth.Credentials) (*dentry, error) {
if err := unix.Symlinkat(target, d.controlFD, name); err != nil {
return nil, err
}
return d.getCreatedChild(name, int(creds.EffectiveKUID), int(creds.EffectiveKGID), false /* isDir */)
return d.getCreatedChild(name, creds.EffectiveKUID, creds.EffectiveKGID, false /* isDir */)
}
func (d *directfsDentry) openCreate(name string, accessFlags uint32, mode linux.FileMode, uid auth.KUID, gid auth.KGID) (*dentry, handle, error) {
@@ -575,7 +583,7 @@ func (d *directfsDentry) openCreate(name string, accessFlags uint32, mode linux.
return nil, noHandle, err
}
child, err := d.getCreatedChild(name, int(uid), int(gid), false /* isDir */)
child, err := d.getCreatedChild(name, uid, gid, false /* isDir */)
if err != nil {
_ = unix.Close(childHandleFD)
return nil, noHandle, err
runsc/fsgofer/lisafs.go
+27 -10
View File
@@ -395,15 +395,15 @@ func (fd *controlFDLisa) SetStat(stat lisafs.SetStatReq) (failureMask uint32, fa
if stat.Mask&(unix.STATX_UID|unix.STATX_GID) != 0 {
// "If the owner or group is specified as -1, then that ID is not changed"
// - chown(2)
uid := -1
uid := lisafs.NoUID
if stat.Mask&unix.STATX_UID != 0 {
uid = int(stat.UID)
uid = stat.UID
}
gid := -1
gid := lisafs.NoGID
if stat.Mask&unix.STATX_GID != 0 {
gid = int(stat.GID)
gid = stat.GID
}
if err := unix.Fchownat(fd.hostFD, "", uid, gid, unix.AT_EMPTY_PATH|unix.AT_SYMLINK_NOFOLLOW); err != nil {
if err := fchown(fd.hostFD, uid, gid); err != nil {
log.Warningf("SetStat fchown failed %q, err: %v", fd.Node().FilePath(), err)
failureMask |= stat.Mask & (unix.STATX_UID | unix.STATX_GID)
failureErr = err
@@ -575,7 +575,7 @@ func (fd *controlFDLisa) OpenCreate(mode linux.FileMode, uid lisafs.UID, gid lis
defer cu.Clean()
// Set the owners as requested by the client.
if err := unix.Fchownat(childHostFD, "", int(uid), int(gid), unix.AT_EMPTY_PATH|unix.AT_SYMLINK_NOFOLLOW); err != nil {
if err := fchown(childHostFD, uid, gid); err != nil {
return nil, linux.Statx{}, nil, -1, err
}
@@ -628,7 +628,7 @@ func (fd *controlFDLisa) Mkdir(mode linux.FileMode, uid lisafs.UID, gid lisafs.G
if err != nil {
return nil, linux.Statx{}, err
}
if err := unix.Fchownat(childDirFd, "", int(uid), int(gid), unix.AT_EMPTY_PATH|unix.AT_SYMLINK_NOFOLLOW); err != nil {
if err := fchown(childDirFd, uid, gid); err != nil {
unix.Close(childDirFd)
return nil, linux.Statx{}, err
}
@@ -671,7 +671,7 @@ func (fd *controlFDLisa) Mknod(mode linux.FileMode, uid lisafs.UID, gid lisafs.G
if err != nil {
return nil, linux.Statx{}, err
}
if err := unix.Fchownat(childFD, "", int(uid), int(gid), unix.AT_EMPTY_PATH|unix.AT_SYMLINK_NOFOLLOW); err != nil {
if err := fchown(childFD, uid, gid); err != nil {
unix.Close(childFD)
return nil, linux.Statx{}, err
}
@@ -705,7 +705,7 @@ func (fd *controlFDLisa) Symlink(name string, target string, uid lisafs.UID, gid
if err != nil {
return nil, linux.Statx{}, err
}
if err := unix.Fchownat(symlinkFD, "", int(uid), int(gid), unix.AT_EMPTY_PATH|unix.AT_SYMLINK_NOFOLLOW); err != nil {
if err := fchown(symlinkFD, uid, gid); err != nil {
unix.Close(symlinkFD)
return nil, linux.Statx{}, err
}
@@ -959,7 +959,7 @@ func (fd *controlFDLisa) BindAt(name string, sockType uint32, mode linux.FileMod
_ = unix.Close(sockFileFD)
})
if err := unix.Fchownat(sockFileFD, "", int(uid), int(gid), unix.AT_EMPTY_PATH|unix.AT_SYMLINK_NOFOLLOW); err != nil {
if err := fchown(sockFileFD, uid, gid); err != nil {
return nil, linux.Statx{}, nil, -1, err
}
@@ -1223,6 +1223,23 @@ func tryOpen(open func(int) (int, error)) (hostFD int, err error) {
return
}
func fchown(hostFD int, uid lisafs.UID, gid lisafs.GID) error {
// "If the owner or group is specified as -1, then that ID is not changed"
// - chown(2). Only bother making the syscall if the owner is changing.
if !uid.Ok() && !gid.Ok() {
return nil
}
u := -1
g := -1
if uid.Ok() {
u = int(uid)
}
if gid.Ok() {
g = int(gid)
}
return unix.Fchownat(hostFD, "", u, g, unix.AT_EMPTY_PATH|unix.AT_SYMLINK_NOFOLLOW)
}
func fstatTo(hostFD int) (linux.Statx, error) {
var stat unix.Stat_t
if err := unix.Fstat(hostFD, &stat); err != nil {