Skip to content
This repository has been archived by the owner on May 12, 2021. It is now read-only.

Commit

Permalink
device: Check type as well as major:minor when looking up devices
Browse files Browse the repository at this point in the history
To update device resource entries from host to guest, we search for the
right entry by host major:minor numbers, then later update it.  However
block and character devices exist in separate major:minor namespaces so
we could have one block and one character device with matching major:minor
and thus incorrectly update both with the details for whichever device is
processed second.

Add a check on device type to prevent this.

Fixes: #835

Signed-off-by: David Gibson <[email protected]>
  • Loading branch information
dgibson committed Sep 17, 2020
1 parent d88d468 commit 27ebdc9
Show file tree
Hide file tree
Showing 2 changed files with 71 additions and 1 deletion.
2 changes: 1 addition & 1 deletion device.go
Original file line number Diff line number Diff line change
Expand Up @@ -446,7 +446,7 @@ func makeDevIndex(spec *pb.Spec) devIndex {

if spec.Linux.Resources != nil && spec.Linux.Resources.Devices != nil {
for j, r := range spec.Linux.Resources.Devices {
if r.Major == d.Major && r.Minor == d.Minor {
if r.Type == d.Type && r.Major == d.Major && r.Minor == d.Minor {
rIdx = append(rIdx, j)
}
}
Expand Down
70 changes: 70 additions & 0 deletions device_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -615,6 +615,76 @@ func TestUpdateSpecDeviceListGuestHostConflict(t *testing.T) {
assert.Equal(guestMinorB, spec.Linux.Resources.Devices[1].Minor)
}

// Test handling in the case that the host has a block device and a
// character device with the same major:minor, but the equivalent
// guest devices do *not* have the same major:minor
func TestUpdateSpecDeviceListCharBlockConflict(t *testing.T) {
assert := assert.New(t)

var nullStat unix.Stat_t
err := unix.Stat("/dev/null", &nullStat)
assert.NoError(err)

guestMajor := int64(unix.Major(nullStat.Rdev))
guestMinor := int64(unix.Minor(nullStat.Rdev))

hostMajor := int64(99)
hostMinor := int64(99)

spec := &pb.Spec{
Linux: &pb.Linux{
Devices: []pb.LinuxDevice{
{
Path: "/dev/char",
Type: "c",
Major: hostMajor,
Minor: hostMinor,
},
{
Path: "/dev/block",
Type: "b",
Major: hostMajor,
Minor: hostMinor,
},
},
Resources: &pb.LinuxResources{
Devices: []pb.LinuxDeviceCgroup{
{
Type: "c",
Major: hostMajor,
Minor: hostMinor,
},
{
Type: "b",
Major: hostMajor,
Minor: hostMinor,
},
},
},
},
}

dev := pb.Device{
ContainerPath: "/dev/char",
VmPath: "/dev/null",
}

assert.Equal(hostMajor, spec.Linux.Resources.Devices[0].Major)
assert.Equal(hostMinor, spec.Linux.Resources.Devices[0].Minor)
assert.Equal(hostMajor, spec.Linux.Resources.Devices[1].Major)
assert.Equal(hostMinor, spec.Linux.Resources.Devices[1].Minor)

devIdx := makeDevIndex(spec)
err = updateSpecDeviceList(dev, spec, devIdx)
assert.NoError(err)

// Only the char device, not the block device should be updated
assert.Equal(guestMajor, spec.Linux.Resources.Devices[0].Major)
assert.Equal(guestMinor, spec.Linux.Resources.Devices[0].Minor)
assert.Equal(hostMajor, spec.Linux.Resources.Devices[1].Major)
assert.Equal(hostMinor, spec.Linux.Resources.Devices[1].Minor)
}

func TestRescanPciBus(t *testing.T) {
skipUnlessRoot(t)

Expand Down

0 comments on commit 27ebdc9

Please sign in to comment.