All of lore.kernel.org
 help / color / mirror / Atom feed
From: Masahiro Yamada <masahiroy@kernel.org>
To: Ard Biesheuvel <ardb@kernel.org>
Cc: linux-arm-kernel@lists.infradead.org,
	Russell King <linux@armlinux.org.uk>,
	linux-kernel@vger.kernel.org, linux-kbuild@vger.kernel.org,
	Nick Desaulniers <ndesaulniers@google.com>
Subject: Re: [PATCH] ARM: fix get_user() broken with veneer
Date: Tue, 26 Sep 2023 23:26:48 +0900	[thread overview]
Message-ID: <CAK7LNAR5bjfRShbbPsoAriGR5=fG85yzBWfvBFaaK_=kd5U_LA@mail.gmail.com> (raw)
In-Reply-To: <CAMj1kXHM_dRNVeMuwiPyniz4qUh8JT6mmX2R2VdhBMLCjFA8WQ@mail.gmail.com>

On Tue, Sep 26, 2023 at 8:37 PM Ard Biesheuvel <ardb@kernel.org> wrote:
>
> Hello Masahiro,
>
> Thanks for getting to the bottom of this.
>
> The patch looks correct to me, but could we please drop this huge
> commit log text? The AAPCS simply states that a BL instruction might
> clobber IP, so any inline asm that contains a BL should have IP in its
> clobber list. In our case, the use of PIC veneers is what might cause
> this, so we should mention that as well. But your entire journey to
> get to that conclusion is not something that deserves to be included
> in the git commit log forever, imho.
>
> So with the commit log trimmed:
>
> Reviewed-by: Ard Biesheuvel <ardb@kernel.org>



Is this commit log OK?


===================================>8============================
The 32-bit ARM kernel stops working if the kernel grows to the point
where veneers for __get_user_* are created.

AAPCS32 [1] states, "Register r12 (IP) may be used by a linker as a
scratch register between a routine and any subroutine it calls. It
can also be used within a routine to hold intermediate values between
subroutine calls."

However, the bl instruction buried within the inline asm is unpredictable
for compilers; hence, "ip" must be added to the clobber list.

This becomes critical when veneers for __get_user_* are created because
veneers use the ip register since commit 02e541db0540 ("ARM: 8323/1:
force linker to use PIC veneers").

[1]: https://github.com/ARM-software/abi-aa/blob/2023Q1/aapcs32/aapcs32.rst
===================================>8============================






-- 
Best Regards
Masahiro Yamada

WARNING: multiple messages have this Message-ID (diff)
From: Masahiro Yamada <masahiroy@kernel.org>
To: Ard Biesheuvel <ardb@kernel.org>
Cc: linux-arm-kernel@lists.infradead.org,
	Russell King <linux@armlinux.org.uk>,
	 linux-kernel@vger.kernel.org, linux-kbuild@vger.kernel.org,
	 Nick Desaulniers <ndesaulniers@google.com>
Subject: Re: [PATCH] ARM: fix get_user() broken with veneer
Date: Tue, 26 Sep 2023 23:26:48 +0900	[thread overview]
Message-ID: <CAK7LNAR5bjfRShbbPsoAriGR5=fG85yzBWfvBFaaK_=kd5U_LA@mail.gmail.com> (raw)
In-Reply-To: <CAMj1kXHM_dRNVeMuwiPyniz4qUh8JT6mmX2R2VdhBMLCjFA8WQ@mail.gmail.com>

On Tue, Sep 26, 2023 at 8:37 PM Ard Biesheuvel <ardb@kernel.org> wrote:
>
> Hello Masahiro,
>
> Thanks for getting to the bottom of this.
>
> The patch looks correct to me, but could we please drop this huge
> commit log text? The AAPCS simply states that a BL instruction might
> clobber IP, so any inline asm that contains a BL should have IP in its
> clobber list. In our case, the use of PIC veneers is what might cause
> this, so we should mention that as well. But your entire journey to
> get to that conclusion is not something that deserves to be included
> in the git commit log forever, imho.
>
> So with the commit log trimmed:
>
> Reviewed-by: Ard Biesheuvel <ardb@kernel.org>



Is this commit log OK?


===================================>8============================
The 32-bit ARM kernel stops working if the kernel grows to the point
where veneers for __get_user_* are created.

AAPCS32 [1] states, "Register r12 (IP) may be used by a linker as a
scratch register between a routine and any subroutine it calls. It
can also be used within a routine to hold intermediate values between
subroutine calls."

However, the bl instruction buried within the inline asm is unpredictable
for compilers; hence, "ip" must be added to the clobber list.

This becomes critical when veneers for __get_user_* are created because
veneers use the ip register since commit 02e541db0540 ("ARM: 8323/1:
force linker to use PIC veneers").

[1]: https://github.com/ARM-software/abi-aa/blob/2023Q1/aapcs32/aapcs32.rst
===================================>8============================






-- 
Best Regards
Masahiro Yamada

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2023-09-26 14:27 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-25 11:00 [PATCH] ARM: fix get_user() broken with veneer Masahiro Yamada
2023-09-25 11:00 ` Masahiro Yamada
2023-09-26 11:37 ` Ard Biesheuvel
2023-09-26 11:37   ` Ard Biesheuvel
2023-09-26 14:26   ` Masahiro Yamada [this message]
2023-09-26 14:26     ` Masahiro Yamada
2023-09-26 15:09     ` Ard Biesheuvel
2023-09-26 15:09       ` Ard Biesheuvel
2023-09-26 16:09 Masahiro Yamada
2023-09-26 16:09 ` Masahiro Yamada

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAK7LNAR5bjfRShbbPsoAriGR5=fG85yzBWfvBFaaK_=kd5U_LA@mail.gmail.com' \
    --to=masahiroy@kernel.org \
    --cc=ardb@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=ndesaulniers@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.