Re: [libgpiod][PATCH v3 1/3] bindings: rust: fix soundness of line_info modeling

[Bartosz Golaszewski <brgl@bgdev.pl>]

On Tue, Oct 3, 2023 at 11:40 AM Erik Schilling
<erik.schilling@linaro.org> wrote:
>
> While attention was provided to prevent freeing in non-owned use-cases,
> the lifetime of these object was not properly modeled.
>
> The line_info from an event may only be used for as long as the event
> exists.
>
> This allowed us to write unsafe-free Rust code that causes a
> use-after-free:
>
>   let event = chip.read_info_event().unwrap();
>   let line_info = event.line_info().unwrap();
>   drop(event);
>   dbg!(line_info.name().unwrap());
>
> Which makes the AddressSanitizer scream:
>
>   ==90154==ERROR: AddressSanitizer: heap-use-after-free on address 0x50b000005dc4 at pc 0x55a4f883a009 bp 0x7f60ac8fbbc0 sp 0x7f60ac8fb388
>   READ of size 2 at 0x50b000005dc4 thread T2
>       [...]
>       #3 0x55a4f8c3d5f3 in libgpiod::line_info::Info::name::h5ba0bfd360ecb405 libgpiod/bindings/rust/libgpiod/src/line_info.rs:70:18
>         [...]
>
>   0x50b000005dc4 is located 4 bytes inside of 112-byte region [0x50b000005dc0,0x50b000005e30)
>   freed by thread T2 here:
>       [...]
>       #1 0x7f60b07f7e31 in gpiod_info_event_free libgpiod/lib/info-event.c:61:2
>       [...]
>
>   previously allocated by thread T2 here:
>       #0 0x55a4f88b04be in malloc /rustc/llvm/src/llvm-project/compiler-rt/lib/asan/asan_malloc_linux.cpp:69:3
>       #1 0x7f60b07f8ff0 in gpiod_line_info_from_uapi libgpiod/lib/line-info.c:144:9
>
> The fix is to distinguish between the owned and non-owned variants and
> assigning lifetimes to non-owned variants.
>
> For modeling the non-owned type there are a couple of options. The ideal
> solution would be using extern_types [1]. But that is still unstable.
> Instead, we are defining a #[repr(transparent)] wrapper around the opaque
> gpiod_line_info struct and cast the pointer to a reference.
>
> This was recommended on the Rust Discord server as good practise.
> (Thanks to Kyuuhachi, shepmaster, pie_flavor and ilyvion! Also thanks to
> @epilys for a brainstorming on this on #linaro-virtualization IRC).
>
> Of course, determining the lifetimes and casting across the types
> requires some care. So this adds a couple of SAFETY comments that would
> probably also have helped the existing code.
>
> [1] https://github.com/rust-lang/rfcs/blob/master/text/1861-extern-types.md
>
> Fixes: 91f9373 ("bindings: rust: Add libgpiod crate")
> Signed-off-by: Erik Schilling <erik.schilling@linaro.org>
> ---

Applied, thanks!

Bart