From 48006d000706c2232fbb6dc7c1e493be41451159 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Thu, 17 Mar 2022 13:27:12 -0700 Subject: [PATCH] libct/configs/validate: rootlessEUIDMount: speedup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. Fix function docs. In particular, remove the part which is not true ("verifies that the user isn't trying to set up any mounts they don't have the rights to do"), and fix the part that says "that doesn't resolve to root" (which is no longer true since commit d8b669400adc). 2. Replace fmt.Sscanf (which is slow and does lots of allocations) with strings.TrimPrefix and strconv.Atoi. 3. Add a benchmark for rootlessEUIDMount. Comparing the old and the new implementations: name old time/op new time/op delta RootlessEUIDMount-4 1.01µs ± 2% 0.16µs ± 1% -84.15% (p=0.008 n=5+5) name old alloc/op new alloc/op delta RootlessEUIDMount-4 224B ± 0% 80B ± 0% -64.29% (p=0.008 n=5+5) name old allocs/op new allocs/op delta RootlessEUIDMount-4 7.00 ± 0% 1.00 ± 0% -85.71% (p=0.008 n=5+5) Note this code is already tested (in rootless_test.go). Fixes: d8b669400adc Signed-off-by: Kir Kolyshkin --- libcontainer/configs/validate/rootless.go | 21 ++++++++----------- .../configs/validate/rootless_test.go | 21 +++++++++++++++++++ 2 files changed, 30 insertions(+), 12 deletions(-) diff --git a/libcontainer/configs/validate/rootless.go b/libcontainer/configs/validate/rootless.go index 7afdb4310f9..99ab3f8f389 100644 --- a/libcontainer/configs/validate/rootless.go +++ b/libcontainer/configs/validate/rootless.go @@ -2,7 +2,7 @@ package validate import ( "errors" - "fmt" + "strconv" "strings" "github.com/opencontainers/runc/libcontainer/configs" @@ -51,9 +51,8 @@ func rootlessEUIDMappings(config *configs.Config) error { return nil } -// mount verifies that the user isn't trying to set up any mounts they don't have -// the rights to do. In addition, it makes sure that no mount has a `uid=` or -// `gid=` option that doesn't resolve to root. +// rootlessEUIDMount verifies that all mounts have valid uid=/gid= options, +// i.e. their arguments has proper ID mappings. func rootlessEUIDMount(config *configs.Config) error { // XXX: We could whitelist allowed devices at this point, but I'm not // convinced that's a good idea. The kernel is the best arbiter of @@ -63,10 +62,9 @@ func rootlessEUIDMount(config *configs.Config) error { // Check that the options list doesn't contain any uid= or gid= entries // that don't resolve to root. for _, opt := range strings.Split(mount.Data, ",") { - if strings.HasPrefix(opt, "uid=") { - var uid int - n, err := fmt.Sscanf(opt, "uid=%d", &uid) - if n != 1 || err != nil { + if str := strings.TrimPrefix(opt, "uid="); len(str) < len(opt) { + uid, err := strconv.Atoi(str) + if err != nil { // Ignore unknown mount options. continue } @@ -75,10 +73,9 @@ func rootlessEUIDMount(config *configs.Config) error { } } - if strings.HasPrefix(opt, "gid=") { - var gid int - n, err := fmt.Sscanf(opt, "gid=%d", &gid) - if n != 1 || err != nil { + if str := strings.TrimPrefix(opt, "gid="); len(str) < len(opt) { + gid, err := strconv.Atoi(str) + if err != nil { // Ignore unknown mount options. continue } diff --git a/libcontainer/configs/validate/rootless_test.go b/libcontainer/configs/validate/rootless_test.go index 0657abf48dc..0e8df6eb297 100644 --- a/libcontainer/configs/validate/rootless_test.go +++ b/libcontainer/configs/validate/rootless_test.go @@ -141,3 +141,24 @@ func TestValidateRootlessEUIDMountGid(t *testing.T) { t.Errorf("Expected error to occur when setting gid=11 in mount options and GidMapping[0].size is 10") } } + +func BenchmarkRootlessEUIDMount(b *testing.B) { + config := rootlessEUIDConfig() + config.GidMappings[0].Size = 10 + config.Mounts = []*configs.Mount{ + { + Source: "devpts", + Destination: "/dev/pts", + Device: "devpts", + Data: "newinstance,ptmxmode=0666,mode=0620,uid=0,gid=5", + }, + } + + b.ResetTimer() + for i := 0; i < b.N; i++ { + err := rootlessEUIDMount(config) + if err != nil { + b.Fatal(err) + } + } +}