FFI: Segmentation fault with CString

Hi

I have a C function like this:

parse_arguments(argc: i32, args: *const *const c_char);

If it is called like the following, it works fine:

let args: Vec<CString> = std::env::args().filter_map(|arg| {
    CString::new(arg).ok()
}).collect();

let c_args: Vec<*const i8> = args.iter().map(|arg| {
    arg.as_ptr()
}).collect();

unsafe { parse_arguments(c_args.len() as i32, c_args.as_ptr()) }

I tried somethings a little different and it segfaults (in the C function):

let iter = vec!["hello", "world"].into_iter();

let args: Vec<CString> = iter.filter_map(|arg| {
    CString::new(arg).ok()
}).collect();

let c_args: Vec<*const i8> = args.iter().map(|arg| {
    arg.as_ptr()
}).collect();

unsafe { parse_arguments(c_args.len() as i32, c_args.as_ptr()) }

What's your C function?

1 Like

This one: https://github.com/esnet/iperf/blob/master/src/iperf_api.c#L765
I don't think it matters because it works with the first example.

Hmm, I can't really tell what's going on here, is there anything else that could be related? Could it possibly be that there is another c function call that could be causing this?
Try adding a println!() right after the c function call.

Also happy cake day @RustyYato!

1 Like

This playground example shows that the Rust code, although dangerously written, works fine (all possible valid dereferences take place). The problem must come from the C code.

For a safer version, useful in more complex code, I suggest forcing the borrow on args with continuation / closure style:

impl<T> WithRef for T {}
trait WithRef {
    fn with_ref<'a, R, F> (self: &'a Self, f: F) -> R
    where
        F : FnOnce(&'a Self) -> R,
    {
        f(self)
    }
}

fn test (iterator: impl Iterator<Item = impl Into<Vec<u8>>>)
{
    println!("\nTesting:");
    let args = Vec::from_iter(
        iterator
            .filter_map(|arg| CString::new(arg).ok())
    );
    // args is guaranteed to be borrowed for the whole closure body
    args.with_ref(|args| {
        let c_args = Vec::from_iter(
            args.iter()
                .map(|c_string: &'_ CString| -> *const c_char {
                    c_string.as_ptr()
                })
        );
        unsafe {
            c_function(
                c_args.len().try_into().expect("argc overflowed"),
                c_args.as_ptr(),
            );
        }
    })
}
2 Likes

Well, it can't be exactly that iperf_parse_arguments function since it takes an addition argument. So it is likely you have a shim around it, where the problem may in fact lie.

Could it be that argv is expected to have at least 3 elements? Remember that env::args as well as the C argv includes the executable name as the first element.

1 Like

Thank you for your answers. I have made a mistake, because I use the release 3.6 so the function is this one instead: https://github.com/esnet/iperf/blob/88f3ad87f38762081a6d7127890ff5c3a052b06a/src/iperf_api.c#L671. It is slightly different but no major changes.

Here is the extern function declaration:

pub fn iperf_parse_arguments(
    test: *mut c_void,
    argc: c_int,
    argv: *const *const c_char,
) -> c_int;

I tried to call the same function from C, to be sure that there is no bug in the library, and it works fine in every situation.

I tried your suggestion and it did not work either:

let iter = vec!["iperf3", "--server"].into_iter();

let args = Vec::from_iter(iter
    .filter_map(|arg| CString::new(arg).ok())
);

args.with_ref(|args| {
    let c_args = Vec::from_iter(args.iter()
        .map(|c_string: &'_ CString| -> *const i8 {
            c_string.as_ptr()
        })
    );
    let argc: i32 = c_args.len().try_into().unwrap();
    if unsafe { c::iperf_parse_arguments(test, argc, c_args.as_ptr()) } != 0 {
        Err("failed to parse arguments")
    } else {
        Ok(())
    }
})

At this point, I can just suggest breaking into the debugger in the C function, or writing your own C function with compatible signature, and carefully inspect the arguments there.

Ok, I have tested with the following code, against the commit you have linked, and I get no segfaults:

#![allow(non_camel_case_types)]

use ::std::{*,
	convert::TryInto,
	iter::FromIterator,
	ffi::CString,
};
use ::libc::{c_char, c_int};

#[repr(C)]
struct iperf_test {
	__inner: [u8; 0],
}

extern "C" {
	fn iperf_parse_arguments (
		test: *mut iperf_test,
		argc: c_int,
		argv: *const *const c_char,
	);
	fn iperf_new_test () -> *mut iperf_test;
	fn iperf_defaults (test: *mut iperf_test) -> c_int;
	fn iperf_free_test (test: *mut iperf_test);
}

impl<T> WithRef for T {}
trait WithRef {
	fn with_ref<'a, R, F> (self: &'a Self, f: F) -> R
	where
		F : FnOnce(&'a Self) -> R,
	{
		f(self)
	}
}

fn main ()
{
	test(env::args());
	test(["foo", "bar"].iter().cloned());
}

fn test (iterator: impl Iterator<Item = impl Into<Vec<u8>>>)
{
	let args = Vec::from_iter(
		iterator
			.map(|arg| CString::new(arg).unwrap())
	);
	args.with_ref(|args| {
		let argv = Vec::from_iter(
			args.iter()
				.map(|arg: &'_ CString| -> *const c_char {
					arg.as_ptr()
				})
		);
		unsafe {
			let test = iperf_new_test();
			if iperf_defaults(test) < 0 {
				panic!("iperf_defaults() failed");
			}
			iperf_parse_arguments(
				test,
				argv.len().try_into().expect("argc overflowed"),
				argv.as_ptr(),
			);
			iperf_free_test(test);
		}
	});
}

Thank you.
The issue was that I forgot to call iperf_defaults().

While not the problem here, I would like to mention for anyone investigating things related to this, that C and C++ argv for main is expected to end with a null pointer at argv[argc]. Some consumers don't bother looking at argc and just loop until they run out of non-null elements.

2 Likes

Good point, this becomes "complex" enough (regarding the aforemention lack of lifetime tracking of raw pointers) that it seems to deserve its own helper function / method:

use ::std::{*,
    borrow::Borrow,
    convert::TryInto,
    ffi::{CStr, CString},
    ops::Deref,
    ptr::NonNull,
};
use ::libc::{c_char, c_int}; // 0.2.51

pub
trait WithArgcAndArgv<Item>
where
    Self : Borrow<[Item]>,
    Item : Deref<Target = CStr>,
{
    /// f can soundly use `unsafe` to **read** up to `argc + 1` `*const c_char` pointers out of `argv`,
    /// and also dereference-read the first `argc` pointers, since they point to valid C strings.
    /// The last pointer (_i.e._, the `argc + 1`-th; index `argc`) is guaranteed to be `NULL`.
    fn with_argc_and_argv<'a, R, F> (self: &'a Self, f: F) -> R
    where
        F : FnOnce(c_int, NonNull<*const c_char>) -> R,
        Item : 'a,
    {
        let c_strs: &'a [Item] = self.borrow();
        let argc: c_int = c_strs.len().try_into().expect("argc overflowed");
        let argv: Vec<*const c_char> =
            c_strs
                .iter()
                .map(|c_string: &'a Item| -> *const c_char {
                    c_string.as_ptr()
                })
                .chain(iter::once(ptr::null()))
                .collect()
        ;
        f(argc, unsafe { NonNull::new_unchecked(argv.as_ptr() as *mut _) })
    }
}
impl<Item, Slf : ?Sized> WithArgcAndArgv<Item> for Slf
where
    Slf : Borrow<[Item]>,
    Item : Deref<Target = CStr>,
{}

Not only is it always safe and sound, but it also allows using inline/stack arrays as argv when dealing with string literals, thanks to ::byte_strings::c_str! macro:

fn main ()
{
    use ::byte_strings::c_str;

    unsafe {
        let test: NonNull<iperf_test> =
            iperf_new_test()
                .expect("iperf_new_test() failed")
        ;
        if iperf_defaults(test) < 0 {
            panic!("iperf_defaults() failed");
        }
        [c_str!("Hello"), c_str!("World!")]
            .with_argc_and_argv(|argc, argv| {
                iperf_parse_arguments(
                    test,
                    argc,
                    argv,
                );
            })
        ;
        iperf_free_test(test);
    }
}

/// given the following tweaked extern declarations
extern "C" {
	fn iperf_parse_arguments (
		test: NonNull<iperf_test>,
		argc: c_int,
		argv: NonNull<*const c_char>,
	);
	fn iperf_new_test () -> Option<NonNull<iperf_test>>; // express error path at the type level
	fn iperf_defaults (test: NonNull<iperf_test>) -> c_int;
	fn iperf_free_test (test: NonNull<iperf_test>);
}
2 Likes

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.