Unregistering a port obtained from `Port::clone_unowned` is unsound

[SludgePhD]

Applying this patch to the sine.rs example causes memory unsafety:

diff --git a/examples/sine.rs b/examples/sine.rs
index cee0003..030beb7 100644
--- a/examples/sine.rs
+++ b/examples/sine.rs
@@ -15,6 +15,8 @@ fn main() {
         .register_port("sine_out", jack::AudioOut::default())
         .unwrap();
 
+    let p = out_port.clone_unowned();
+
     // 3. define process callback handler
     let (tx, rx) = bounded(1_000_000);
     struct State {
@@ -68,6 +70,8 @@ fn main() {
         .unwrap();
     // processing starts here
 
+    active_client.as_client().unregister_port(p).unwrap();
+
     // 5. wait or do some processing while your handler is running in real time.
     println!("Enter an integer value to change the frequency of the sine wave.");
     while let Some(f) = read_freq() {

Result:

thread '<unnamed>' panicked at core/src/panicking.rs:221:5:
unsafe precondition(s) violated: slice::from_raw_parts_mut requires the pointer to be aligned and non-null, and the total size of the slice not to exceed `isize::MAX`
stack backtrace:
   0: rust_begin_unwind
             at /rustc/90b35a6239c3d8bdabc530a6a0816f7ff89a0aaf/library/std/src/panicking.rs:665:5
   1: core::panicking::panic_nounwind_fmt::runtime
             at /rustc/90b35a6239c3d8bdabc530a6a0816f7ff89a0aaf/library/core/src/panicking.rs:112:18
   2: core::panicking::panic_nounwind_fmt
             at /rustc/90b35a6239c3d8bdabc530a6a0816f7ff89a0aaf/library/core/src/panicking.rs:122:5
   3: core::panicking::panic_nounwind
             at /rustc/90b35a6239c3d8bdabc530a6a0816f7ff89a0aaf/library/core/src/panicking.rs:221:5
   4: core::slice::raw::from_raw_parts_mut::precondition_check
             at /home/sludge/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ub_checks.rs:70:21
   5: core::slice::raw::from_raw_parts_mut
             at /home/sludge/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ub_checks.rs:77:17
   6: jack::port::audio::<impl jack::port::port_impl::Port<jack::port::audio::AudioOut>>::as_mut_slice
             at ./src/port/audio.rs:92:13

Sounds like accessing the buffer of an unregistered port results in a NULL pointer?

[SludgePhD]

Client::port_by_id also allows someone to obtain an aliasing Port<Unowned>. Maybe Client::unregister_port should be restricted to PortSpecs that are not Unowned, or it should fail at runtime if passed an Unowned port?

[wmedrano]

Yeah, restricting some items to owned ports makes sense.

For anyone willing to take this up:

• PortSpec is defined at https://docs.rs/jack/latest/jack/trait.PortSpec.html , it can be extended to determine ownership.
• The implementers built in implementers are all listed on that page. So AudioIn, AudioOut, MidiIn, MidiOut, Unowned.