Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lxd/device: unix-hotplug ownership fix #14417

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions doc/api-extensions.md
Original file line number Diff line number Diff line change
Expand Up @@ -2520,3 +2520,7 @@ contains the total available logical CPUs available when LXD started.
## `vm_limits_cpu_pin_strategy`

Adds a new {config:option}`instance-resource-limits:limits.cpu.pin_strategy` configuration option for virtual machines. This option controls the CPU pinning strategy. When set to `none`, CPU auto pinning is disabled. When set to `auto`, CPU auto pinning is enabled.

## `unix_hotplug_ownership_inherit`

Adds a new {config:option}`device-unix-hotplug-device-conf:ownership.inherit` configuration option for `unix-hotplug` devices. This option controls whether the device inherits ownership (GID and UID) from the host. When set to `true`, host ownership is inherited. When set to `false`, host ownership is not inherited and ownership can be configured by setting {config:option}`device-unix-hotplug-device-conf:gid` and {config:option}`device-unix-hotplug-device-conf:uid`.
7 changes: 7 additions & 0 deletions doc/metadata.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1545,6 +1545,13 @@ See {ref}`devices-unix-char-hotplugging` for more information.

```

```{config:option} ownership.inherit device-unix-hotplug-device-conf
:defaultdesc: "`false`"
:shortdesc: "Whether this device inherits ownership (GID and UID) from the host"
:type: "bool"

```

```{config:option} productid device-unix-hotplug-device-conf
:shortdesc: "Product ID of the Unix device"
:type: "string"
Expand Down
70 changes: 44 additions & 26 deletions lxd/device/device_utils_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,16 @@ import (
// unixDefaultMode default mode to create unix devices with if not specified in device config.
const unixDefaultMode = 0660

// unixDeviceAttributes returns the decice type, major and minor numbers for a device.
func unixDeviceAttributes(path string) (string, uint32, uint32, error) {
// unixDeviceAttributes returns the device type, major and minor numbers for a device.
func unixDeviceAttributes(path string) (dType string, major uint32, minor uint32, err error) {
kadinsayani marked this conversation as resolved.
Show resolved Hide resolved
// Get a stat struct from the provided path
stat := unix.Stat_t{}
err := unix.Stat(path, &stat)
err = unix.Stat(path, &stat)
if err != nil {
return "", 0, 0, err
}

// Check what kind of file it is
dType := ""
if stat.Mode&unix.S_IFMT == unix.S_IFBLK {
dType = "b"
} else if stat.Mode&unix.S_IFMT == unix.S_IFCHR {
Expand All @@ -40,11 +39,26 @@ func unixDeviceAttributes(path string) (string, uint32, uint32, error) {
}

// Return the device information
major := unix.Major(uint64(stat.Rdev))
minor := unix.Minor(uint64(stat.Rdev))
major = unix.Major(uint64(stat.Rdev))
minor = unix.Minor(uint64(stat.Rdev))

return dType, major, minor, nil
}

// unixDeviceOwnership returns the ownership (gid and uid) for a device.
func unixDeviceOwnership(path string) (gid uint32, uid uint32, err error) {
stat := unix.Stat_t{}
err = unix.Stat(path, &stat)
if err != nil {
return 0, 0, err
}

gid = stat.Gid
uid = stat.Uid

return gid, uid, nil
}

// unixDeviceModeOct converts a string unix octal mode to an int.
func unixDeviceModeOct(strmode string) (int, error) {
i, err := strconv.ParseInt(strmode, 8, 32)
Expand Down Expand Up @@ -97,7 +111,7 @@ func unixDeviceDestPath(m deviceConfig.Device) string {
// defaultMode is set as true, then the device is created with the supplied or default mode (0660)
// respectively, otherwise the origin device's mode is used. If the device config doesn't contain a
// type field then it defaults to created a unix-char device. The ownership of the created device
// defaults to root (0) but can be specified with the uid and gid fields in the device config map.
// defaults to root (0) but can be specified with the uid and gid fields in the device config map. If ownership.inherit is set to true, the device ownership is inherited from the host.
// It returns a UnixDevice containing information about the device created.
func UnixDeviceCreate(s *state.State, idmapSet *idmap.IdmapSet, devicesPath string, prefix string, m deviceConfig.Device, defaultMode bool) (*UnixDevice, error) {
var err error
Expand Down Expand Up @@ -170,18 +184,28 @@ func UnixDeviceCreate(s *state.State, idmapSet *idmap.IdmapSet, devicesPath stri
d.Type = "c"
}

// Get the device owner.
if m["uid"] != "" {
d.UID, err = strconv.Atoi(m["uid"])
if shared.IsTrue(m["ownership.inherit"]) {
gid, uid, err := unixDeviceOwnership(srcPath)
if err != nil {
return nil, fmt.Errorf("Invalid uid %s in device %s", m["uid"], srcPath)
return nil, fmt.Errorf("Failed to retrieve host ownership of device %s: %w", srcPath, err)
}
}

if m["gid"] != "" {
d.GID, err = strconv.Atoi(m["gid"])
if err != nil {
return nil, fmt.Errorf("Invalid gid %s in device %s", m["gid"], srcPath)
d.GID = int(gid)
d.UID = int(uid)
kadinsayani marked this conversation as resolved.
Show resolved Hide resolved
} else {
// Get the device owner.
if m["uid"] != "" {
d.UID, err = strconv.Atoi(m["uid"])
if err != nil {
return nil, fmt.Errorf("Invalid uid %s in device %s", m["uid"], srcPath)
}
}

if m["gid"] != "" {
d.GID, err = strconv.Atoi(m["gid"])
if err != nil {
return nil, fmt.Errorf("Invalid gid %s in device %s", m["gid"], srcPath)
}
}
}

Expand Down Expand Up @@ -326,12 +350,9 @@ func unixDeviceSetup(s *state.State, devicesPath string, typePrefix string, devi
// device to ascertain these attributes. If defaultMode is true or mode is supplied in the device
// config then the origin device does not need to be accessed for its file mode.
func unixDeviceSetupCharNum(s *state.State, devicesPath string, typePrefix string, deviceName string, m deviceConfig.Device, major uint32, minor uint32, path string, defaultMode bool, runConf *deviceConfig.RunConfig) error {
configCopy := deviceConfig.Device{}
for k, v := range m {
configCopy[k] = v
}
configCopy := m.Clone()

// Overridng these in the config copy should avoid the need for unixDeviceSetup to stat
// Overriding these in the config copy should avoid the need for unixDeviceSetup to stat
// the origin device to ascertain this information.
configCopy["type"] = "unix-char"
configCopy["major"] = fmt.Sprintf("%d", major)
Expand All @@ -347,12 +368,9 @@ func unixDeviceSetupCharNum(s *state.State, devicesPath string, typePrefix strin
// device to ascertain these attributes. If defaultMode is true or mode is supplied in the device
// config then the origin device does not need to be accessed for its file mode.
func unixDeviceSetupBlockNum(s *state.State, devicesPath string, typePrefix string, deviceName string, m deviceConfig.Device, major uint32, minor uint32, path string, defaultMode bool, runConf *deviceConfig.RunConfig) error {
configCopy := deviceConfig.Device{}
for k, v := range m {
configCopy[k] = v
}
configCopy := m.Clone()

// Overridng these in the config copy should avoid the need for unixDeviceSetup to stat
// Overriding these in the config copy should avoid the need for unixDeviceSetup to stat
// the origin device to ascertain this information.
configCopy["type"] = "unix-block"
configCopy["major"] = fmt.Sprintf("%d", major)
Expand Down
12 changes: 12 additions & 0 deletions lxd/device/unix_hotplug.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,14 @@ func (d *unixHotplug) validateConfig(instConf instance.ConfigReader) error {
"gid": unixValidUserID,
"mode": unixValidOctalFileMode,
"required": validate.Optional(validate.IsBool),

// lxdmeta:generate(entities=device-unix-hotplug; group=device-conf; key=ownership.inherit)
//
// ---
// type: bool
// defaultdesc: `false`
// shortdesc: Whether this device inherits ownership (GID and UID) from the host
"ownership.inherit": validate.Optional(validate.IsBool),
}

err := d.config.Validate(rules)
Expand All @@ -84,6 +92,10 @@ func (d *unixHotplug) validateConfig(instConf instance.ConfigReader) error {
return fmt.Errorf("Unix hotplug devices require a vendorid or a productid")
}

if d.config["gid"] != "" || d.config["uid"] != "" && shared.IsTrue(d.config["ownership.inherit"]) {
return fmt.Errorf("Unix hotplug device ownership cannot be inherited from host while GID or UID is set")
}

return nil
}

Expand Down
8 changes: 8 additions & 0 deletions lxd/metadata/configuration.json
Original file line number Diff line number Diff line change
Expand Up @@ -1783,6 +1783,14 @@
"type": "integer"
}
},
{
"ownership.inherit": {
"defaultdesc": "`false`",
"longdesc": "",
"shortdesc": "Whether this device inherits ownership (GID and UID) from the host",
"type": "bool"
}
},
{
"productid": {
"longdesc": "",
Expand Down
1 change: 1 addition & 0 deletions shared/version/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -423,6 +423,7 @@ var APIExtensions = []string{
"network_ovn_uplink_vlan",
"state_logical_cpus",
"vm_limits_cpu_pin_strategy",
"unix_hotplug_ownership_inherit",
}

// APIExtensionsCount returns the number of available API extensions.
Expand Down
Loading