Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.35 as permitted sender) client-ip=23.128.96.35;
MIME-Version: 1.0
References: <20231007170448.505487-1-masahiroy@kernel.org> <20231007170448.505487-3-masahiroy@kernel.org>
 <CAKwvOdkP-28Z51UZcDL4434Uns9pb0kYYFwzHmQCg7x2V0E_TQ@mail.gmail.com>
In-Reply-To: <CAKwvOdkP-28Z51UZcDL4434Uns9pb0kYYFwzHmQCg7x2V0E_TQ@mail.gmail.com>
From:   Masahiro Yamada <masahiroy@kernel.org>
Date:   Sat, 14 Oct 2023 23:16:32 +0900
Message-ID: <CAK7LNARwDqb4hTLLNPZ1sG_CYGWHF7y88LRhaeS0BKkaancX8w@mail.gmail.com>
Subject: Re: [PATCH 3/5] modpost: define TO_NATIVE() using bswap_* functions
To:     Nick Desaulniers <ndesaulniers@google.com>
Cc:     linux-kbuild@vger.kernel.org, linux-kernel@vger.kernel.org,
        Nathan Chancellor <nathan@kernel.org>,
        Nicolas Schier <nicolas@fjasle.eu>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Tue, Oct 10, 2023 at 2:44=E2=80=AFAM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> On Sat, Oct 7, 2023 at 10:05=E2=80=AFAM Masahiro Yamada <masahiroy@kernel=
.org> wrote:
> >
> > The current TO_NATIVE() has some limitations:
> >
> >  1) You cannot cast the argument.
> >
> >  2) You cannot pass a variable marked as 'const'.
> >
> >  3) Passing an array is a bug, but it is not detected.
> >
> > Impelement TO_NATIVE() using bswap_*() functions. These are GNU
> > extensions. If we face portability issues, we can port the code from
> > include/uapi/linux/swab.h.
> >
> > With this change, get_rel_type_and_sym() can be simplified by casting
> > the arguments directly.
> >
> > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> > ---
> >
> >  scripts/mod/modpost.c | 13 ++++---------
> >  scripts/mod/modpost.h | 25 ++++++++++++-------------
> >  2 files changed, 16 insertions(+), 22 deletions(-)
> >
> > diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> > index 2f3b0fe6f68d..99476a9695c5 100644
> > --- a/scripts/mod/modpost.c
> > +++ b/scripts/mod/modpost.c
> > @@ -1410,15 +1410,10 @@ static void get_rel_type_and_sym(struct elf_inf=
o *elf, uint64_t r_info,
> >                 return;
> >         }
> >
> > -       if (is_64bit) {
> > -               Elf64_Xword r_info64 =3D r_info;
> > -
> > -               r_info =3D TO_NATIVE(r_info64);
> > -       } else {
> > -               Elf32_Word r_info32 =3D r_info;
> > -
> > -               r_info =3D TO_NATIVE(r_info32);
> > -       }
> > +       if (is_64bit)
> > +               r_info =3D TO_NATIVE((Elf64_Xword)r_info);
> > +       else
> > +               r_info =3D TO_NATIVE((Elf32_Word)r_info);
> >
> >         *r_type =3D ELF_R_TYPE(r_info);
> >         *r_sym =3D ELF_R_SYM(r_info);
> > diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h
> > index 6413f26fcb6b..1392afec118c 100644
> > --- a/scripts/mod/modpost.h
> > +++ b/scripts/mod/modpost.h
> > @@ -1,4 +1,5 @@
> >  /* SPDX-License-Identifier: GPL-2.0 */
> > +#include <byteswap.h>
> >  #include <stdbool.h>
> >  #include <stdio.h>
> >  #include <stdlib.h>
> > @@ -51,21 +52,19 @@
> >  #define ELF_R_TYPE  ELF64_R_TYPE
> >  #endif
> >
> > +#define bswap(x) \
> > +({ \
> > +       _Static_assert(sizeof(x) =3D=3D 1 || sizeof(x) =3D=3D 2 || \
>
> Seems fine, but do we need to support folks trying to swap 1B values?
> i.e. is someone calling TO_NATIVE with 1B values?


Yes.

In scripts/mod/file2alias.c,
DEF_FIELD() calls TO_NATIVE().

DEF_FIELD() is also used to get access to 1-byte fields.
So, TO_NATIVE() needs to accept sizeof(x)=3D=3D1.


>  Seems silly unless
> one of these types is variable length dependent on the target machine
> type?


You can use DEF_FIELD() without knowing the
field width. This is good.


--=20
Best Regards
Masahiro Yamada