Skip to content

Commit

Permalink
oci/deviceFromPath(): correctly check device types
Browse files Browse the repository at this point in the history
This ports the changes of opencontainers/runc@95a59bf
to this repository.

From that PR:

    (mode&S_IFCHR == S_IFCHR) is the wrong way of checking the type of an
    inode because the S_IF* bits are actually not a bitmask and instead must
    be checked using S_IF*. This bug was neatly hidden behind a (major == 0)
    sanity-check but that was removed by [1].

    In addition, add a test that makes sure that HostDevices() doesn't give
    rubbish results -- because we broke this and fixed this before[2].

    [1]: opencontainers/runc@24388be ("configs: use different types for .Devices and .Resources.Devices")
    [2]: opencontainers/runc@3ed492a ("Handle non-devices correctly in DeviceFromPath")

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
  • Loading branch information
thaJeztah committed Dec 1, 2021
1 parent 591d709 commit 94462d8
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 8 deletions.
29 changes: 21 additions & 8 deletions oci/utils_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,9 @@ func getDevices(path, containerPath string) ([]specs.LinuxDevice, error) {
}
return nil, err
}
if device.Type == fifoDevice {
continue
}
if containerPath != "" {
device.Path = filepath.Join(containerPath, filepath.Base(f.Name()))
}
Expand All @@ -99,6 +102,14 @@ func getDevices(path, containerPath string) ([]specs.LinuxDevice, error) {
return out, nil
}

// TODO consider adding these consts to the OCI runtime-spec.
const (
wildcardDevice = "a" //nolint // currently unused, but should be included when upstreaming to OCI runtime-spec.
blockDevice = "b"
charDevice = "c" // or "u"
fifoDevice = "p"
)

func deviceFromPath(path string) (*specs.LinuxDevice, error) {
var stat unix.Stat_t
if err := unix.Lstat(path, &stat); err != nil {
Expand All @@ -110,19 +121,21 @@ func deviceFromPath(path string) (*specs.LinuxDevice, error) {
major = unix.Major(devNumber)
minor = unix.Minor(devNumber)
)
if major == 0 {
return nil, errNotADevice
}

var (
devType string
mode = stat.Mode
)
switch {
case mode&unix.S_IFBLK == unix.S_IFBLK:
devType = "b"
case mode&unix.S_IFCHR == unix.S_IFCHR:
devType = "c"

switch mode & unix.S_IFMT {
case unix.S_IFBLK:
devType = blockDevice
case unix.S_IFCHR:
devType = charDevice
case unix.S_IFIFO:
devType = fifoDevice
default:
return nil, errNotADevice
}
fm := os.FileMode(mode &^ unix.S_IFMT)
return &specs.LinuxDevice{
Expand Down
44 changes: 44 additions & 0 deletions oci/utils_unix_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
//go:build !windows && !darwin
// +build !windows,!darwin

/*
Copyright The containerd Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package oci

import "testing"

func TestHostDevicesAllValid(t *testing.T) {
devices, err := HostDevices()
if err != nil {
t.Fatalf("failed to get host devices: %v", err)
}

for _, device := range devices {
// Devices can't have major number 0.
if device.Major == 0 {
t.Errorf("device entry %+v has zero major number", device)
}
switch device.Type {
case blockDevice, charDevice:
case fifoDevice:
t.Logf("fifo devices shouldn't show up from HostDevices")
fallthrough
default:
t.Errorf("device entry %+v has unexpected type %v", device, device.Type)
}
}
}

0 comments on commit 94462d8

Please sign in to comment.