Skip to content

Commit 81f712e

Browse files
authored
fix(drivers): validate vendor-agnostic CDI device names (#4306)
Reject host paths and malformed CDI selectors for Docker and Podman using shared qualified-name validation. Preserve the generic GPU request checks for drivers using other identifier formats, including VM PCI addresses. Validate explicit and resolved CDI selectors, cover vendor-agnostic names and rejected paths, and update the related documentation. Signed-off-by: Adrien Langou <alangou@nvidia.com> Signed-off-by: Evan Lezar <elezar@nvidia.com>
1 parent 67adcf1 commit 81f712e

8 files changed

Lines changed: 342 additions & 20 deletions

File tree

‎crates/openshell-core/src/gpu.rs‎

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -302,6 +302,62 @@ fn cdi_nvidia_gpu_suffix(id: &str) -> Option<&str> {
302302
id.strip_prefix(CDI_NVIDIA_GPU_PREFIX)
303303
}
304304

305+
// Local equivalent of the upstream CDI parser until its public API is released:
306+
// https://github.com/cncf-tags/container-device-interface-rs/pull/177
307+
fn is_qualified_name(device: &str) -> bool {
308+
let Some((kind, name)) = device.split_once('=') else {
309+
return false;
310+
};
311+
let Some((vendor, class)) = kind.split_once('/') else {
312+
return false;
313+
};
314+
let valid_component = |value: &str, device_name: bool| {
315+
let bytes = value.as_bytes();
316+
bytes.first().is_some_and(|byte| {
317+
if device_name {
318+
byte.is_ascii_alphanumeric()
319+
} else {
320+
byte.is_ascii_alphabetic()
321+
}
322+
}) && bytes.last().is_some_and(u8::is_ascii_alphanumeric)
323+
&& bytes.iter().all(|byte| {
324+
byte.is_ascii_alphanumeric()
325+
|| matches!(byte, b'_' | b'-' | b'.')
326+
|| (device_name && *byte == b':')
327+
})
328+
};
329+
valid_component(vendor, false) && valid_component(class, false) && valid_component(name, true)
330+
}
331+
332+
/// Validate vendor-agnostic CDI qualified names without resolving or authorizing devices.
333+
///
334+
/// # Errors
335+
/// Returns an error for host paths or malformed `<vendor>/<class>=<name>` selectors.
336+
pub fn validate_cdi_device_names(devices: &[String], field: &str) -> Result<(), String> {
337+
if devices.iter().any(|device| !is_qualified_name(device)) {
338+
return Err(format!(
339+
"{field} must contain CDI qualified names (<vendor>/<class>=<name>)"
340+
));
341+
}
342+
Ok(())
343+
}
344+
345+
/// Validate explicit CDI selectors and their GPU request requirements.
346+
///
347+
/// Docker and Podman use this helper; drivers with other identifier formats use
348+
/// [`validate_specific_gpu_device_request`] directly.
349+
///
350+
/// # Errors
351+
/// Returns an error for malformed CDI names or invalid GPU request requirements.
352+
pub fn validate_cdi_gpu_device_request(
353+
gpu: Option<&DriverGpuResourceRequirements>,
354+
devices: &[String],
355+
field: &str,
356+
) -> Result<(), String> {
357+
validate_cdi_device_names(devices, field)?;
358+
validate_specific_gpu_device_request(gpu, devices, field)
359+
}
360+
305361
/// Validate a compute-driver GPU request against driver-owned specific devices.
306362
///
307363
/// Drivers call this when a sandbox request combines portable GPU requirements
@@ -347,6 +403,64 @@ pub fn validate_specific_gpu_device_request(
347403
mod tests {
348404
use super::*;
349405

406+
#[test]
407+
fn cdi_device_names_accept_vendor_agnostic_selectors() {
408+
for device in [
409+
"nvidia.com/gpu=0",
410+
"nvidia.com/gpu=all",
411+
"nvidia.com/gpu=GPU-5b2d",
412+
"nvidia.com/gpu=MIG-5b2d",
413+
"nvidia.com/gpu=0:1",
414+
"example.com/accelerator=card_0.1",
415+
"intel.com/gpu=0",
416+
"amd.com/gpu=0",
417+
"v/c=0",
418+
] {
419+
let devices = vec![device.to_string()];
420+
validate_cdi_device_names(&devices, "driver_config.cdi_devices")
421+
.unwrap_or_else(|error| panic!("{device:?}: {error}"));
422+
}
423+
}
424+
425+
#[test]
426+
fn cdi_device_names_reject_host_paths_and_malformed_selectors() {
427+
for device in [
428+
"",
429+
"/dev/sda",
430+
"/dev",
431+
"/dev/sda:/dev/sda:rwm",
432+
"dev/sda",
433+
"../dev/sda",
434+
"vendor/gpu",
435+
"/gpu=0",
436+
"vendor/=0",
437+
"vendor/gpu=",
438+
"vendor/gpu=/dev/sda",
439+
"vendor/gpu=0/1",
440+
"vendor/gpu=0=1",
441+
"vendor/gpu=0,1",
442+
"vendor/gpu=-0",
443+
"vendor/gpu=0:",
444+
"vendor/gpu=0 ",
445+
" vendor/gpu=0",
446+
"vendor/gpu=0\n",
447+
"vendor/gpu=0\0",
448+
"vendor/gpu=é",
449+
"véndor/gpu=0",
450+
"vendor/gpü=0",
451+
"0vendor/gpu=0",
452+
"vendor/0gpu=0",
453+
"vendor./gpu=0",
454+
"vendor/gpu-=0",
455+
"vendor/gpu/other=0",
456+
] {
457+
let error =
458+
validate_cdi_device_names(&[device.to_string()], "driver_config.cdi_devices")
459+
.unwrap_err();
460+
assert!(error.contains("CDI qualified names"), "{device:?}: {error}");
461+
}
462+
}
463+
350464
#[test]
351465
fn effective_driver_gpu_count_normalizes_missing_count() {
352466
let gpu = DriverGpuResourceRequirements { count: None };
@@ -547,6 +661,18 @@ mod tests {
547661
);
548662
}
549663

664+
#[test]
665+
fn specific_gpu_device_request_preserves_non_cdi_identifiers() {
666+
let gpu = DriverGpuResourceRequirements { count: Some(1) };
667+
let devices = vec!["0000:2d:00.0".to_string()];
668+
validate_specific_gpu_device_request(Some(&gpu), &devices, "driver_config.gpu_device_ids")
669+
.expect("generic GPU request validation must accept PCI device identifiers");
670+
let error =
671+
validate_cdi_gpu_device_request(Some(&gpu), &devices, "driver_config.cdi_devices")
672+
.expect_err("CDI drivers must reject non-CDI identifiers");
673+
assert!(error.contains("CDI qualified names"));
674+
}
675+
550676
#[test]
551677
fn validate_specific_gpu_device_request_ignores_empty_devices() {
552678
validate_specific_gpu_device_request(None, &[], "driver_config.cdi_devices")

‎crates/openshell-driver-docker/src/lib.rs‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ use openshell_core::driver_utils::{
3333
};
3434
use openshell_core::gpu::{
3535
CdiGpuDefaultSelector, CdiGpuInventory, CdiGpuSelectionError, driver_gpu_requirements,
36-
effective_driver_gpu_count, validate_specific_gpu_device_request,
36+
effective_driver_gpu_count, validate_cdi_device_names, validate_cdi_gpu_device_request,
3737
};
3838
use openshell_core::progress::{
3939
PROGRESS_STEP_PULLING_IMAGE, PROGRESS_STEP_REQUESTING_SANDBOX, PROGRESS_STEP_STARTING_SANDBOX,
@@ -1126,7 +1126,7 @@ impl DockerComputeDriver {
11261126
}
11271127

11281128
if let Some(cdi_devices) = driver_config.cdi_devices.as_deref() {
1129-
validate_specific_gpu_device_request(
1129+
validate_cdi_gpu_device_request(
11301130
gpu_requirements,
11311131
cdi_devices,
11321132
"driver_config.cdi_devices",
@@ -1309,7 +1309,7 @@ impl DockerComputeDriver {
13091309
) -> Result<Vec<String>, CdiGpuSelectionError>,
13101310
) -> Result<Option<Vec<String>>, Status> {
13111311
if let Some(cdi_devices) = driver_config.cdi_devices.as_deref() {
1312-
validate_specific_gpu_device_request(
1312+
validate_cdi_gpu_device_request(
13131313
gpu_requirements,
13141314
cdi_devices,
13151315
"driver_config.cdi_devices",
@@ -5618,12 +5618,8 @@ fn build_container_create_body(
56185618
.as_ref()
56195619
.and_then(|spec| driver_gpu_requirements(spec.resource_requirements.as_ref()));
56205620
let cdi_devices = if let Some(cdi_devices) = driver_config.cdi_devices.as_ref() {
5621-
validate_specific_gpu_device_request(
5622-
gpu_requirements,
5623-
cdi_devices,
5624-
"driver_config.cdi_devices",
5625-
)
5626-
.map_err(Status::invalid_argument)?;
5621+
validate_cdi_gpu_device_request(gpu_requirements, cdi_devices, "driver_config.cdi_devices")
5622+
.map_err(Status::invalid_argument)?;
56275623
Some(cdi_devices.as_slice())
56285624
} else {
56295625
None
@@ -5674,6 +5670,10 @@ fn build_container_create_body_for_image(
56745670
image: &DockerImageMetadata,
56755671
workload_identity: &ResolvedWorkloadIdentity,
56765672
) -> Result<ContainerCreateBody, Status> {
5673+
if let Some(device_ids) = gpu_device_ids {
5674+
validate_cdi_device_names(device_ids, "driver_config.cdi_devices")
5675+
.map_err(Status::invalid_argument)?;
5676+
}
56775677
let spec = sandbox
56785678
.spec
56795679
.as_ref()

‎crates/openshell-driver-docker/src/tests.rs‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2720,6 +2720,91 @@ fn build_container_create_body_omits_devices_without_resolved_default_cdi_device
27202720
);
27212721
}
27222722

2723+
#[test]
2724+
fn container_spec_accepts_non_nvidia_explicit_and_resolved_cdi_devices() {
2725+
let mut config = runtime_config();
2726+
config.gpu.cdi_supported = true;
2727+
let mut sandbox = test_sandbox();
2728+
sandbox.spec.as_mut().unwrap().resource_requirements = Some(gpu_resources(None));
2729+
for device in ["example.com/gpu=0", "intel.com/gpu=0", "amd.com/gpu=0"] {
2730+
sandbox
2731+
.spec
2732+
.as_mut()
2733+
.unwrap()
2734+
.template
2735+
.as_mut()
2736+
.unwrap()
2737+
.driver_config = Some(cdi_devices_config(&[device]));
2738+
let explicit = build_container_create_body(&sandbox, &config).unwrap();
2739+
sandbox
2740+
.spec
2741+
.as_mut()
2742+
.unwrap()
2743+
.template
2744+
.as_mut()
2745+
.unwrap()
2746+
.driver_config = None;
2747+
let resolved = build_container_create_body_with_gpu_devices(
2748+
&sandbox,
2749+
&config,
2750+
&DockerSandboxDriverConfig::default(),
2751+
Some(&[device.to_string()]),
2752+
)
2753+
.unwrap();
2754+
for body in [explicit, resolved] {
2755+
let requests = body.host_config.unwrap().device_requests.unwrap();
2756+
assert_eq!(requests[0].driver.as_deref(), Some("cdi"));
2757+
assert_eq!(
2758+
requests[0].device_ids.as_ref().unwrap(),
2759+
&[device.to_string()]
2760+
);
2761+
}
2762+
}
2763+
}
2764+
2765+
#[test]
2766+
fn container_spec_rejects_host_paths_in_explicit_and_resolved_cdi_devices() {
2767+
let mut config = runtime_config();
2768+
config.gpu.cdi_supported = true;
2769+
let mut sandbox = test_sandbox();
2770+
sandbox.spec.as_mut().unwrap().resource_requirements = Some(gpu_resources(None));
2771+
for device in [
2772+
"/dev/sda",
2773+
"/dev",
2774+
"/dev/sda:/dev/sda:rwm",
2775+
"vendor/gpu=0/1",
2776+
] {
2777+
sandbox
2778+
.spec
2779+
.as_mut()
2780+
.unwrap()
2781+
.template
2782+
.as_mut()
2783+
.unwrap()
2784+
.driver_config = Some(cdi_devices_config(&[device]));
2785+
let explicit_error = build_container_create_body(&sandbox, &config).unwrap_err();
2786+
sandbox
2787+
.spec
2788+
.as_mut()
2789+
.unwrap()
2790+
.template
2791+
.as_mut()
2792+
.unwrap()
2793+
.driver_config = None;
2794+
let resolved_error = build_container_create_body_with_gpu_devices(
2795+
&sandbox,
2796+
&config,
2797+
&DockerSandboxDriverConfig::default(),
2798+
Some(&[device.to_string()]),
2799+
)
2800+
.unwrap_err();
2801+
for error in [explicit_error, resolved_error] {
2802+
assert_eq!(error.code(), tonic::Code::InvalidArgument);
2803+
assert!(error.message().contains("CDI qualified names"));
2804+
}
2805+
}
2806+
}
2807+
27232808
#[test]
27242809
fn build_container_create_body_passes_explicit_cdi_device_id_through() {
27252810
let mut config = runtime_config();

0 commit comments

Comments
 (0)