Comments (6)
Since COM methods will almost always result in accesses to memory outside the bounds of the base class struct, calling these methods will usually result in undefined behavior.
I recently came to this same conclusion and have been worrying about it. Theoretically this could be worked around with exposed provenance:
Of course, these are nightly only APIs, which is a little problematic. The tracking issue FAQ links sptr
for a stable polyfill, although I've no idea if that works as far back as winapi's MSRV of rustc 1.6. If it's sufficient to have a magically named method, perhaps winapi can roll its own?
from winapi-rs.
Using exposed provenance is trickier than I'd hoped.
#![feature(strict_provenance)]
#![forbid(unsafe_op_in_unsafe_fn)]
#![allow(dead_code)]
#![allow(non_snake_case)]
use std::sync::{Arc, atomic::*};
use core::ffi::c_void;
fn main() {
let com = CComObject::new();
unsafe { Arc::increment_strong_count(com) };
unsafe { (*com).AddRef() };
unsafe { (*com).Release() };
}
#[repr(C)] pub struct CComObject {
vtbl: *const IUnknownVtbl,
// above is accessible with &IUnknown's strict spatial provenance
// bellow isn't accessible with &IUnknown's strict spatial provenance
member: AtomicUsize,
}
impl core::ops::Deref for CComObject {
type Target = IUnknown;
fn deref(&self) -> &IUnknown {
// This dance does nothing of value, since converting to &IUnknown
// immediately re-introduces a new restricted spatial provenance?
unsafe { &*core::ptr::from_exposed_addr(<*const Self>::expose_addr(self)) }
}
}
impl CComObject {
pub fn new() -> *const CComObject {
Arc::into_raw(Arc::new(Self { vtbl: &Self::VTBL, member: AtomicUsize::new(0) }))
}
const VTBL : IUnknownVtbl = IUnknownVtbl {
AddRef: Self::add_ref,
Release: Self::release,
QueryInterface: Self::query_interface,
};
extern "system" fn add_ref(unknown: *mut IUnknown) -> ULONG {
// Rely on (&IUnknown -> *IUnknown -> usize) having the same value as:
// <*const CComObject>::expose_addr(com_object) ?
let this : *const CComObject = core::ptr::from_exposed_addr_mut(unknown.addr());
//let this : *const CComObject = unknown.cast(); // breaks fetch_add if uncommented
// MIRI at least no longer complains about this:
unsafe { (*this).member.fetch_add(1, Ordering::Relaxed) };
// MIRI still complains about this though:
// error: Undefined Behavior: trying to retag from <wildcard> for SharedReadWrite permission at alloc1566[0x0], but no exposed tags have suitable permission in the borrow stack for this location
unsafe { Arc::increment_strong_count(this) };
dbg!("add_ref finished");
0
}
// Stubby placeholders
extern "system" fn release(_this: *mut IUnknown) -> ULONG { 0 }
extern "system" fn query_interface(_this: *mut IUnknown, _riid: REFIID, _ppv_object: *mut *mut c_void) -> HRESULT { 0 }
}
// winapi fills
pub type HRESULT = i32;
pub type ULONG = u32;
pub struct GUID { pub Data1: u32, pub Data2: u16, pub Data3: u16, pub Data4: [u8; 16] }
pub type REFIID = *const GUID;
#[repr(C)] pub struct IUnknown {
lpVtbl: *const IUnknownVtbl,
}
#[repr(C)] pub struct IUnknownVtbl {
pub QueryInterface: unsafe extern "system" fn(This: *mut IUnknown, riid: REFIID, ppvObject: *mut *mut c_void) -> HRESULT,
pub AddRef: unsafe extern "system" fn(This: *mut IUnknown) -> ULONG,
pub Release: unsafe extern "system" fn(This: *mut IUnknown) -> ULONG,
}
impl IUnknown {
pub unsafe fn QueryInterface(&self, riid: REFIID, ppvObject: *mut *mut c_void) -> HRESULT { unsafe { ((*self.lpVtbl).QueryInterface)(self as *const _ as *mut _, riid, ppvObject) } }
pub unsafe fn AddRef(&self) -> ULONG { unsafe { ((*self.lpVtbl).AddRef)(self as *const _ as *mut _) } }
pub unsafe fn Release(&self) -> ULONG { unsafe { ((*self.lpVtbl).Release)(self as *const _ as *mut _) } }
}
from winapi-rs.
Unresolved Questions
What's Problematic (And Should Work)?
- downcasting to subclasses?
- Would be nice if you could create a reference without shrinking its provenance to allow for ergonomic references to a baseclass that can be (unsafely) cast to a reference to a subclass.
Seems more "necessary" than "would be nice"
from winapi-rs.
I believe Miri is complaining in your example because you're only exposing the provenance of the CComObject
itself when you call expose_addr
from the CComObject::deref
impl, but Arc
operations require provenance for the entire Arc
allocation containing both the Arc
's reference counts and the CComObject
itself. I was able to get Miri to stop complaining by simply adding a call to expose_addr
with the original pointer returned by Arc::into_raw
in CComObject::new
(playground). This works because that pointer has provenance for the entire Arc
allocation.
So, I think you are right that it can be sound (under permissive provenance but not strict provenance!) for COM methods to take &self
, as long as expose_addr
gets called for the entire allocation at creation time, and from_exposed_addr
is used when casting a base class pointer to a derived class pointer in method bodies; and both of these things should only be necessary when object creation code and method bodies are defined in Rust (so the current state of things should be totally fine for calling methods on COM objects that originated over an FFI boundary).
Seems more "necessary" than "would be nice"
Like I said in the issue description, it's perfectly possible to write COM code in a way which is unambiguously sound with respect to strict provenance by simply passing around raw pointers instead of converting them to references and back, with the only downside being that method calls have to look like IUnknown::Release(this)
rather than this.Release()
(and it would even be possible to define a wrapper type struct IUnknownPtr(*mut IUnknown)
which would support postfix methods). So it does seem more "nice" than strictly necessary to me.
from winapi-rs.
I believe Miri is complaining in your example because you're only exposing the provenance of the
CComObject
itself when you callexpose_addr
from theCComObject::deref
impl
Ahh, good catch! self
being &CComObject
there instead of *const CComObject
being what narrowed provenance.
Like I said in the issue description, it's perfectly possible to write COM code in a way which is unambiguously sound with respect to strict provenance by simply passing around raw pointers instead of converting them to references and back, with the only downside being that method calls have to look like
IUnknown::Release(this)
rather thanthis.Release()
The other downside is that this would necessitate a winapi 0.4 / abandon attempting to maintain soundness for existing winapi 0.3 based code.
(and it would even be possible to define a wrapper type
struct IUnknownPtr(*mut IUnknown)
which would support postfix methods). So it does seem more "nice" than strictly necessary to me.
This is more or less what windows-rs does FWIW (although they impl Drop and make it a COM style pointer AFAIK)
There's also #![feature(arbitrary_self_types)]
, which allows self: *mut IUnknown
or self: NonNull<IUnknown>
, which would also support postfix methods without a crate-provided wrapper type. Beyond winapi's 1.6 MSRV reach of course...
from winapi-rs.
I suppose defining the COM interfaces as extern types rather than structs would solve the problem of out of bounds access.
from winapi-rs.
Related Issues (20)
- Cannot initialize NOTIFYICONDATAA_u
- Cannot import winapi::um::lmjoin HOT 1
- Error when calling winapi::um::fileapi::CreateFileW()
- [Help] crate winapi 0.28 cannot be found HOT 1
- Error: could not find `wow64apiset` in `um`
- Missing most ole2.h definitions
- add richedit?
- Shell extensions? HOT 17
- Missing SHGetDesktopFolder
- winapi crashes on x86 because of conflicting stack alignment assumptions HOT 16
- ID3D11Device1::CreateRasterizerState1 is called “CreateRasterizerState” in winapi
- devquery, swdevice, shlwapi, verrsrc and winternl related modules are not defined
- Usage of GetThumbnail (?)
- Hide `winapi`'s documentation behind a feature flag?
- WritePrinter doesn't support international characters HOT 2
- CopyFileTranslatedA takes LPCWSTR as a parameter type HOT 2
- Linker error when cross compiling on Linux for Windows HOT 1
- Is this project dead replaced by windows-rs?
- Add missing `GetIconInfoEx` and related structures
- Superseded by windows-sys?
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from winapi-rs.