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

Harden QueryInterface implementation #2660

Merged
merged 4 commits into from
Sep 15, 2023
Merged
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 crates/libs/bindgen/src/rust/delegates.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,10 @@ fn gen_win_delegate(writer: &Writer, def: TypeDef) -> TokenStream {
unsafe extern "system" fn QueryInterface(this: *mut ::core::ffi::c_void, iid: *const ::windows_core::GUID, interface: *mut *mut ::core::ffi::c_void) -> ::windows_core::HRESULT {
let this = this as *mut *mut ::core::ffi::c_void as *mut Self;

if iid.is_null() || interface.is_null() {
return ::windows_core::HRESULT(-2147467261); // E_POINTER
}

*interface = if *iid == <#ident as ::windows_core::ComInterface>::IID ||
*iid == <::windows_core::IUnknown as ::windows_core::ComInterface>::IID ||
*iid == <::windows_core::imp::IAgileObject as ::windows_core::ComInterface>::IID {
Expand Down
8 changes: 8 additions & 0 deletions crates/libs/core/src/imp/weak_ref_count.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,10 @@ impl TearOff {
unsafe extern "system" fn StrongQueryInterface(ptr: *mut std::ffi::c_void, iid: *const crate::GUID, interface: *mut *mut std::ffi::c_void) -> crate::HRESULT {
let this = Self::from_strong_ptr(ptr);

if iid.is_null() || interface.is_null() {
return ::windows_core::HRESULT(-2147467261); // E_POINTER
}

// Only directly respond to queries for the the tear-off's strong interface. This is
// effectively a self-query.
if *iid == IWeakReferenceSource::IID {
Expand All @@ -142,6 +146,10 @@ impl TearOff {
unsafe extern "system" fn WeakQueryInterface(ptr: *mut std::ffi::c_void, iid: *const crate::GUID, interface: *mut *mut std::ffi::c_void) -> crate::HRESULT {
let this = Self::from_weak_ptr(ptr);

if iid.is_null() || interface.is_null() {
return ::windows_core::HRESULT(-2147467261); // E_POINTER
}

// While the weak vtable is packed into the same allocation as the strong vtable and
// tear-off, it represents a distinct COM identity and thus does not share or delegate to
// the object.
Expand Down
58 changes: 28 additions & 30 deletions crates/libs/implement/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,16 +64,14 @@ pub fn implement(attributes: proc_macro::TokenStream, original_type: proc_macro:
}
}
impl #generics ::windows::core::AsImpl<#original_ident::#generics> for #interface_ident where #constraints {
// SAFETY: the offset is guranteed to be in bounds, and the implementation struct
// is guaranteed to live at least as long as `self`.
unsafe fn as_impl(&self) -> &#original_ident::#generics {
let this = ::windows::core::Interface::as_raw(self);
// SAFETY: the offset is guranteed to be in bounds, and the implementation struct
// is guaranteed to live at least as long as `self`.
unsafe {
// Subtract away the vtable offset plus 1, for the `identity` field, to get
// to the impl struct which contains that original implementation type.
let this = (this as *mut *mut ::core::ffi::c_void).sub(1 + #offset) as *mut #impl_ident::#generics;
&(*this).this
}
// Subtract away the vtable offset plus 1, for the `identity` field, to get
// to the impl struct which contains that original implementation type.
let this = (this as *mut *mut ::core::ffi::c_void).sub(1 + #offset) as *mut #impl_ident::#generics;
&(*this).this
}
}
}
Expand Down Expand Up @@ -105,27 +103,29 @@ pub fn implement(attributes: proc_macro::TokenStream, original_type: proc_macro:
&self.this
}
unsafe fn QueryInterface(&self, iid: *const ::windows::core::GUID, interface: *mut *mut ::core::ffi::c_void) -> ::windows::core::HRESULT {
unsafe {
*interface = if *iid == <::windows::core::IUnknown as ::windows::core::ComInterface>::IID
|| *iid == <::windows::core::IInspectable as ::windows::core::ComInterface>::IID
|| *iid == <::windows::core::imp::IAgileObject as ::windows::core::ComInterface>::IID {
&self.identity as *const _ as *mut _
} #(#queries)* else {
::core::ptr::null_mut()
};

if !(*interface).is_null() {
self.count.add_ref();
return ::windows::core::HRESULT(0);
}
if iid.is_null() || interface.is_null() {
return ::windows::core::HRESULT(-2147467261); // E_POINTER
}

*interface = self.count.query(iid, &self.identity as *const _ as *mut _);
*interface = if *iid == <::windows::core::IUnknown as ::windows::core::ComInterface>::IID
|| *iid == <::windows::core::IInspectable as ::windows::core::ComInterface>::IID
|| *iid == <::windows::core::imp::IAgileObject as ::windows::core::ComInterface>::IID {
&self.identity as *const _ as *mut _
} #(#queries)* else {
::core::ptr::null_mut()
};

if (*interface).is_null() {
::windows::core::HRESULT(0x8000_4002) // E_NOINTERFACE
} else {
::windows::core::HRESULT(0)
}
if !(*interface).is_null() {
self.count.add_ref();
return ::windows::core::HRESULT(0);
}

*interface = self.count.query(iid, &self.identity as *const _ as *mut _);

if (*interface).is_null() {
::windows::core::HRESULT(-2147467262) // E_NOINTERFACE
} else {
::windows::core::HRESULT(0)
}
}
fn AddRef(&self) -> u32 {
Expand All @@ -134,9 +134,7 @@ pub fn implement(attributes: proc_macro::TokenStream, original_type: proc_macro:
unsafe fn Release(&self) -> u32 {
let remaining = self.count.release();
if remaining == 0 {
unsafe {
_ = ::std::boxed::Box::from_raw(self as *const Self as *mut Self);
}
_ = ::std::boxed::Box::from_raw(self as *const Self as *mut Self);
}
remaining
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5826,6 +5826,9 @@ impl<F: FnMut(::core::option::Option<&IBackgroundTaskInstance>, BackgroundTaskCa
};
unsafe extern "system" fn QueryInterface(this: *mut ::core::ffi::c_void, iid: *const ::windows_core::GUID, interface: *mut *mut ::core::ffi::c_void) -> ::windows_core::HRESULT {
let this = this as *mut *mut ::core::ffi::c_void as *mut Self;
if iid.is_null() || interface.is_null() {
return ::windows_core::HRESULT(-2147467261);
}
*interface = if *iid == <BackgroundTaskCanceledEventHandler as ::windows_core::ComInterface>::IID || *iid == <::windows_core::IUnknown as ::windows_core::ComInterface>::IID || *iid == <::windows_core::imp::IAgileObject as ::windows_core::ComInterface>::IID { &mut (*this).vtable as *mut _ as _ } else { ::core::ptr::null_mut() };
if (*interface).is_null() {
::windows_core::HRESULT(-2147467262)
Expand Down Expand Up @@ -5897,6 +5900,9 @@ impl<F: FnMut(::core::option::Option<&BackgroundTaskRegistration>, ::core::optio
};
unsafe extern "system" fn QueryInterface(this: *mut ::core::ffi::c_void, iid: *const ::windows_core::GUID, interface: *mut *mut ::core::ffi::c_void) -> ::windows_core::HRESULT {
let this = this as *mut *mut ::core::ffi::c_void as *mut Self;
if iid.is_null() || interface.is_null() {
return ::windows_core::HRESULT(-2147467261);
}
*interface = if *iid == <BackgroundTaskCompletedEventHandler as ::windows_core::ComInterface>::IID || *iid == <::windows_core::IUnknown as ::windows_core::ComInterface>::IID || *iid == <::windows_core::imp::IAgileObject as ::windows_core::ComInterface>::IID { &mut (*this).vtable as *mut _ as _ } else { ::core::ptr::null_mut() };
if (*interface).is_null() {
::windows_core::HRESULT(-2147467262)
Expand Down Expand Up @@ -5968,6 +5974,9 @@ impl<F: FnMut(::core::option::Option<&BackgroundTaskRegistration>, ::core::optio
};
unsafe extern "system" fn QueryInterface(this: *mut ::core::ffi::c_void, iid: *const ::windows_core::GUID, interface: *mut *mut ::core::ffi::c_void) -> ::windows_core::HRESULT {
let this = this as *mut *mut ::core::ffi::c_void as *mut Self;
if iid.is_null() || interface.is_null() {
return ::windows_core::HRESULT(-2147467261);
}
*interface = if *iid == <BackgroundTaskProgressEventHandler as ::windows_core::ComInterface>::IID || *iid == <::windows_core::IUnknown as ::windows_core::ComInterface>::IID || *iid == <::windows_core::imp::IAgileObject as ::windows_core::ComInterface>::IID { &mut (*this).vtable as *mut _ as _ } else { ::core::ptr::null_mut() };
if (*interface).is_null() {
::windows_core::HRESULT(-2147467262)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3304,6 +3304,9 @@ impl<F: FnMut(::core::option::Option<&DataProviderRequest>) -> ::windows_core::R
};
unsafe extern "system" fn QueryInterface(this: *mut ::core::ffi::c_void, iid: *const ::windows_core::GUID, interface: *mut *mut ::core::ffi::c_void) -> ::windows_core::HRESULT {
let this = this as *mut *mut ::core::ffi::c_void as *mut Self;
if iid.is_null() || interface.is_null() {
return ::windows_core::HRESULT(-2147467261);
}
*interface = if *iid == <DataProviderHandler as ::windows_core::ComInterface>::IID || *iid == <::windows_core::IUnknown as ::windows_core::ComInterface>::IID || *iid == <::windows_core::imp::IAgileObject as ::windows_core::ComInterface>::IID { &mut (*this).vtable as *mut _ as _ } else { ::core::ptr::null_mut() };
if (*interface).is_null() {
::windows_core::HRESULT(-2147467262)
Expand Down Expand Up @@ -3374,6 +3377,9 @@ impl<F: FnMut(::core::option::Option<&ShareProviderOperation>) -> ::windows_core
};
unsafe extern "system" fn QueryInterface(this: *mut ::core::ffi::c_void, iid: *const ::windows_core::GUID, interface: *mut *mut ::core::ffi::c_void) -> ::windows_core::HRESULT {
let this = this as *mut *mut ::core::ffi::c_void as *mut Self;
if iid.is_null() || interface.is_null() {
return ::windows_core::HRESULT(-2147467261);
}
*interface = if *iid == <ShareProviderHandler as ::windows_core::ComInterface>::IID || *iid == <::windows_core::IUnknown as ::windows_core::ComInterface>::IID || *iid == <::windows_core::imp::IAgileObject as ::windows_core::ComInterface>::IID { &mut (*this).vtable as *mut _ as _ } else { ::core::ptr::null_mut() };
if (*interface).is_null() {
::windows_core::HRESULT(-2147467262)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2249,6 +2249,9 @@ impl<F: FnMut(::core::option::Option<&PaymentRequest>, ::core::option::Option<&P
};
unsafe extern "system" fn QueryInterface(this: *mut ::core::ffi::c_void, iid: *const ::windows_core::GUID, interface: *mut *mut ::core::ffi::c_void) -> ::windows_core::HRESULT {
let this = this as *mut *mut ::core::ffi::c_void as *mut Self;
if iid.is_null() || interface.is_null() {
return ::windows_core::HRESULT(-2147467261);
}
*interface = if *iid == <PaymentRequestChangedHandler as ::windows_core::ComInterface>::IID || *iid == <::windows_core::IUnknown as ::windows_core::ComInterface>::IID || *iid == <::windows_core::imp::IAgileObject as ::windows_core::ComInterface>::IID { &mut (*this).vtable as *mut _ as _ } else { ::core::ptr::null_mut() };
if (*interface).is_null() {
::windows_core::HRESULT(-2147467262)
Expand Down
3 changes: 3 additions & 0 deletions crates/libs/windows/src/Windows/ApplicationModel/Store/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1496,6 +1496,9 @@ impl<F: FnMut() -> ::windows_core::Result<()> + ::core::marker::Send + 'static>
};
unsafe extern "system" fn QueryInterface(this: *mut ::core::ffi::c_void, iid: *const ::windows_core::GUID, interface: *mut *mut ::core::ffi::c_void) -> ::windows_core::HRESULT {
let this = this as *mut *mut ::core::ffi::c_void as *mut Self;
if iid.is_null() || interface.is_null() {
return ::windows_core::HRESULT(-2147467261);
}
*interface = if *iid == <LicenseChangedEventHandler as ::windows_core::ComInterface>::IID || *iid == <::windows_core::IUnknown as ::windows_core::ComInterface>::IID || *iid == <::windows_core::imp::IAgileObject as ::windows_core::ComInterface>::IID { &mut (*this).vtable as *mut _ as _ } else { ::core::ptr::null_mut() };
if (*interface).is_null() {
::windows_core::HRESULT(-2147467262)
Expand Down
6 changes: 6 additions & 0 deletions crates/libs/windows/src/Windows/Data/Text/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1334,6 +1334,9 @@ impl<F: FnMut(::core::option::Option<&super::super::Foundation::Collections::IIt
};
unsafe extern "system" fn QueryInterface(this: *mut ::core::ffi::c_void, iid: *const ::windows_core::GUID, interface: *mut *mut ::core::ffi::c_void) -> ::windows_core::HRESULT {
let this = this as *mut *mut ::core::ffi::c_void as *mut Self;
if iid.is_null() || interface.is_null() {
return ::windows_core::HRESULT(-2147467261);
}
*interface = if *iid == <SelectableWordSegmentsTokenizingHandler as ::windows_core::ComInterface>::IID || *iid == <::windows_core::IUnknown as ::windows_core::ComInterface>::IID || *iid == <::windows_core::imp::IAgileObject as ::windows_core::ComInterface>::IID { &mut (*this).vtable as *mut _ as _ } else { ::core::ptr::null_mut() };
if (*interface).is_null() {
::windows_core::HRESULT(-2147467262)
Expand Down Expand Up @@ -1418,6 +1421,9 @@ impl<F: FnMut(::core::option::Option<&super::super::Foundation::Collections::IIt
};
unsafe extern "system" fn QueryInterface(this: *mut ::core::ffi::c_void, iid: *const ::windows_core::GUID, interface: *mut *mut ::core::ffi::c_void) -> ::windows_core::HRESULT {
let this = this as *mut *mut ::core::ffi::c_void as *mut Self;
if iid.is_null() || interface.is_null() {
return ::windows_core::HRESULT(-2147467261);
}
*interface = if *iid == <WordSegmentsTokenizingHandler as ::windows_core::ComInterface>::IID || *iid == <::windows_core::IUnknown as ::windows_core::ComInterface>::IID || *iid == <::windows_core::imp::IAgileObject as ::windows_core::ComInterface>::IID { &mut (*this).vtable as *mut _ as _ } else { ::core::ptr::null_mut() };
if (*interface).is_null() {
::windows_core::HRESULT(-2147467262)
Expand Down
3 changes: 3 additions & 0 deletions crates/libs/windows/src/Windows/Devices/SmartCards/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4530,6 +4530,9 @@ impl<F: FnMut(::core::option::Option<&SmartCardProvisioning>, ::core::option::Op
};
unsafe extern "system" fn QueryInterface(this: *mut ::core::ffi::c_void, iid: *const ::windows_core::GUID, interface: *mut *mut ::core::ffi::c_void) -> ::windows_core::HRESULT {
let this = this as *mut *mut ::core::ffi::c_void as *mut Self;
if iid.is_null() || interface.is_null() {
return ::windows_core::HRESULT(-2147467261);
}
*interface = if *iid == <SmartCardPinResetHandler as ::windows_core::ComInterface>::IID || *iid == <::windows_core::IUnknown as ::windows_core::ComInterface>::IID || *iid == <::windows_core::imp::IAgileObject as ::windows_core::ComInterface>::IID { &mut (*this).vtable as *mut _ as _ } else { ::core::ptr::null_mut() };
if (*interface).is_null() {
::windows_core::HRESULT(-2147467262)
Expand Down
6 changes: 6 additions & 0 deletions crates/libs/windows/src/Windows/Devices/Sms/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4413,6 +4413,9 @@ impl<F: FnMut(::core::option::Option<&SmsDevice>) -> ::windows_core::Result<()>
};
unsafe extern "system" fn QueryInterface(this: *mut ::core::ffi::c_void, iid: *const ::windows_core::GUID, interface: *mut *mut ::core::ffi::c_void) -> ::windows_core::HRESULT {
let this = this as *mut *mut ::core::ffi::c_void as *mut Self;
if iid.is_null() || interface.is_null() {
return ::windows_core::HRESULT(-2147467261);
}
*interface = if *iid == <SmsDeviceStatusChangedEventHandler as ::windows_core::ComInterface>::IID || *iid == <::windows_core::IUnknown as ::windows_core::ComInterface>::IID || *iid == <::windows_core::imp::IAgileObject as ::windows_core::ComInterface>::IID { &mut (*this).vtable as *mut _ as _ } else { ::core::ptr::null_mut() };
if (*interface).is_null() {
::windows_core::HRESULT(-2147467262)
Expand Down Expand Up @@ -4497,6 +4500,9 @@ impl<F: FnMut(::core::option::Option<&SmsDevice>, ::core::option::Option<&SmsMes
};
unsafe extern "system" fn QueryInterface(this: *mut ::core::ffi::c_void, iid: *const ::windows_core::GUID, interface: *mut *mut ::core::ffi::c_void) -> ::windows_core::HRESULT {
let this = this as *mut *mut ::core::ffi::c_void as *mut Self;
if iid.is_null() || interface.is_null() {
return ::windows_core::HRESULT(-2147467261);
}
*interface = if *iid == <SmsMessageReceivedEventHandler as ::windows_core::ComInterface>::IID || *iid == <::windows_core::IUnknown as ::windows_core::ComInterface>::IID || *iid == <::windows_core::imp::IAgileObject as ::windows_core::ComInterface>::IID { &mut (*this).vtable as *mut _ as _ } else { ::core::ptr::null_mut() };
if (*interface).is_null() {
::windows_core::HRESULT(-2147467262)
Expand Down
6 changes: 6 additions & 0 deletions crates/libs/windows/src/Windows/Foundation/Collections/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1488,6 +1488,9 @@ impl<K: ::windows_core::RuntimeType + 'static, V: ::windows_core::RuntimeType +
};
unsafe extern "system" fn QueryInterface(this: *mut ::core::ffi::c_void, iid: *const ::windows_core::GUID, interface: *mut *mut ::core::ffi::c_void) -> ::windows_core::HRESULT {
let this = this as *mut *mut ::core::ffi::c_void as *mut Self;
if iid.is_null() || interface.is_null() {
return ::windows_core::HRESULT(-2147467261);
}
*interface = if *iid == <MapChangedEventHandler<K, V> as ::windows_core::ComInterface>::IID || *iid == <::windows_core::IUnknown as ::windows_core::ComInterface>::IID || *iid == <::windows_core::imp::IAgileObject as ::windows_core::ComInterface>::IID { &mut (*this).vtable as *mut _ as _ } else { ::core::ptr::null_mut() };
if (*interface).is_null() {
::windows_core::HRESULT(-2147467262)
Expand Down Expand Up @@ -1571,6 +1574,9 @@ impl<T: ::windows_core::RuntimeType + 'static, F: FnMut(::core::option::Option<&
};
unsafe extern "system" fn QueryInterface(this: *mut ::core::ffi::c_void, iid: *const ::windows_core::GUID, interface: *mut *mut ::core::ffi::c_void) -> ::windows_core::HRESULT {
let this = this as *mut *mut ::core::ffi::c_void as *mut Self;
if iid.is_null() || interface.is_null() {
return ::windows_core::HRESULT(-2147467261);
}
*interface = if *iid == <VectorChangedEventHandler<T> as ::windows_core::ComInterface>::IID || *iid == <::windows_core::IUnknown as ::windows_core::ComInterface>::IID || *iid == <::windows_core::imp::IAgileObject as ::windows_core::ComInterface>::IID { &mut (*this).vtable as *mut _ as _ } else { ::core::ptr::null_mut() };
if (*interface).is_null() {
::windows_core::HRESULT(-2147467262)
Expand Down
Loading