lkml.org 
[lkml]   [2023]   [Jan]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH v4 16/24] powerpc/pseries: Implement signed update for PLPKS objects
    From
    Date
    On Tue, 2023-01-24 at 14:16 +1000, Nicholas Piggin wrote:
    > > diff --git a/arch/powerpc/platforms/pseries/plpks.c
    > > b/arch/powerpc/platforms/pseries/plpks.c
    > > index 1189246b03dc..796ed5544ee5 100644
    > > --- a/arch/powerpc/platforms/pseries/plpks.c
    > > +++ b/arch/powerpc/platforms/pseries/plpks.c
    > > @@ -81,6 +81,12 @@ static int pseries_status_to_err(int rc)
    > >                 err = -ENOENT;
    > >                 break;
    > >         case H_BUSY:
    > > +       case H_LONG_BUSY_ORDER_1_MSEC:
    > > +       case H_LONG_BUSY_ORDER_10_MSEC:
    > > +       case H_LONG_BUSY_ORDER_100_MSEC:
    > > +       case H_LONG_BUSY_ORDER_1_SEC:
    > > +       case H_LONG_BUSY_ORDER_10_SEC:
    > > +       case H_LONG_BUSY_ORDER_100_SEC:
    > >                 err = -EBUSY;
    > >                 break;
    > >         case H_AUTHORITY:
    >
    > This is a bit sad to maintain here. It's duplicating bits with
    > hvcs_convert, and a bunch of open coded places. Probably not the
    > series to do anything about. Would be nice if we could standardise
    > it though.

    Agreed - though we're not going to touch it in this series.

    >
    > > @@ -184,14 +190,17 @@ static struct label *construct_label(char
    > > *component, u8 varos, u8 *name,
    > >                                      u16 namelen)
    > >  {
    > >         struct label *label;
    > > -       size_t slen;
    > > +       size_t slen = 0;
    > >  
    > >         if (!name || namelen > PLPKS_MAX_NAME_SIZE)
    > >                 return ERR_PTR(-EINVAL);
    > >  
    > > -       slen = strlen(component);
    > > -       if (component && slen > sizeof(label->attr.prefix))
    > > -               return ERR_PTR(-EINVAL);
    > > +       // Support NULL component for signed updates
    > > +       if (component) {
    > > +               slen = strlen(component);
    > > +               if (slen > sizeof(label->attr.prefix))
    > > +                       return ERR_PTR(-EINVAL);
    > > +       }
    >
    > Is this already a bug? Code checks for component != NULL but
    > previously
    > calls strlen which would oops on NULL component AFAIKS. Granted
    > nothing
    > is actually using any of this these days.

    True, it should have been checking for NULL first, but as you say no-
    one is using it.

    >
    > It already seems like it's supposed to be allowed to rad NULL
    > component
    > with read_var though? Why the differences, why not always allow NULL
    > component? (I assume there is some reason, I just don't know anything
    > about secvar or secure boot).

    I think the comment confuses more than it clarifies, I'll remove it.

    As you say, read_var() should work fine with component == NULL, though
    write_var() checks it. The only rule I can find in the spec is that
    signed update calls *must* set the component to NULL. I'm seeking
    clarification on that.

    > > +EXPORT_SYMBOL(plpks_signed_update_var);
    >
    > Sorry I missed it before -- can this be a _GPL export?

    Indeed it should be - actually, I should check if I can get rid of the
    export completely...

    --
    Andrew Donnellan OzLabs, ADL Canberra
    ajd@linux.ibm.com IBM Australia Limited
    \
     
     \ /
      Last update: 2023-03-27 00:01    [W:4.143 / U:0.032 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site