Received-SPF: pass (google.com: domain of linux-crypto-owner@vger.kernel.org designates 2620:137:e000::3:5 as permitted sender) client-ip=2620:137:e000::3:5;
Date:   Tue, 10 Oct 2023 10:03:24 +0200
From:   Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To:     Alexander Graf <graf@amazon.com>
Cc:     linux-kernel@vger.kernel.org, linux-crypto@vger.kernel.org,
        Arnd Bergmann <arnd@arndb.de>,
        Herbert Xu <herbert@gondor.apana.org.au>,
        Olivia Mackall <olivia@selenic.com>,
        Petre Eftime <petre.eftime@gmail.com>,
        Erdem Meydanlli <meydanli@amazon.nl>,
        Benjamin Herrenschmidt <benh@kernel.crashing.org>,
        David Woodhouse <dwmw@amazon.co.uk>,
        "Michael S . Tsirkin" <mst@redhat.com>,
        Jason Wang <jasowang@redhat.com>,
        Xuan Zhuo <xuanzhuo@linux.alibaba.com>,
        Kyunghwan Kwon <k@mononn.com>
Subject: Re: [PATCH v4 1/2] Import CBOR library
Message-ID: <2023101009-accustom-manifesto-8bdb@gregkh>
References: <20231009212053.2007-1-graf@amazon.com>
 <20231009212053.2007-2-graf@amazon.com>
 <2023101010-overwrite-parakeet-91d5@gregkh>
 <0ee221bc-ea99-4724-9ebd-436e91417e4b@amazon.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <0ee221bc-ea99-4724-9ebd-436e91417e4b@amazon.com>
Precedence: bulk

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