Re: [PATCH v4 1/2] Import CBOR library

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Tue, Oct 10, 2023 at 09:55:25AM +0200, Alexander Graf wrote:
> Hey Greg,
> 
> On 10.10.23 08:13, Greg Kroah-Hartman wrote:
> > On Mon, Oct 09, 2023 at 09:20:52PM +0000, Alexander Graf wrote:
> > > To fully support the Nitro Secure Module communication protocol, we need
> > > to encode and decode CBOR binary data. Import an MIT licensed library
> > > from https://github.com/libmcu/cbor (commit f3d1696f886) so that we can
> > > easily consume CBOR data.
> > What is "CBOR"?  I don't see a description of it here.
> 
> 
> CBOR is the "Concise Binary Object Representation"
> (https://en.wikipedia.org/wiki/CBOR) binary format.
> 
> 
> > 
> > And I guess you are going to keep this in sync with upstream?  Or do you
> > really need the full library here (you #ifdef the float stuff out), does
> > your module really need all of the functionality and complexity of this
> > library, or can it use just a much smaller one instead?
> 
> 
> CBOR knows a total of 9 data types:
> 
>   - Unsigned integers
>   - Signed integers
>   - Binary string
>   - UTF-8 string
>   - Arrays
>   - Maps (like a python dictionary)
>   - Semantic tag
>   - Bools
>   - Floats
> 
> Out of these, the NSM communication protocol uses all except Semantic tags
> and Floats. The CBOR library that this patch imports does not have special
> handling for Semantic tags, which leaves only floats which are already
> #ifdef'ed out. That means there is not much to trim.
> 
> What you see here is what's needed to parse CBOR in kernel - if that's what
> we want to do. I'm happy to rip it out again and make it a pure user space
> problem to do CBOR :).

Yes, why are we parsing this in the kernel?  What could go wrong with
adding yet-another-parser in privileged context?  :)

Why does this have to be in the kernel, the data sent/recieved is over
virtio, so why does the kernel have to parse it?  I couldn't figure that
out from the driver, yet the driver seems to have a lot of hard-coded
parsing logic in it to assume specific message formats?

thanks,

greg k-h