Skip to content

Commit 188a756

Browse files
ayushr2gvisor-bot
authored andcommitted
Add validateMap() to compare maps correctly during OCI spec validation.
reflect.Equal() considers nil and empty maps/slices different. But from spec validation perspective, they should be considered the same. So added validateMap() generic utility to do the comparison correctly. Also fixed the validation of spec.Process.Rlimits, which was an array of structs. Apart from suffering from the said reflect.Equal() issue, it also suffered from ordering comparison. A list of shuffled-but-same rlimits should be restorable. Fixed it to use validateArray(). There were only 2 users of `validateStructMap` remaining. So removed it switched its users to call validateStruct() directly. Fixes #11248 PiperOrigin-RevId: 703302621
1 parent 5ca2186 commit 188a756

File tree

1 file changed

+29
-17
lines changed

1 file changed

+29
-17
lines changed

runsc/boot/restore.go

Lines changed: 29 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -314,6 +314,19 @@ func validateArray[T any](field, cName string, oldArr, newArr []T) error {
314314
return nil
315315
}
316316

317+
func validateMap[K comparable, V comparable](field, cName string, oldM map[K]V, newM map[K]V) error {
318+
if len(oldM) != len(newM) {
319+
return validateError(field, cName, oldM, newM)
320+
}
321+
for k, v1 := range oldM {
322+
v2, ok := newM[k]
323+
if !ok || v1 != v2 {
324+
return validateError(field, cName, oldM, newM)
325+
}
326+
}
327+
return nil
328+
}
329+
317330
func sortCapabilities(o *specs.LinuxCapabilities) {
318331
sort.Strings(o.Bounding)
319332
sort.Strings(o.Effective)
@@ -431,9 +444,14 @@ func validateSpecForContainer(oSpec, nSpec *specs.Spec, cName string) error {
431444
if oldProcess.Cwd != newProcess.Cwd {
432445
return validateError("Cwd", cName, oldProcess.Cwd, newProcess.Cwd)
433446
}
434-
validateStructMap := make(map[string][2]any)
435-
validateStructMap["User"] = [2]any{oldProcess.User, newProcess.User}
436-
validateStructMap["Rlimits"] = [2]any{oldProcess.Rlimits, newProcess.Rlimits}
447+
if err := validateStruct("User", cName, oldProcess.User, newProcess.User); err != nil {
448+
return err
449+
}
450+
oldProcess.User, newProcess.User = specs.User{}, specs.User{}
451+
if err := validateArray("Rlimits", cName, oldProcess.Rlimits, newProcess.Rlimits); err != nil {
452+
return err
453+
}
454+
oldProcess.Rlimits, newProcess.Rlimits = nil, nil
437455
if ok := slices.Equal(oldProcess.Args, newProcess.Args); !ok {
438456
return validateError("Args", cName, oldProcess.Args, newProcess.Args)
439457
}
@@ -445,8 +463,14 @@ func validateSpecForContainer(oSpec, nSpec *specs.Spec, cName string) error {
445463
// Validate specs.Linux.
446464
oldSpec.Linux, newSpec.Linux = ifNil(oldSpec.Linux), ifNil(newSpec.Linux)
447465
oldLinux, newLinux := *oldSpec.Linux, *newSpec.Linux
448-
validateStructMap["Sysctl"] = [2]any{oldLinux.Sysctl, newLinux.Sysctl}
449-
validateStructMap["Seccomp"] = [2]any{oldLinux.Seccomp, newLinux.Seccomp}
466+
if err := validateMap("Sysctl", cName, oldLinux.Sysctl, newLinux.Sysctl); err != nil {
467+
return err
468+
}
469+
oldLinux.Sysctl, newLinux.Sysctl = nil, nil
470+
if err := validateStruct("Seccomp", cName, oldLinux.Seccomp, newLinux.Seccomp); err != nil {
471+
return err
472+
}
473+
oldLinux.Seccomp, newLinux.Seccomp = nil, nil
450474
if err := validateDevices("Devices", cName, oldLinux.Devices, newLinux.Devices); err != nil {
451475
return err
452476
}
@@ -468,18 +492,6 @@ func validateSpecForContainer(oSpec, nSpec *specs.Spec, cName string) error {
468492
}
469493
oldLinux.Namespaces, newLinux.Namespaces = nil, nil
470494

471-
// Validate all the structs collected in validateStructMap above.
472-
for key, val := range validateStructMap {
473-
if err := validateStruct(key, cName, val[0], val[1]); err != nil {
474-
return err
475-
}
476-
}
477-
// Set the fields in validateStructMap to nil after validation.
478-
oldProcess.User, newProcess.User = specs.User{}, specs.User{}
479-
oldProcess.Rlimits, newProcess.Rlimits = nil, nil
480-
oldLinux.Sysctl, newLinux.Sysctl = nil, nil
481-
oldLinux.Seccomp, newLinux.Seccomp = nil, nil
482-
483495
// Hostname, Domainname, Environment variables and CgroupsPath are
484496
// allowed to change during restore. Hooks contain callbacks for
485497
// lifecycle of the container such as prestart and teardown, and can

0 commit comments

Comments
 (0)