diff --git a/patina_dxe_core/src/driver_services.rs b/patina_dxe_core/src/driver_services.rs index 968b2dfd0..26975087c 100644 --- a/patina_dxe_core/src/driver_services.rs +++ b/patina_dxe_core/src/driver_services.rs @@ -390,7 +390,7 @@ extern "efiapi" fn connect_controller( /// valid for the duration of its execution. For example, if a driver were be unloaded in a timer callback after /// returning true from Supported() before Start() is called, then the driver binding instance would be uninstalled or /// invalid and Start() would be an invalid function pointer when invoked. In general, the spec implicitly assumes -/// that driver binding instances that are valid at the start of he call to ConnectController() must remain valid for +/// that driver binding instances that are valid at the start of the call to ConnectController() must remain valid for /// the duration of the ConnectController() call. If this is not true, then behavior is undefined. This function is /// marked unsafe for this reason. /// @@ -481,12 +481,23 @@ pub unsafe fn core_disconnect_controller( child_handles.retain(|x| child_set.insert(*x)); let total_children = child_handles.len(); - let mut is_only_child = false; + let mut child_handle_valid = false; if let Some(handle) = child_handle { - //if the child was specified, but was the only child, then the driver should be disconnected. - //if the child was specified, but other children were present, then the driver should not be disconnected. - child_handles.retain(|x| x == &handle); - is_only_child = total_children == child_handles.len(); + // Check if the specified child handle is in the list of children for this driver. + // If found, filter to only that child. If not found, child_handle_valid stays false. + child_handles.retain(|x| { + if x == &handle { + child_handle_valid = true; + true + } else { + false + } + }); + } + + // If a specific child handle was requested but wasn't found, skip this driver. + if child_handle.is_some() && !child_handle_valid { + continue; } //resolve the handle to the driver_binding. @@ -516,7 +527,7 @@ pub unsafe fn core_disconnect_controller( child_handles.as_mut_ptr(), ); } - if status == efi::Status::SUCCESS && (child_handle.is_none() || is_only_child) { + if status == efi::Status::SUCCESS && (child_handle.is_none() || total_children == child_handles.len()) { status = (driver_binding.stop)(driver_binding_interface, controller_handle, 0, core::ptr::null_mut()); } if status == efi::Status::SUCCESS { @@ -1763,6 +1774,282 @@ mod tests { }); } + #[test] + fn test_disconnect_specific_child_not_managed_by_driver() { + // Verifies that when a specific child handle is requested but NOT managed by + // any driver, the driver is skipped and returns not found + with_locked_state(|| { + use std::sync::atomic::{AtomicUsize, Ordering}; + + static STOP_CALLS: AtomicUsize = AtomicUsize::new(0); + + extern "efiapi" fn mock_stop_tracking( + _this: *mut efi::protocols::driver_binding::Protocol, + _controller_handle: efi::Handle, + _num_children: usize, + _child_handle_buffer: *mut efi::Handle, + ) -> efi::Status { + STOP_CALLS.fetch_add(1, Ordering::SeqCst); + efi::Status::SUCCESS + } + + let guid1 = patina::Guid::from_string("0e896c7a-57dc-4987-bc22-abc3a8263210"); + let interface1: *mut c_void = 0x1234 as *mut c_void; + let (handle1, _) = PROTOCOL_DB.install_protocol_interface(None, guid1.to_efi_guid(), interface1).unwrap(); + + // Create controller handle + let (controller_handle, _) = PROTOCOL_DB + .install_protocol_interface( + None, + efi::protocols::device_path::PROTOCOL_GUID, + 0x1111 as *mut core::ffi::c_void, + ) + .unwrap(); + + // Create driver handle + let (driver_handle, _) = PROTOCOL_DB + .install_protocol_interface( + None, + efi::protocols::device_path::PROTOCOL_GUID, + 0x2222 as *mut core::ffi::c_void, + ) + .unwrap(); + + // Create a child handle that is managed by the driver + let (managed_child_handle, _) = PROTOCOL_DB + .install_protocol_interface( + None, + efi::protocols::device_path::PROTOCOL_GUID, + 0x3333 as *mut core::ffi::c_void, + ) + .unwrap(); + + // Create a child handle that is not managed by the driver + let (unmanaged_child_handle, _) = PROTOCOL_DB + .install_protocol_interface( + None, + efi::protocols::device_path::PROTOCOL_GUID, + 0x4444 as *mut core::ffi::c_void, + ) + .unwrap(); + + // Create driver binding protocol + let binding = Box::new(efi::protocols::driver_binding::Protocol { + version: 10, + supported: mock_supported_success, + start: mock_start_success, + stop: mock_stop_tracking, + driver_binding_handle: driver_handle, + image_handle: DXE_CORE_HANDLE, + }); + let binding_ptr = Box::into_raw(binding) as *mut core::ffi::c_void; + + PROTOCOL_DB + .install_protocol_interface( + Some(driver_handle), + efi::protocols::driver_binding::PROTOCOL_GUID, + binding_ptr, + ) + .unwrap(); + + // Have the driver manage the controller + PROTOCOL_DB + .add_protocol_usage( + controller_handle, + efi::protocols::device_path::PROTOCOL_GUID, + Some(driver_handle), + Some(handle1), + efi::OPEN_PROTOCOL_BY_DRIVER, + ) + .unwrap(); + + // The driver only manages managed_child_handle, not unmanaged_child_handle + PROTOCOL_DB + .add_protocol_usage( + controller_handle, + efi::protocols::device_path::PROTOCOL_GUID, + Some(driver_handle), + Some(managed_child_handle), + efi::OPEN_PROTOCOL_BY_CHILD_CONTROLLER, + ) + .unwrap(); + + STOP_CALLS.store(0, Ordering::SeqCst); + + // Attempt to disconnect unmanaged_child_handle (not managed by this driver) + unsafe { + let result = core_disconnect_controller(controller_handle, None, Some(unmanaged_child_handle)); + // Should return not found since no driver manages this child + assert!(result.is_err(), "disconnect should fail when child not managed by any driver"); + if let Err(err) = result { + assert_eq!(err, EfiError::NotFound, "Should return NotFound for an unmanaged child"); + } + } + + // Driver's stop function should NOT have been called since child wasn't found + assert_eq!( + STOP_CALLS.load(Ordering::SeqCst), + 0, + "Driver stop should not be called when specified child is not managed by driver" + ); + }); + } + + #[test] + fn test_disconnect_specific_child_among_multiple_children() { + // Checks that when a specific child is requested and the driver has multiple children, + // only that specific child is disconnected (not the entire driver). + with_locked_state(|| { + use std::sync::atomic::{AtomicUsize, Ordering}; + + static CHILD_STOP_CALLS: AtomicUsize = AtomicUsize::new(0); + static DRIVER_STOP_CALLS: AtomicUsize = AtomicUsize::new(0); + + extern "efiapi" fn mock_stop_tracking( + _this: *mut efi::protocols::driver_binding::Protocol, + _controller_handle: efi::Handle, + num_children: usize, + _child_handle_buffer: *mut efi::Handle, + ) -> efi::Status { + if num_children == 0 { + DRIVER_STOP_CALLS.fetch_add(1, Ordering::SeqCst); + } else { + CHILD_STOP_CALLS.fetch_add(1, Ordering::SeqCst); + } + efi::Status::SUCCESS + } + + let guid1 = patina::Guid::from_string("0e896c7a-57dc-4987-bc22-abc3a8263210"); + let interface1: *mut c_void = 0x1234 as *mut c_void; + let (handle1, _) = PROTOCOL_DB.install_protocol_interface(None, guid1.to_efi_guid(), interface1).unwrap(); + + // Create controller handle + let (controller_handle, _) = PROTOCOL_DB + .install_protocol_interface( + None, + efi::protocols::device_path::PROTOCOL_GUID, + 0x1111 as *mut core::ffi::c_void, + ) + .unwrap(); + + // Create driver handle + let (driver_handle, _) = PROTOCOL_DB + .install_protocol_interface( + None, + efi::protocols::device_path::PROTOCOL_GUID, + 0x2222 as *mut core::ffi::c_void, + ) + .unwrap(); + + // Create multiple child handles + let (child_handle_a, _) = PROTOCOL_DB + .install_protocol_interface( + None, + efi::protocols::device_path::PROTOCOL_GUID, + 0x3333 as *mut core::ffi::c_void, + ) + .unwrap(); + + let (child_handle_b, _) = PROTOCOL_DB + .install_protocol_interface( + None, + efi::protocols::device_path::PROTOCOL_GUID, + 0x4444 as *mut core::ffi::c_void, + ) + .unwrap(); + + let (child_handle_c, _) = PROTOCOL_DB + .install_protocol_interface( + None, + efi::protocols::device_path::PROTOCOL_GUID, + 0x5555 as *mut core::ffi::c_void, + ) + .unwrap(); + + // Create driver binding protocol + let binding = Box::new(efi::protocols::driver_binding::Protocol { + version: 10, + supported: mock_supported_success, + start: mock_start_success, + stop: mock_stop_tracking, + driver_binding_handle: driver_handle, + image_handle: DXE_CORE_HANDLE, + }); + let binding_ptr = Box::into_raw(binding) as *mut core::ffi::c_void; + + PROTOCOL_DB + .install_protocol_interface( + Some(driver_handle), + efi::protocols::driver_binding::PROTOCOL_GUID, + binding_ptr, + ) + .unwrap(); + + // Have the driver manage the controller + PROTOCOL_DB + .add_protocol_usage( + controller_handle, + efi::protocols::device_path::PROTOCOL_GUID, + Some(driver_handle), + Some(handle1), + efi::OPEN_PROTOCOL_BY_DRIVER, + ) + .unwrap(); + + // Add both child handles as managed by this driver + PROTOCOL_DB + .add_protocol_usage( + controller_handle, + efi::protocols::device_path::PROTOCOL_GUID, + Some(driver_handle), + Some(child_handle_a), + efi::OPEN_PROTOCOL_BY_CHILD_CONTROLLER, + ) + .unwrap(); + + PROTOCOL_DB + .add_protocol_usage( + controller_handle, + efi::protocols::device_path::PROTOCOL_GUID, + Some(driver_handle), + Some(child_handle_b), + efi::OPEN_PROTOCOL_BY_CHILD_CONTROLLER, + ) + .unwrap(); + + PROTOCOL_DB + .add_protocol_usage( + controller_handle, + efi::protocols::device_path::PROTOCOL_GUID, + Some(driver_handle), + Some(child_handle_c), + efi::OPEN_PROTOCOL_BY_CHILD_CONTROLLER, + ) + .unwrap(); + + CHILD_STOP_CALLS.store(0, Ordering::SeqCst); + DRIVER_STOP_CALLS.store(0, Ordering::SeqCst); + + // Disconnect only child_handle_a (one of the two children) + // Because total_children != child_handles.len(), the driver should not be fully stopped + unsafe { + let result = core_disconnect_controller(controller_handle, None, Some(child_handle_a)); + assert!(result.is_ok(), "disconnect should succeed"); + } + + assert_eq!( + CHILD_STOP_CALLS.load(Ordering::SeqCst), + 1, + "Child stop should be called once for the specified child" + ); + assert_eq!( + DRIVER_STOP_CALLS.load(Ordering::SeqCst), + 0, + "Driver stop should NOT be called when other children still exist" + ); + }); + } + #[test] fn test_core_disconnect_controller_invalid_child_handle() { with_locked_state(|| {