Coder Social home page Coder Social logo

dync's People

Contributors

elrnv avatar

Stargazers

avatar avatar avatar avatar avatar

Watchers

avatar avatar avatar

dync's Issues

VecCopy allows for misaligned access

Hey there, I noticed that in VecCopy the backing storage for the Vec is a u8.

dync/src/vec_copy.rs

Lines 64 to 65 in c133056

/// Raw data stored as bytes.
pub(crate) data: Vec<MaybeUninit<u8>>,

I believe this let's you trigger undefined behavior in the form of misaligned memory access through safe rust code by instantiating a VecCopy with a type that has different alignment requirements from u8. Running the following program under miri:

#![forbid(unsafe_code)]

use dync::{VecCopy, VTable};

#[repr(align(256))]
#[derive(Copy, Clone)]
struct LargeAlign(u8);

impl VTable<LargeAlign> for LargeAlign {
    fn build_vtable() -> Self {
        LargeAlign(0)
    }
}

fn main() {
    // The backing storage for a VecCopy is a u8, meaning that casting to a type
    // with different alginment requires triggers undefined behavior.
    // https://github.com/elrnv/dync/blob/c133056676582dd0e28c14526175d0c9ae01a905/src/vec_copy.rs#L64-L65
    let mut x = VecCopy::<LargeAlign>::with_type();
    x.push_as::<LargeAlign>(LargeAlign(0));

    let _ref_to_element = x.get_ref_as::<LargeAlign>(0).unwrap();
}

results in:

❯ cargo miri run
error: Undefined Behavior: accessing memory with alignment 8, but alignment 256 is required
   --> /home/ammar/.cargo/registry/src/github.com-1ecc6299db9ec823/dync-0.4.0/src/vec_copy.rs:523:23
    |
523 |         Some(unsafe { &*ptr.add(i) })
    |                       ^^^^^^^^^^^^ accessing memory with alignment 8, but alignment 256 is required
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
            
    = note: inside `dync::VecCopy::<LargeAlign>::get_ref_as::<LargeAlign>` at /home/ammar/.cargo/registry/src/github.com-1ecc6299db9ec823/dync-0.4.0/src/vec_copy.rs:523:23
note: inside `main` at src/main.rs:41:27
   --> src/main.rs:41:27
    |
41  |     let _ref_to_element = x.get_ref_as::<LargeAlign>(0).unwrap();
    |                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    = note: inside `<fn() as std::ops::FnOnce<()>>::call_once - shim(fn())` at /home/ammar/.rustup/toolchains/nightly-2020-09-24-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/function.rs:227:5
    = note: inside `std::sys_common::backtrace::__rust_begin_short_backtrace::<fn(), ()>` at /home/ammar/.rustup/toolchains/nightly-2020-09-24-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/sys_common/backtrace.rs:137:18
    = note: inside closure at /home/ammar/.rustup/toolchains/nightly-2020-09-24-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/rt.rs:66:18
    = note: inside `std::ops::function::impls::<impl std::ops::FnOnce<()> for &dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe>::call_once` at /home/ammar/.rustup/toolchains/nightly-2020-09-24-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/function.rs:259:13
    = note: inside `std::panicking::r#try::do_call::<&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe, i32>` at /home/ammar/.rustup/toolchains/nightly-2020-09-24-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panicking.rs:381:40
    = note: inside `std::panicking::r#try::<i32, &dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe>` at /home/ammar/.rustup/toolchains/nightly-2020-09-24-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panicking.rs:345:19
    = note: inside `std::panic::catch_unwind::<&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe, i32>` at /home/ammar/.rustup/toolchains/nightly-2020-09-24-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panic.rs:382:14
    = note: inside `std::rt::lang_start_internal` at /home/ammar/.rustup/toolchains/nightly-2020-09-24-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/rt.rs:51:25
    = note: inside `std::rt::lang_start::<()>` at /home/ammar/.rustup/toolchains/nightly-2020-09-24-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/rt.rs:65:5

error: aborting due to previous error

Generate more built in traits and their implementations

Currently only a small handful of traits are supported. This is not too useful in general.

A number of traits would be necessary to enable the use of Values in other std containers like BTreeSet/BTreeMap and other custom containers.

This will probably require a bit of refactoring to write code generators and the like since a number of types need implementations (Value, ValueRef, VecCopy, VecDrop, etc.) for each new trait.

Simplify the API

The issue is that currently too many variations of types need to be maintained. More precisely there are Copy vs Drop variants, Owned vs Refereced (Vec vs Slice), Immutable vs Mutable types (ValueRef vs ValueMut). It is a maintenance burden on the implementation as well as API to have these around. I think this is fixable, especially because these types share a lot of functionality.

Notes:

  • Drop variants of all the types can also contain copy types, whereas the copy variants are more restricted. It doesn't seem like there is any performance penalty for using the Drop value/container types for Copy types so it's reasonable to simply deprecate the Copy variants from the public API or simply document the restricted use cases for the Copy variants.

  • It seems like there is no way around Ref and Mut variants though it is still possible to unify their implementations using macros.

  • Following the pattern of the standard library it would be preferable to have the Owned variants to deref to borrowed variants in some way, which will allow us avoid replicating the API for both owned and referenced variants of types. It is unclear how to do this reliably yet since the Deref and DerefMut traits expect & type references instead of custom ones. The example of how this works currently in the standard library is the RefCell and Ref types which have an awkward but possibly necessary API.

    Further it is worth noting that trying to implement this functionality using DST is not possible without copying data because the TypeId and vtable would need to be stored alongside the data itself, which is not how its stored in the Vec or Slice types.

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo 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.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.