mirror of
https://github.com/netbirdio/gvisor.git
synced 2026-05-22 17:12:49 -07:00
[op] Prevent file leak in MultiGetAttr's error path.
The old implementation was mostly correct but error prone - making way for the issue in question here. In its error path, it would leak the intermediate file being walked. Each return/break needed explicit cleanup. This change implements a more clean way to cleaning up intermediate directories. If the code were to evolve to be more complex, it would still work. PiperOrigin-RevId: 392102826
This commit is contained in:
+8
-8
@@ -325,6 +325,12 @@ func (*DisallowServerCalls) Renamed(File, string) {
|
||||
func DefaultMultiGetAttr(start File, names []string) ([]FullStat, error) {
|
||||
stats := make([]FullStat, 0, len(names))
|
||||
parent := start
|
||||
closeParent := func() {
|
||||
if parent != start {
|
||||
_ = parent.Close()
|
||||
}
|
||||
}
|
||||
defer closeParent()
|
||||
mask := AttrMaskAll()
|
||||
for i, name := range names {
|
||||
if len(name) == 0 && i == 0 {
|
||||
@@ -340,15 +346,14 @@ func DefaultMultiGetAttr(start File, names []string) ([]FullStat, error) {
|
||||
continue
|
||||
}
|
||||
qids, child, valid, attr, err := parent.WalkGetAttr([]string{name})
|
||||
if parent != start {
|
||||
_ = parent.Close()
|
||||
}
|
||||
if err != nil {
|
||||
if errors.Is(err, unix.ENOENT) {
|
||||
return stats, nil
|
||||
}
|
||||
return nil, err
|
||||
}
|
||||
closeParent()
|
||||
parent = child
|
||||
stats = append(stats, FullStat{
|
||||
QID: qids[0],
|
||||
Valid: valid,
|
||||
@@ -357,13 +362,8 @@ func DefaultMultiGetAttr(start File, names []string) ([]FullStat, error) {
|
||||
if attr.Mode.FileType() != ModeDirectory {
|
||||
// Doesn't need to continue if entry is not a dir. Including symlinks
|
||||
// that cannot be followed.
|
||||
_ = child.Close()
|
||||
break
|
||||
}
|
||||
parent = child
|
||||
}
|
||||
if parent != start {
|
||||
_ = parent.Close()
|
||||
}
|
||||
return stats, nil
|
||||
}
|
||||
|
||||
@@ -1242,13 +1242,14 @@ func (l *localFile) MultiGetAttr(names []string) ([]p9.FullStat, error) {
|
||||
}
|
||||
|
||||
parent := l.file.FD()
|
||||
closeParent := func() {
|
||||
if parent != l.file.FD() {
|
||||
_ = unix.Close(parent)
|
||||
}
|
||||
}
|
||||
defer closeParent()
|
||||
for _, name := range names {
|
||||
child, err := unix.Openat(parent, name, openFlags|unix.O_PATH, 0)
|
||||
if parent != l.file.FD() {
|
||||
// Parent is no longer needed.
|
||||
_ = unix.Close(parent)
|
||||
parent = -1
|
||||
}
|
||||
if err != nil {
|
||||
if errors.Is(err, unix.ENOENT) {
|
||||
// No pont in continuing any further.
|
||||
@@ -1256,10 +1257,11 @@ func (l *localFile) MultiGetAttr(names []string) ([]p9.FullStat, error) {
|
||||
}
|
||||
return nil, err
|
||||
}
|
||||
closeParent()
|
||||
parent = child
|
||||
|
||||
var stat unix.Stat_t
|
||||
if err := unix.Fstat(child, &stat); err != nil {
|
||||
_ = unix.Close(child)
|
||||
return nil, err
|
||||
}
|
||||
valid, attr := l.fillAttr(&stat)
|
||||
@@ -1271,13 +1273,9 @@ func (l *localFile) MultiGetAttr(names []string) ([]p9.FullStat, error) {
|
||||
if (stat.Mode & unix.S_IFMT) != unix.S_IFDIR {
|
||||
// Doesn't need to continue if entry is not a dir. Including symlinks
|
||||
// that cannot be followed.
|
||||
_ = unix.Close(child)
|
||||
break
|
||||
}
|
||||
parent = child
|
||||
}
|
||||
if parent != -1 && parent != l.file.FD() {
|
||||
_ = unix.Close(parent)
|
||||
}
|
||||
return stats, nil
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user