linux-kernel - Re: [RFC PATCH] ARM: zynq: Use memcpy_toio instead of memcpy on smp bring-up

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: <26427757-9ed1-36fa-3e4d-7357703ec548@xilinx.com>
Date:   Mon, 5 Aug 2019 11:53:10 +0200
From:   Michal Simek <michal.simek@...inx.com>
To:     Luis Araneda <luaraneda@...il.com>,
        Russell King - ARM Linux admin <linux@...linux.org.uk>
Cc:     Michal Simek <michal.simek@...inx.com>,
        linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
        linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH] ARM: zynq: Use memcpy_toio instead of memcpy on smp
 bring-up

On 31. 07. 19 6:12, Luis Araneda wrote:
> Hi Russell,
> 
> Thanks for reviewing.
> 
> On Tue, Jul 30, 2019 at 6:47 AM Russell King - ARM Linux admin
> <linux@...linux.org.uk> wrote:
>>
>> On Tue, Jul 30, 2019 at 12:43:26AM -0400, Luis Araneda wrote:
>>> This fixes a kernel panic (read overflow) on memcpy when
>>> FORTIFY_SOURCE is enabled.
> [...]
>>
>> I'm not convinced that this is correct.  It looks like
>> zynq_secondary_trampoline could be either ARM or Thumb code - there is
>> no .arm directive before it.  If it's ARM code, then this is fine.  If
>> Thumb code, then zynq_secondary_trampoline will be offset by one, and
>> we will miss copying the first byte of code.
> 
> You're right, I tested what happens if the zynq_secondary_trampoline
> is ARM or Thumb by editing the file where it's defined, headsmp.S
> 
> When the .arm directive is used, the CPU is brought-up correctly,
> but if I use .thumb, I get the following message (no panic):
>> CPU1: failed to come online
> 
> This seems unrelated to solving the panic, as the message
> even appears with memcpy and FORTIFY_SOURCE disabled.
> 
> I could add the .arm directive to headsmp.S
> Is that your expected solution?
> Should that change be on a separate commit?
> 
> I'd like to know Michal's opinion, as he wrote the code.
> 

There are two things together. Thanks Russel to pointing to it.
1. How to support SMP in thumb2 mode?
Adding .arm mode to headsmp.S which ensure that cpu starts in proper
mode and correct code size is copied.
And also point to secondary_startup_arm in zynq_boot_secondary to switch
cpu from arm to thumb mode.

2. And the second is this patch to fix FORTIFY_SOURCE.

Feel free to create the first patch too or I will do it myself.

Thanks,
Michal