error.rs: check range and add type invariant to Error

[ojeda]

From @nbdd0121 #287 (comment):

We will need to make sure that no Error with out of range error code can be constructed (e.g. 0, > MAX_ERROR), as otherwise it might be a soundness hole (with the use of from_kernel_result for example).

[foxhlchen]

Sorry, I didn't quite get it.

Rust doesn't have the "constructor" concept, so we can't check that on
Error(error_val)
One way to solve this is to add a "new" method to Error and force people to use
Error::new(error_val)
But I don't think this is what you're asking.

Do you mean to add checks on from_kernel_errno and to_kernel_errno, and panics if it failed the check??

[ojeda]

But I don't think this is what you're asking.

It is! The idea of type invariants is that we check something once at construction time and then we know it holds for the lifetime of the object (as long as other methods do not break it, of course). Search for INVARIANT inside rust/ for other examples.

[nbdd0121]

Rust doesn't have the "constructor" concept, so we can't check that on
Error(error_val)

The field is private, so it wouldn't be seen outside the module.

Do you mean to add checks on from_kernel_errno and to_kernel_errno, and panics if it failed the check??

from_kernel_errno would require some check (and we might want a from_kernel_errno_unchecked to bypass the check, e.g. when converting from err ptr), but probably panicking is not the best option here. We can maybe warn and convert it to a sensible error like EINVAL.

to_kernel_errno does not need checks.

[foxhlchen]

Thank you both.

Combining what you are suggesting, I will do:

1. Add a "new()" method to Error providing checks in creation.
2. Add check to from_kernel_errno()
3. Provide a non-checked version from_kernel_errno_unchecked()

During checking when an invalid err_no is found, they will 1) log a warning 2) convert it to EINVAL.

[TheSven73]

Does merging #324 close this Issue?

[ojeda]

Yeah, even if we are still discussing how to approach errors, this one can go out.