Skip to content

Commit

Permalink
configs: make id mappings int64 to better handle 32-bit
Browse files Browse the repository at this point in the history
Using ints for all of our mapping structures means that a 32-bit binary
errors out when trying to parse /proc/self/*id_map:

  failed to cache mappings for userns: failed to parse uid_map of userns /proc/1/ns/user:
  parsing id map failed: invalid format in line "         0          0 4294967295": integer overflow on token 4294967295

This issue was unearthed by commit 1912d59 ("*: actually support
joining a userns with a new container") but the underlying issue has
been present since the docker/libcontainer days.

In theory, switching to uint32 (to match the spec) instead of int64
would also work, but keeping everything signed seems much less
error-prone. It's also important to note that a mapping might be too
large for an int on 32-bit, so we detect this during the mapping.

Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
  • Loading branch information
cyphar committed Dec 14, 2023
1 parent ebcef3e commit 482e563
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 12 deletions.
6 changes: 3 additions & 3 deletions libcontainer/configs/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@ type Rlimit struct {

// IDMap represents UID/GID Mappings for User Namespaces.
type IDMap struct {
ContainerID int `json:"container_id"`
HostID int `json:"host_id"`
Size int `json:"size"`
ContainerID int64 `json:"container_id"`
HostID int64 `json:"host_id"`
Size int64 `json:"size"`
}

// Seccomp represents syscall restrictions
Expand Down
25 changes: 20 additions & 5 deletions libcontainer/configs/config_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package configs
import (
"errors"
"fmt"
"math"
)

var (
Expand Down Expand Up @@ -30,11 +31,18 @@ func (c Config) HostUID(containerId int) (int, error) {
if len(c.UIDMappings) == 0 {
return -1, errNoUIDMap
}
id, found := c.hostIDFromMapping(containerId, c.UIDMappings)
id, found := c.hostIDFromMapping(int64(containerId), c.UIDMappings)
if !found {
return -1, fmt.Errorf("user namespaces enabled, but no mapping found for uid %d", containerId)
}
return id, nil
// If we are a 32-bit binary running on a 64-bit system, it's possible
// the mapped user is too large to store in an int, which means we
// cannot do the mapping. We can't just return an int64, because
// os.Setuid() takes an int.
if id > math.MaxInt {
return -1, fmt.Errorf("mapping for uid %d (host id %d) is larger than native integer size (%d)", containerId, id, math.MaxInt)
}
return int(id), nil
}
// Return unchanged id.
return containerId, nil
Expand All @@ -53,11 +61,18 @@ func (c Config) HostGID(containerId int) (int, error) {
if len(c.GIDMappings) == 0 {
return -1, errNoGIDMap
}
id, found := c.hostIDFromMapping(containerId, c.GIDMappings)
id, found := c.hostIDFromMapping(int64(containerId), c.GIDMappings)
if !found {
return -1, fmt.Errorf("user namespaces enabled, but no mapping found for gid %d", containerId)
}
return id, nil
// If we are a 32-bit binary running on a 64-bit system, it's possible
// the mapped user is too large to store in an int, which means we
// cannot do the mapping. We can't just return an int64, because
// os.Setgid() takes an int.
if id > math.MaxInt {
return -1, fmt.Errorf("mapping for gid %d (host id %d) is larger than native integer size (%d)", containerId, id, math.MaxInt)
}
return int(id), nil
}
// Return unchanged id.
return containerId, nil
Expand All @@ -71,7 +86,7 @@ func (c Config) HostRootGID() (int, error) {

// Utility function that gets a host ID for a container ID from user namespace map
// if that ID is present in the map.
func (c Config) hostIDFromMapping(containerID int, uMap []IDMap) (int, bool) {
func (c Config) hostIDFromMapping(containerID int64, uMap []IDMap) (int64, bool) {
for _, m := range uMap {
if (containerID >= m.ContainerID) && (containerID <= (m.ContainerID + m.Size - 1)) {
hostID := m.HostID + (containerID - m.ContainerID)
Expand Down
2 changes: 1 addition & 1 deletion libcontainer/container_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -1351,7 +1351,7 @@ func ignoreTerminateErrors(err error) error {

func requiresRootOrMappingTool(c *configs.Config) bool {
gidMap := []configs.IDMap{
{ContainerID: 0, HostID: os.Getegid(), Size: 1},
{ContainerID: 0, HostID: int64(os.Getegid()), Size: 1},
}
return !reflect.DeepEqual(c.GIDMappings, gidMap)
}
6 changes: 3 additions & 3 deletions libcontainer/specconv/spec_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -522,9 +522,9 @@ func toConfigIDMap(specMaps []specs.LinuxIDMapping) []configs.IDMap {
idmaps := make([]configs.IDMap, len(specMaps))
for i, id := range specMaps {
idmaps[i] = configs.IDMap{
ContainerID: int(id.ContainerID),
HostID: int(id.HostID),
Size: int(id.Size),
ContainerID: int64(id.ContainerID),
HostID: int64(id.HostID),
Size: int64(id.Size),
}
}
return idmaps
Expand Down

0 comments on commit 482e563

Please sign in to comment.