linux-kernel - Re: [PATCH 1/5] KVM: selftests: Implement memcmp(), memcpy(), and memset() for guest use

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]

Message-ID: <CALzav=dfL+DCFnKQnUP27SzWLMdWY0kKXr93=nL_VC1nj=aNkg@mail.gmail.com>
Date:   Thu, 22 Sep 2022 10:49:59 -0700
From:   David Matlack <dmatlack@...gle.com>
To:     Sean Christopherson <seanjc@...gle.com>
Cc:     Paolo Bonzini <pbonzini@...hat.com>,
        kvm list <kvm@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Andrew Jones <andrew.jones@...ux.dev>,
        Anup Patel <anup@...infault.org>,
        Atish Patra <atishp@...shpatra.org>,
        Christian Borntraeger <borntraeger@...ux.ibm.com>,
        Janosch Frank <frankja@...ux.ibm.com>,
        Claudio Imbrenda <imbrenda@...ux.ibm.com>,
        Oliver Upton <oliver.upton@...ux.dev>
Subject: Re: [PATCH 1/5] KVM: selftests: Implement memcmp(), memcpy(), and
 memset() for guest use

On Thu, Sep 22, 2022 at 10:40 AM Sean Christopherson <seanjc@...gle.com> wrote:
>
> On Thu, Sep 22, 2022, David Matlack wrote:
> > > +LIBKVM_STRING += lib/kvm_string.c
> >
> > Can this file be named lib/string.c instead? This file has nothing to do
> > with KVM per-se.
>
> Yes and no.  I deliberately chose kvm_string to avoid confusion with
> tools/lib/string.c and tools/include/nolibc/string.h.  The implementations
> themselves aren't KVM specific, but the reason the file _exists_ is 100% unique
> to KVM as there is no other environment where tools and/or selftests link to
> glibc but need to override the string ops.
>
> I'm not completely opposed to calling it string.c, but my preference is to keep
> it kvm_string.c so that it's slightly more obvious that KVM selftests are a
> special snowflake.

Makes sense, the "kvm" prefix just made me do a double-take. Perhaps
string_overrides.c? That would make it clear that this file is
overriding string functions. And the fact that this file is in the KVM
selftests directory signals that the overrides are specific to the KVM
selftests.


>
> > > diff --git a/tools/testing/selftests/kvm/lib/kvm_string.c b/tools/testing/selftests/kvm/lib/kvm_string.c
> > > new file mode 100644
> > > index 000000000000..a60d56d4e5b8
> > > --- /dev/null
> > > +++ b/tools/testing/selftests/kvm/lib/kvm_string.c
> > > @@ -0,0 +1,33 @@
> > > +// SPDX-License-Identifier: GPL-2.0-only
> > > +#include "kvm_util.h"
> >
> > Is this include necesary?
>
> Nope, I added the include because I also added declarations in kvm_util_base.h,
> but that's unnecessary because stddef.h also provides the declarations, and those
> _must_ match the prototypes of the definitions.  So yeah, this is better written as:
>
> // SPDX-License-Identifier: GPL-2.0-only
> #include <stddef.h>
>
> /*
>  * Override the "basic" built-in string helpers so that they can be used in
>  * guest code.  KVM selftests don't support dynamic loading in guest code and
>  * will jump into the weeds if the compiler decides to insert an out-of-line
>  * call via the PLT.
>  */
> int memcmp(const void *cs, const void *ct, size_t count)
> {
>         const unsigned char *su1, *su2;
>         int res = 0;
>
>         for (su1 = cs, su2 = ct; 0 < count; ++su1, ++su2, count--) {
>                 if ((res = *su1 - *su2) != 0)
>                         break;
>         }
>         return res;
> }
>
> void *memcpy(void *dest, const void *src, size_t count)
> {
>         char *tmp = dest;
>         const char *s = src;
>
>         while (count--)
>                 *tmp++ = *s++;
>         return dest;
> }
>
> void *memset(void *s, int c, size_t count)
> {
>         char *xs = s;
>
>         while (count--)
>                 *xs++ = c;
>         return s;
> }