Dear crypto maintainers,
Fujita Tomonori has created some Rust bindings for the crypto API seen in
this thread. Here is a fragment of my review of said code:
On 25.06.23 12:08, Benno Lossin wrote:
>>>> + /// Adds data to message digest for processing.
>>>> + pub fn update(&mut self, data: &[u8]) -> Result {
>>>> + // SAFETY: The type invariant guarantees that the pointer is valid.
>>>> + to_result(unsafe {
>>>> + bindings::crypto_shash_update(self.ptr, data.as_ptr(), data.len() as u32)
>>>> + })
>>>
>>> What if `data.len() > u32::MAX`?
>>
>> The buffer might not be updated properly, I guess. Should check the case?
>
> Not sure what we should do in that case, will bring it up at the next
> team meeting. In Rust, `write` and `read` functions often output the
> number of bytes that were actually read/written. So maybe we should also
> do that here? Then you could just return `u32::MAX` and the user would
> have to call again. We could also call the C side multiple times until
> the entire buffer has been processed. But as the C side only supports
> u32 anyway, I think it would be a rare occurrence for `data` to be large.
I noted that in the code segment above that the length of the data
that is to be hashed is cast from a `usize` to a `u32`. Since
`usize = uintptr_t` this might be a problem for very large arguments.
Since the C side only accepts an `unsigned int`, it seems as if large inputs
are never the case. On the Rust side we are forced to use `usize`, since that
is the length of slices (the input type `&[u8]`).
We came up with the following solutions, but could not come to a consensus on any
particular one, could you please assist us in making this decision?
1. create a loop that calls the C API multiple times if the input is large
2. panic
3. truncate
4. return an error
Thanks a lot!
--
Cheers,
Benno
On Fri, Jun 30, 2023 at 02:48:37PM +0000, Benno Lossin wrote:
> We came up with the following solutions, but could not come to a consensus on any
> particular one, could you please assist us in making this decision?
>
> 2. panic
Never an option in kernel code, sorry, please don't even suggest it.
thanks,
greg k-h
On Fri, Jun 30, 2023 at 02:48:37PM +0000, Benno Lossin wrote:
>
> 4. return an error
This would seem to make the most sense.
If there is ever a need to hash more than 4G of data, we would
be adding this to C first.
At this point I can't see why we would need to do that so an
error would be the appropriate response.
Thanks,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
------- Original Message -------
On Tuesday, July 4th, 2023 at 01:19, Herbert Xu <[email protected]> wrote:
> On Fri, Jun 30, 2023 at 02:48:37PM +0000, Benno Lossin wrote:
>
> > 4. return an error
>
>
> This would seem to make the most sense.
>
> If there is ever a need to hash more than 4G of data, we would
> be adding this to C first.
>
> At this point I can't see why we would need to do that so an
> error would be the appropriate response.
>
> Thanks,
> --
> Email: Herbert Xu [email protected]
>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Thanks a lot for taking a look!
--
Cheers,
Benno