linux-kernel - Re: [PATCH V3 1/4] crypto: ccp - Fix SEV

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:   Wed, 10 Nov 2021 10:29:54 -0700
From:   Peter Gonda <pgonda@...gle.com>
To:     Tom Lendacky <Thomas.Lendacky@....com>
Cc:     Sean Christopherson <seanjc@...gle.com>,
        Marc Orr <marcorr@...gle.com>,
        David Rientjes <rientjes@...gle.com>,
        Brijesh Singh <brijesh.singh@....com>,
        Joerg Roedel <jroedel@...e.de>,
        Herbert Xu <herbert@...dor.apana.org.au>,
        John Allen <john.allen@....com>,
        "David S. Miller" <davem@...emloft.net>,
        Paolo Bonzini <pbonzini@...hat.com>,
        linux-crypto@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH V3 1/4] crypto: ccp - Fix SEV_INIT error logging on init

On Tue, Nov 9, 2021 at 12:25 PM Tom Lendacky <thomas.lendacky@....com> wrote:
>
> On 11/9/21 10:46 AM, Peter Gonda wrote:
> > On Tue, Nov 9, 2021 at 9:27 AM Sean Christopherson <seanjc@...gle.com> wrote:
> >> On Tue, Nov 02, 2021, Peter Gonda wrote:
>
> ...
>
> >>
> >>          SEV: failed to INIT error 0, rc -16
> >>
> >> which a bit head scratching without looking at the code.  AFAICT, the PSP return
> >> codes aren't intrinsically hex, so printing error as a signed demical and thus
> >>
> >>          SEV: failed to INIT error -1, rc -16
> >>
> >> would be less confusing.
> >>
> >> And IMO requiring the caller to initialize error is will be neverending game of
> >> whack-a-mole.  E.g. sev_ioctl() fails to set "error" in the userspace structure,
> >> and literally every function exposed via include/linux/psp-sev.h has this same
> >> issue.  Case in point, the retry path fails to re-initialize "error" and will
> >> display stale information if the second sev_platform_init() fails without reaching
> >> the PSP.
> >
> > OK I can update __sev_platform_init_locked() to set error to -1. That
> > seems pretty reasonable. Tom, is that OK with you?
>
> Yup, I'm ok with using -1.
>
> >
>
> ...
>
> >
> > These comments seem fine to me. But I'll refrain from updating
> > anything here since this seems out-of-scope of this series. Happy to
> > discuss further and work on that if Tom is interested in those
> > refactors too.
>
> That's one of those things we've wanted to get around to improving but
> just haven't had the time. So, yes, if you wish to refactor the 'error'
> related area, that would be great.

OK so when I actually sat down to work on this. I realized this is
bigger than I thought. If we want to have error == -1 for all return
from psp-sev.h functions where the PSP isn't called yet there are a
lot of changes. For example if CONFIG_CRYPTO_DEV_SP_PSP is not defined
all these stubs need to be to set `*errror == -`, basically all these
functions need to be updated.

So to keep this series more targeted. I think I'll drop the error
here. And just have this patch print the rc value. If what I said
above seems reasonable I'll do those error refactors. Are people
envisioning something else for the error fixups?

>
> Thanks,
> Tom
>
> >