Hi Yury, On 04/06/17 13:00, Yury Norov wrote: > Signed-off-by: Yury Norov <ynorov@xxxxxxxxxxxxxxxxxx> 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. > 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. > 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. Thanks, James -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
