From: Catalin Marinas <catalin.marinas@arm.com> To: Yury Norov <ynorov@caviumnetworks.com> Cc: Pratyush Anand <panand@redhat.com>, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 2/2] arm64: cleanup {COMPAT_,}SET_PERSONALITY() macro Date: Tue, 8 Aug 2017 14:55:05 +0100 [thread overview] Message-ID: <20170808135504.pgbc5bbk6nytoc6u@armageddon.cambridge.arm.com> (raw) In-Reply-To: <20170805144022.17260-3-ynorov@caviumnetworks.com> On Sat, Aug 05, 2017 at 05:40:22PM +0300, Yury Norov wrote: > Originally {COMPAT_,}SET_PERSONALITY() only sets the 32-bit flag in thread_info > structure. But there is some work that should be done after setting the personality. > Currently it's done in the macro, which is not the best idea. > > In this patch new arch_setup_new_exec() routine is introduced, and all setup code > is moved there, as suggested by Catalin: > https://lkml.org/lkml/2017/8/4/494 > > Note: mm->context.flags doesn't require the atomic strong ordered acceess to the > field, so use __set_bit() there; As I replied to patch 1, we don't even need __set_bit() but just '|=' for setting and '&' for testing. > diff --git a/arch/arm64/include/asm/elf.h b/arch/arm64/include/asm/elf.h > index de11ed1484e3..615953243961 100644 > --- a/arch/arm64/include/asm/elf.h > +++ b/arch/arm64/include/asm/elf.h > @@ -137,11 +137,14 @@ typedef struct user_fpsimd_state elf_fpregset_t; > */ > #define ELF_PLAT_INIT(_r, load_addr) (_r)->regs[0] = 0 > > +/* > + * Don't modify this macro unless you add new personality. > + * All personality-related setup should be done at proper place. > + * If not sure, consider the arch_setup_new_exec() function. > + */ > #define SET_PERSONALITY(ex) \ > ({ \ > - clear_bit(MMCF_AARCH32, ¤t->mm->context.flags); \ > clear_thread_flag(TIF_32BIT); \ > - current->personality &= ~READ_IMPLIES_EXEC; \ > }) What I meant is that we keep the personality setting in SET_PERSONALITY, together with the existing TIF bits but just move the mm->context.flags setting out. > diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h > index 46c3b93cf865..c823d2f12b4c 100644 > --- a/arch/arm64/include/asm/thread_info.h > +++ b/arch/arm64/include/asm/thread_info.h > @@ -68,6 +68,9 @@ struct thread_info { > #define thread_saved_fp(tsk) \ > ((unsigned long)(tsk->thread.cpu_context.fp)) > > +void arch_setup_new_exec(void); > +#define arch_setup_new_exec arch_setup_new_exec I'm fine with out of line implementation, it probably helps with any header conflicts (and it's not a fast path anyway). > diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c > index 659ae8094ed5..ebca9e4f62c7 100644 > --- a/arch/arm64/kernel/process.c > +++ b/arch/arm64/kernel/process.c > @@ -417,3 +417,20 @@ unsigned long arch_randomize_brk(struct mm_struct *mm) > else > return randomize_page(mm->brk, SZ_1G); > } > + > +/* > + * Called immediately after a successful exec. > + */ > +void arch_setup_new_exec(void) > +{ > + current->mm->context.flags = 0; > + > + /* > + * Unlike the native one, the compat version of exec() inherits > + * READ_IMPLIES_EXEC since this is the behaviour on arch/arm/. > + */ > + if (is_compat_task()) > + __set_bit(MMCF_AARCH32, ¤t->mm->context.flags); > + else > + current->personality &= ~READ_IMPLIES_EXEC; > +} As I said above, just context.flags |= MMCF_AARCH32. Thanks. -- Catalin
WARNING: multiple messages have this Message-ID (diff)
From: catalin.marinas@arm.com (Catalin Marinas) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH 2/2] arm64: cleanup {COMPAT_,}SET_PERSONALITY() macro Date: Tue, 8 Aug 2017 14:55:05 +0100 [thread overview] Message-ID: <20170808135504.pgbc5bbk6nytoc6u@armageddon.cambridge.arm.com> (raw) In-Reply-To: <20170805144022.17260-3-ynorov@caviumnetworks.com> On Sat, Aug 05, 2017 at 05:40:22PM +0300, Yury Norov wrote: > Originally {COMPAT_,}SET_PERSONALITY() only sets the 32-bit flag in thread_info > structure. But there is some work that should be done after setting the personality. > Currently it's done in the macro, which is not the best idea. > > In this patch new arch_setup_new_exec() routine is introduced, and all setup code > is moved there, as suggested by Catalin: > https://lkml.org/lkml/2017/8/4/494 > > Note: mm->context.flags doesn't require the atomic strong ordered acceess to the > field, so use __set_bit() there; As I replied to patch 1, we don't even need __set_bit() but just '|=' for setting and '&' for testing. > diff --git a/arch/arm64/include/asm/elf.h b/arch/arm64/include/asm/elf.h > index de11ed1484e3..615953243961 100644 > --- a/arch/arm64/include/asm/elf.h > +++ b/arch/arm64/include/asm/elf.h > @@ -137,11 +137,14 @@ typedef struct user_fpsimd_state elf_fpregset_t; > */ > #define ELF_PLAT_INIT(_r, load_addr) (_r)->regs[0] = 0 > > +/* > + * Don't modify this macro unless you add new personality. > + * All personality-related setup should be done at proper place. > + * If not sure, consider the arch_setup_new_exec() function. > + */ > #define SET_PERSONALITY(ex) \ > ({ \ > - clear_bit(MMCF_AARCH32, ¤t->mm->context.flags); \ > clear_thread_flag(TIF_32BIT); \ > - current->personality &= ~READ_IMPLIES_EXEC; \ > }) What I meant is that we keep the personality setting in SET_PERSONALITY, together with the existing TIF bits but just move the mm->context.flags setting out. > diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h > index 46c3b93cf865..c823d2f12b4c 100644 > --- a/arch/arm64/include/asm/thread_info.h > +++ b/arch/arm64/include/asm/thread_info.h > @@ -68,6 +68,9 @@ struct thread_info { > #define thread_saved_fp(tsk) \ > ((unsigned long)(tsk->thread.cpu_context.fp)) > > +void arch_setup_new_exec(void); > +#define arch_setup_new_exec arch_setup_new_exec I'm fine with out of line implementation, it probably helps with any header conflicts (and it's not a fast path anyway). > diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c > index 659ae8094ed5..ebca9e4f62c7 100644 > --- a/arch/arm64/kernel/process.c > +++ b/arch/arm64/kernel/process.c > @@ -417,3 +417,20 @@ unsigned long arch_randomize_brk(struct mm_struct *mm) > else > return randomize_page(mm->brk, SZ_1G); > } > + > +/* > + * Called immediately after a successful exec. > + */ > +void arch_setup_new_exec(void) > +{ > + current->mm->context.flags = 0; > + > + /* > + * Unlike the native one, the compat version of exec() inherits > + * READ_IMPLIES_EXEC since this is the behaviour on arch/arm/. > + */ > + if (is_compat_task()) > + __set_bit(MMCF_AARCH32, ¤t->mm->context.flags); > + else > + current->personality &= ~READ_IMPLIES_EXEC; > +} As I said above, just context.flags |= MMCF_AARCH32. Thanks. -- Catalin
next prev parent reply other threads:[~2017-08-08 13:55 UTC|newest] Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-08-05 14:40 [PATCH 0/2] arm64: cleanup {COMPAT_,}SET_PERSONALITY Yury Norov 2017-08-05 14:40 ` Yury Norov 2017-08-05 14:40 ` [PATCH 1/2] arm64: introduce separated bits for mm_context_t flags Yury Norov 2017-08-05 14:40 ` Yury Norov 2017-08-08 13:51 ` Catalin Marinas 2017-08-08 13:51 ` Catalin Marinas 2017-08-05 14:40 ` [PATCH 2/2] arm64: cleanup {COMPAT_,}SET_PERSONALITY() macro Yury Norov 2017-08-05 14:40 ` Yury Norov 2017-08-08 13:55 ` Catalin Marinas [this message] 2017-08-08 13:55 ` Catalin Marinas 2017-08-20 10:20 [PATCH v2 0/2] cleanup {COMPAT_,}SET_PERSONALITY Yury Norov 2017-08-20 10:20 ` [PATCH 2/2] arm64: cleanup {COMPAT_,}SET_PERSONALITY() macro Yury Norov 2017-08-20 10:20 ` Yury Norov
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=20170808135504.pgbc5bbk6nytoc6u@armageddon.cambridge.arm.com \ --to=catalin.marinas@arm.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=panand@redhat.com \ --cc=ynorov@caviumnetworks.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: linkBe 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.