How does a `#![no_std]` crate with a function that takes in a closure handles the case when the closure panics?

As titled.

With catch_unwind and AssertUnwindSafe and panicking within a closure, one can leak out the inner T of a type Wrapper<T> which owns T internally across the unwind boundary.

Playground

In build environment with std, we can 're-panic' if catch_unwind returns an Err, a prime example is shown in std::thread::scope codes.

However, in #![no_std] builds, catch_unwind is unavailable. Moreover, one needs to provide a #[panic_handler] which virtually can do about anything in a #![no_std] build.

As a crate writer meant for #![no_std] users, how can I ensure that even in such builds, the inner T does not get leaked out from Wrapper<T> even across unwind boundaries?

Another question is, referring to the playground codes, why does cargo test passes the test but cargo run panics?

I have two answers to this. But I have to hear what the forum has to say about this.

First is in #![no_std], crate writers don't account for the event that a closure panics.

Second is, if crate writers want to account for the event that the closure panics in builds with std, then conditional compilation with feature will help, i.e. two different functions with the same name but only one for each variant of the feature flag, e.g. #[cfg(feature = "std")] and #[cfg(not(feature = "std"))].

Your code never actually gave ownership of the T to the Wrapper. It panicked before that could happen.

I presume that you actually mean a scenario where the Wrapper already owned the T, and temporarily transferred it to the closure expecting to get it back.

Moreover, one needs to provide a #[panic_handler] which virtually can do about anything in a #![no_std] build.

And std-using builds also let you set a panic hook, which can also do arbitrary things before any unwind happens.

As a crate writer meant for #![no_std] users, how can I ensure that even in such builds, the inner T does not get leaked out from Wrapper<T> even across unwind boundaries?

If I understand what you're trying to do correctly, then my personal opinion on this matter is “don't do that”; if you don't want to hand off ownership, then provide only &mut access. Don't rely on anything in particular happening in the event of a panic.

In both cases, the unwind was caught, but the panic hook printed the panic info before the unwind even started; catch_unwind cannot affect what the panic hook does. But, separately, the standard test harness captures all print!ed output, including the output from the default panic hook, and only shows it to you if the test fails. Add a assert!(false); to the end of the test and you'll see the panic message you expected, as well as the panic from the assertion.

1 Like

Thank you for your warm response.

Apologies for not being clear on my codes that Wrapper<T> should first own T then the program should panic; which is the case I am worried about as I do not want to leak the inner T wrapped by Wrapper<T> in panic context.

Playground

Apparently, with the addition of assert!(false) at the end, it does show that the test failed with a panic.

Hence, is it your advice that in #![no_std] (or even with std) builds, since panics are 'propagated' up the call stack even in catch_unwind(AssertUnwindSafe(...)) context, there is no need to account for the case that the closure parameter of Wrapper::new might panic? Bearing in mind that I want Wrapper<T> to own T regardless. Or think of it as a value of Wrapper<T>, which has already owned T, has a method that exposes T within the closure parameter and I do not want T to leak in the event that the closure argument panics.

Playground

Well, in this case leakage is possible only because the type in question is Copy (on mobile, so can't prove this right now, but you can simply replace it with custom ZST to get an error). Will it be Copy in your real use-case too?

It is meant to be a crate for others, possibly #![no_std] users.

Playground for ZST

Non-copyable types trivially prevents leaking but probably it is fine for cloneable types to 'leak' by the implementation of users since that is what Clone means.

I think there is no way to specify that T: !Clone.

No. I am saying: you cannot enforce this property you want, so choose a different design which does not require it.

Thank you for your response. What kind of design would you be recommending?

As I mentioned before, you can give &mut T to the closure; then you have a guarantee that even on panic, there will still be some T value present. However, that won't guarantee that it's the same T; nothing at all enforces that it wasn't swapped with another T value that the closure had access to.

More generally; whatever property you are trying to ensure by not losing the T, consider changing the design so that losing the T doesn't violate it.

If you want more specific advice, tell us about that property, and about what the T type actually is when this wrapper is being used.

3 Likes

Thank you for your reply.

I am writing to build this crate call sosecrets-rs, which has two types within, Secret<T, MEC, EC> and ExposedSecret<'brand, T>.

pub struct Secret<
    T: Zeroize,
    MEC: Unsigned,
    EC: Add<U1> + IsLessOrEqual<MEC, Output = True> + Unsigned = U0,
>(ManuallyDrop<T>, PhantomData<(MEC, EC)>);

pub struct ExposedSecret<'brand, T>(T, PhantomData<fn(&'brand ()) -> &'brand ()>);

The traits Zeroize comes from zeroize crate and Unsigned and IsLessOrEqual come from typenum crate.

The main aim of this crate is to allow user to 'expose' secret for a certain MEC (Maximum Exposed Count) number of times. The method expose_secret of Secret<T, MEC, EC> raises a compile time error if EC (Exposed Count) > MEC.

impl<
        'max,
        T: Zeroize,
        MEC: Unsigned,
        EC: Add<U1> + Unsigned + IsLessOrEqual<MEC, Output = True>,
    > ExposeSecret<'max, &'max T, MEC, EC> for Secret<T, MEC, EC>
{
    type Exposed<'brand> = ExposedSecret<'brand, &'brand T>
    where
        'max: 'brand;

    type Next = Secret<T, MEC, Sum<EC, U1>>
    where
        EC: Add<U1> + Unsigned + IsLessOrEqual<MEC, Output = True>,
        Sum<EC, U1>: Unsigned + IsLessOrEqual<MEC, Output = True> + Add<U1>,
        T: Zeroize;

    #[inline(always)]
    fn expose_secret<ReturnType, ClosureType>(
        mut self,
        scope: ClosureType,
    ) -> (Secret<T, MEC, AddU1<EC>>, ReturnType)
    where
        AddU1<EC>: Add<U1> + Unsigned + IsLessOrEqual<MEC, Output = True>,
        for<'brand> ClosureType: FnOnce(ExposedSecret<'brand, &'brand T>) -> ReturnType,
    {
        let returned_value = scope(ExposedSecret(&self.0, PhantomData));
        // SAFETY: Since compile error prevents constructing a `Secret` with `EC` > `MEC`,
        // `zeroize()` is only called when `Secret` is maximally exposed
        // and it is not possible to call `expose_secret(...)`
        // when `Secret` is maximally exposed to access **private** `self.0` field,
        // therefore, this is safe.
        let inner = ManuallyDrop::new(unsafe { ManuallyDrop::take(&mut self.0) });
        forget(self);
        (Secret(inner, PhantomData), returned_value)
    }
}

My concern here is that I wouldn't want users of my crate (whether in #![no_std] or std builds) to be able to leak the inner T via the panic-catch_unwind mechanism.

Please advise how I should go about designing this so that it prevents such abuses.

Thank you.

Since you give ExposedSecret<'brand, &'brand T> to the closure, the ExposedSecret cannot be leaked.

To ensure that the contents of T itself cannot be leaked, T must not implement Clone or provide any similar function. If it does, there is no way to prevent leakage since the data can be copied into unrelated storage — whether or not a panic is involved.

You may find that for your actual requirements, the best you can do is to document “the closure should be reviewed to confirm it does not make a copy of the secret”.

1 Like

Thank you for your warm reply @kpreid!