Re: [PATCH] livepatch-build-tools: allow patch file name sizes up to 128 characters

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Tue, Feb 27, 2024 at 01:32:05PM +0000, Ross Lagerwall wrote:
> On Wed, Jan 31, 2024 at 4:58 PM Roger Pau Monne <roger.pau@xxxxxxxxxx> wrote:
> >
> > XenServer uses quite long Xen version names, and encode such in the 
> > livepatch
> > filename, and it's currently running out of space in the file name.
> >
> > Bump max filename size to 128, so it also matches the patch name length in 
> > the
> > hypervisor interface.
> >
> > Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> > ---
> >  livepatch-build | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/livepatch-build b/livepatch-build
> > index cdb852cc7fea..78dd2d801048 100755
> > --- a/livepatch-build
> > +++ b/livepatch-build
> > @@ -72,8 +72,8 @@ function make_patch_name()
> >      fi
> >
> >      # Only allow alphanumerics and '_' and '-' in the patch name.  
> > Everything
> > -    # else is replaced with '-'.  Truncate to 48 chars.
> > -    echo ${PATCHNAME//[^a-zA-Z0-9_-]/-} |cut -c 1-48
> > +    # else is replaced with '-'.  Truncate to 128 chars.
> > +    echo ${PATCHNAME//[^a-zA-Z0-9_-]/-} |cut -c -128
> >  }
> 
> I think it should be XEN_LIVEPATCH_NAME_SIZE - 1 to match the hypervisor
> since AFAICT the hypervisor expects the last character to be NUL.
> 
> In get_name(), it checks:
> 
>     if ( !name->size || name->size > XEN_LIVEPATCH_NAME_SIZE )
>         return -EINVAL;
> 
> ... and:
> 
>     if ( n[name->size - 1] )
>         return -EINVAL;
> 
> Do you agree with that?

Oh, I didn't catch that one.  xen-livepatch should also be adjusted
then, as get_name() there does:

static int get_name(int argc, char *argv[], char *name)
{
    ssize_t len = strlen(argv[0]);
    if ( len > XEN_LIVEPATCH_NAME_SIZE )

Which needs to be adjusted to len >= instead, and the error printed
should also mention that ID must be no more than
XEN_LIVEPATCH_NAME_SIZE - 1.

Thanks, Roger.