linux-kernel - Re: [RFC][PATCH 03/12] digest

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 28 Jun 2021 11:32:18 +0200
From:   Greg KH <gregkh@...uxfoundation.org>
To:     Roberto Sassu <roberto.sassu@...wei.com>
Cc:     "zohar@...ux.ibm.com" <zohar@...ux.ibm.com>,
        "linux-integrity@...r.kernel.org" <linux-integrity@...r.kernel.org>,
        "linux-security-module@...r.kernel.org" 
        <linux-security-module@...r.kernel.org>,
        "linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
        "linux-kselftest@...r.kernel.org" <linux-kselftest@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [RFC][PATCH 03/12] digest_lists: Basic definitions

On Mon, Jun 28, 2021 at 09:27:05AM +0000, Roberto Sassu wrote:
> > From: Greg KH [mailto:gregkh@...uxfoundation.org]
> > Sent: Monday, June 28, 2021 10:46 AM
> > On Mon, Jun 28, 2021 at 08:30:32AM +0000, Roberto Sassu wrote:
> > > > > +struct compact_list_hdr {
> > > > > +	__u8 version;
> > > >
> > > > You should never need a version, that way lies madness.
> > >
> > > We wanted to have a way to switch to a new format, if necessary.
> > 
> > Then just add a new ioctl if you need that in the future, no need to try
> > to cram it into this one.
> 
> Given that digest lists are generated elsewhere, it would be still
> unclear when the ioctl() would be issued. Maybe the kernel needs
> to parse both v1 and v2 digest lists (I expect that v1 cannot be easily
> converted to v2, if they are signed).
> 
>  It would be also unpractical if digest lists are loaded at kernel
> initialization time (I didn't send the patch yet).

Then that is up to your api design, I do not know.  But note that
"version" fields almost always never work, so be careful about assuming
that this will solve any future issues.

> > > > > +	__le16 type;
> > > > > +	__le16 modifiers;
> > > > > +	__le16 algo;
> > > > > +	__le32 count;
> > > > > +	__le32 datalen;
> > > >
> > > > Why are user/kernel apis specified in little endian format?  Why would
> > > > that matter?  Shouldn't they just be "native" endian?
> > >
> > > I thought this would make it clear that the kernel always expects the
> > > digest lists to be in little endian.
> > 
> > Why would a big endian system expect the data from userspace to be in
> > little endian?  Shouldn't this always just be "native" endian given that
> > this is not something that is being sent to hardware?
> 
> The digest list might come from a system with different endianness.

Ok, I have no idea what digests really are used for then.  So stick with
little endian and be sure to properly convert within the kernel as
needed.

thanks,

greg k-h