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
| ||
|
Date: Tue, 20 Jun 2017 17:16:23 +0300 From: Yury Norov <ynorov@...iumnetworks.com> To: James Morse <james.morse@....com> Cc: Catalin Marinas <catalin.marinas@....com>, linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org, linux-doc@...r.kernel.org, Arnd Bergmann <arnd@...db.de>, Andrew Pinski <Andrew.Pinski@...iumnetworks.com>, Heiko Carstens <heiko.carstens@...ibm.com>, Chris Metcalf <cmetcalf@...hip.com>, philipp.tomsich@...obroma-systems.com, Joseph Myers <joseph@...esourcery.com>, zhouchengming1@...wei.com, Steve Ellcey <sellcey@...iumnetworks.com>, Prasun.Kapoor@...iumnetworks.com, Andreas Schwab <schwab@...e.de>, agraf@...e.de, szabolcs.nagy@....com, geert@...ux-m68k.org, Adam Borowski <kilobyte@...band.pl>, manuel.montezelo@...il.com, Chris Metcalf <cmetcalf@...lanox.com>, Andrew Pinski <pinskia@...il.com>, linyongting@...wei.com, klimov.linux@...il.com, broonie@...nel.org, Bamvor Zhangjian <bamvor.zhangjian@...wei.com>, Maxim Kuvyrkov <maxim.kuvyrkov@...aro.org>, Florian Weimer <fweimer@...hat.com>, Nathan_Lynch@...tor.com, Ramana Radhakrishnan <ramana.gcc@...glemail.com>, schwidefsky@...ibm.com, davem@...emloft.net, christoph.muellner@...obroma-systems.com Subject: Re: [PATCH 16/20] arm64: signal32: move ilp32 and aarch32 common code to separated file On Mon, Jun 19, 2017 at 05:16:42PM +0100, James Morse wrote: > Hi Yury, > > On 04/06/17 13:00, Yury Norov wrote: > > Signed-off-by: Yury Norov <ynorov@...iumnetworks.com> > > Can I offer a body for the commit message: > ILP32 needs to mix 32bit struct siginfo and 64bit sigframe for its signal > handlers. Move the existing compat code for copying siginfo to user space and > manipulating signal masks into signal32_common.c so it can be used to deliver > aarch32 and ilp32 signals. Ok > > diff --git a/arch/arm64/include/asm/signal32.h b/arch/arm64/include/asm/signal32.h > > index e68fcce538e1..1c4ede717bd2 100644 > > --- a/arch/arm64/include/asm/signal32.h > > +++ b/arch/arm64/include/asm/signal32.h > > @@ -13,6 +13,9 @@ > > * You should have received a copy of the GNU General Public License > > * along with this program. If not, see <http://www.gnu.org/licenses/>. > > */ > > + > > +#include <asm/signal32_common.h> > > + > > #ifndef __ASM_SIGNAL32_H > > #define __ASM_SIGNAL32_H > > Nit: This should go inside the guard. Ok, thanks. Will fix this and all below > > diff --git a/arch/arm64/kernel/signal32_common.c b/arch/arm64/kernel/signal32_common.c > > new file mode 100644 > > index 000000000000..5bddc25dca12 > > --- /dev/null > > +++ b/arch/arm64/kernel/signal32_common.c > > @@ -0,0 +1,135 @@ > [...] > > +#include <linux/compat.h> > > +#include <linux/signal.h> > > +#include <linux/ratelimit.h> > > What do you need ratelimit.h for? > > > > +#include <linux/uaccess.h> > > + > > +#include <asm/esr.h> > > I can't see anything using these ESR_ macros in here... > > > > +#include <asm/fpsimd.h> > > This was for the VFP save/restore code, which you didn't move... > > > > +#include <asm/signal32_common.h> > > +#include <asm/unistd.h> > > [...] > > > > +int copy_siginfo_to_user32(compat_siginfo_t __user *to, const siginfo_t *from) > [...] > > + case __SI_FAULT: > > + err |= __put_user((compat_uptr_t)(unsigned long)from->si_addr, > > + &to->si_addr); > > This looks tricky. si_addr comes from FAR_EL1 when user-space touches something > it shouldn't. This could be a 64bit value as ilp32 processes can still branch to > 64bit addresses in registers and generate loads that cross the invisible 4GB > boundary. Here you truncate the 64bit address. > Obviously this can't happen at all with aarch32, and for C programs its into > undefined-behaviour territory, but it doesn't feel right to pass an address to > user-space that we know is wrong... but we don't have an alternative. > > This looks like a class of problem particular to ilp32/x32: 'accessed an address > you can't encode with a signal'. After a quick dig in x86's x32 code, it looks > like they only pass the first 32bits of si_addr too. > > One option is to mint a new si_code to go with SIGBUS meaning something like > 'address overflowed si_addr'. Alternatively we could just kill tasks that do this. New SIGBUS sounds reasonable at the first glance, but I think it should be discussed widely at first, and the patch that implements it should touch all arches that may be affected. Yury
Powered by blists - more mailing lists