From: Catalin Marinas <catalin.marinas@arm.com> To: Suzuki K Poulose <suzuki.poulose@arm.com> Cc: linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, will@kernel.org, maz@kernel.org, mark.rutland@arm.com, dave.martin@arm.com, ard.biesheuvel@linaro.org, christoffer.dall@arm.com, Will Deacon <will.deacon@arm.com> Subject: Re: [PATCH v2 6/7] arm64: signal: nofpsimd: Handle fp/simd context for signal frames Date: Fri, 10 Jan 2020 12:34:42 +0000 [thread overview] Message-ID: <20200110123441.GB8786@arrakis.emea.arm.com> (raw) In-Reply-To: <20191217183402.2259904-7-suzuki.poulose@arm.com> On Tue, Dec 17, 2019 at 06:34:01PM +0000, Suzuki K Poulose wrote: > diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c > index dd2cdc0d5be2..c648f7627035 100644 > --- a/arch/arm64/kernel/signal.c > +++ b/arch/arm64/kernel/signal.c > @@ -173,6 +173,10 @@ static int preserve_fpsimd_context(struct fpsimd_context __user *ctx) > ¤t->thread.uw.fpsimd_state; > int err; > > + /* This must not be called when FP/SIMD support is missing */ > + if (WARN_ON(!system_supports_fpsimd())) > + return -EINVAL; I'd drop this, see below. > @@ -191,6 +195,10 @@ static int restore_fpsimd_context(struct fpsimd_context __user *ctx) > __u32 magic, size; > int err = 0; > > + /* This must not be called when FP/SIMD support is missing */ > + if (WARN_ON(!system_supports_fpsimd())) > + return -EINVAL; > + > /* check the magic/size information */ > __get_user_error(magic, &ctx->head.magic, err); > __get_user_error(size, &ctx->head.size, err); > @@ -261,6 +269,9 @@ static int restore_sve_fpsimd_context(struct user_ctxs *user) > struct user_fpsimd_state fpsimd; > struct sve_context sve; > > + if (WARN_ON(!system_supports_fpsimd())) > + return -EINVAL; > + > if (__copy_from_user(&sve, user->sve, sizeof(sve))) > return -EFAULT; > > @@ -371,6 +382,8 @@ static int parse_user_sigframe(struct user_ctxs *user, > goto done; > > case FPSIMD_MAGIC: > + if (!system_supports_fpsimd()) > + goto invalid; > if (user->fpsimd) > goto invalid; > > @@ -506,7 +519,7 @@ static int restore_sigframe(struct pt_regs *regs, > if (err == 0) > err = parse_user_sigframe(&user, sf); > > - if (err == 0) { > + if (err == 0 && system_supports_fpsimd()) { > if (!user.fpsimd) > return -EINVAL; I don't think we need to be over paranoid here and add three/four checks and two warnings in static functions. parse_user_sigframe() already returns -EINVAL if SVE or FPSIMD is missing (the latter with your check above). We don't need this additional check in restore_sigframe() and restore_{sve_,}fpsimd_context(), the call paths are simple enough. > > @@ -623,7 +636,7 @@ static int setup_sigframe(struct rt_sigframe_user_layout *user, > > err |= __copy_to_user(&sf->uc.uc_sigmask, set, sizeof(*set)); > > - if (err == 0) { > + if (err == 0 && system_supports_fpsimd()) { > struct fpsimd_context __user *fpsimd_ctx = > apply_user_offset(user, user->fpsimd_offset); > err |= preserve_fpsimd_context(fpsimd_ctx); This check is also sufficient for a static function not to have the WARN_ON. > diff --git a/arch/arm64/kernel/signal32.c b/arch/arm64/kernel/signal32.c > index 12a585386c2f..97ace6919bc2 100644 > --- a/arch/arm64/kernel/signal32.c > +++ b/arch/arm64/kernel/signal32.c > @@ -100,6 +100,9 @@ static int compat_preserve_vfp_context(struct compat_vfp_sigframe __user *frame) > compat_ulong_t fpscr, fpexc; > int i, err = 0; > > + /* This must not be called when the FP/SIMD is missing */ > + if (WARN_ON(!system_supports_fpsimd())) > + return -EINVAL; > /* > * Save the hardware registers to the fpsimd_state structure. > * Note that this also saves V16-31, which aren't visible > @@ -149,6 +152,10 @@ static int compat_restore_vfp_context(struct compat_vfp_sigframe __user *frame) > compat_ulong_t fpscr; > int i, err = 0; > > + /* This must not be called when the FP/SIMD is missing */ > + if (WARN_ON(!system_supports_fpsimd())) > + return -EINVAL; > + > __get_user_error(magic, &frame->magic, err); > __get_user_error(size, &frame->size, err); > > @@ -223,7 +230,7 @@ static int compat_restore_sigframe(struct pt_regs *regs, > err |= !valid_user_regs(®s->user_regs, current); > > aux = (struct compat_aux_sigframe __user *) sf->uc.uc_regspace; > - if (err == 0) > + if (err == 0 && system_supports_fpsimd()) > err |= compat_restore_vfp_context(&aux->vfp); > > return err; > @@ -419,7 +426,7 @@ static int compat_setup_sigframe(struct compat_sigframe __user *sf, > > aux = (struct compat_aux_sigframe __user *) sf->uc.uc_regspace; > > - if (err == 0) > + if (err == 0 && system_supports_fpsimd()) > err |= compat_preserve_vfp_context(&aux->vfp); > __put_user_error(0, &aux->end_magic, err); Same comments as for the native signals. -- Catalin
WARNING: multiple messages have this Message-ID (diff)
From: Catalin Marinas <catalin.marinas@arm.com> To: Suzuki K Poulose <suzuki.poulose@arm.com> Cc: mark.rutland@arm.com, ard.biesheuvel@linaro.org, maz@kernel.org, Will Deacon <will.deacon@arm.com>, linux-kernel@vger.kernel.org, christoffer.dall@arm.com, will@kernel.org, dave.martin@arm.com, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v2 6/7] arm64: signal: nofpsimd: Handle fp/simd context for signal frames Date: Fri, 10 Jan 2020 12:34:42 +0000 [thread overview] Message-ID: <20200110123441.GB8786@arrakis.emea.arm.com> (raw) In-Reply-To: <20191217183402.2259904-7-suzuki.poulose@arm.com> On Tue, Dec 17, 2019 at 06:34:01PM +0000, Suzuki K Poulose wrote: > diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c > index dd2cdc0d5be2..c648f7627035 100644 > --- a/arch/arm64/kernel/signal.c > +++ b/arch/arm64/kernel/signal.c > @@ -173,6 +173,10 @@ static int preserve_fpsimd_context(struct fpsimd_context __user *ctx) > ¤t->thread.uw.fpsimd_state; > int err; > > + /* This must not be called when FP/SIMD support is missing */ > + if (WARN_ON(!system_supports_fpsimd())) > + return -EINVAL; I'd drop this, see below. > @@ -191,6 +195,10 @@ static int restore_fpsimd_context(struct fpsimd_context __user *ctx) > __u32 magic, size; > int err = 0; > > + /* This must not be called when FP/SIMD support is missing */ > + if (WARN_ON(!system_supports_fpsimd())) > + return -EINVAL; > + > /* check the magic/size information */ > __get_user_error(magic, &ctx->head.magic, err); > __get_user_error(size, &ctx->head.size, err); > @@ -261,6 +269,9 @@ static int restore_sve_fpsimd_context(struct user_ctxs *user) > struct user_fpsimd_state fpsimd; > struct sve_context sve; > > + if (WARN_ON(!system_supports_fpsimd())) > + return -EINVAL; > + > if (__copy_from_user(&sve, user->sve, sizeof(sve))) > return -EFAULT; > > @@ -371,6 +382,8 @@ static int parse_user_sigframe(struct user_ctxs *user, > goto done; > > case FPSIMD_MAGIC: > + if (!system_supports_fpsimd()) > + goto invalid; > if (user->fpsimd) > goto invalid; > > @@ -506,7 +519,7 @@ static int restore_sigframe(struct pt_regs *regs, > if (err == 0) > err = parse_user_sigframe(&user, sf); > > - if (err == 0) { > + if (err == 0 && system_supports_fpsimd()) { > if (!user.fpsimd) > return -EINVAL; I don't think we need to be over paranoid here and add three/four checks and two warnings in static functions. parse_user_sigframe() already returns -EINVAL if SVE or FPSIMD is missing (the latter with your check above). We don't need this additional check in restore_sigframe() and restore_{sve_,}fpsimd_context(), the call paths are simple enough. > > @@ -623,7 +636,7 @@ static int setup_sigframe(struct rt_sigframe_user_layout *user, > > err |= __copy_to_user(&sf->uc.uc_sigmask, set, sizeof(*set)); > > - if (err == 0) { > + if (err == 0 && system_supports_fpsimd()) { > struct fpsimd_context __user *fpsimd_ctx = > apply_user_offset(user, user->fpsimd_offset); > err |= preserve_fpsimd_context(fpsimd_ctx); This check is also sufficient for a static function not to have the WARN_ON. > diff --git a/arch/arm64/kernel/signal32.c b/arch/arm64/kernel/signal32.c > index 12a585386c2f..97ace6919bc2 100644 > --- a/arch/arm64/kernel/signal32.c > +++ b/arch/arm64/kernel/signal32.c > @@ -100,6 +100,9 @@ static int compat_preserve_vfp_context(struct compat_vfp_sigframe __user *frame) > compat_ulong_t fpscr, fpexc; > int i, err = 0; > > + /* This must not be called when the FP/SIMD is missing */ > + if (WARN_ON(!system_supports_fpsimd())) > + return -EINVAL; > /* > * Save the hardware registers to the fpsimd_state structure. > * Note that this also saves V16-31, which aren't visible > @@ -149,6 +152,10 @@ static int compat_restore_vfp_context(struct compat_vfp_sigframe __user *frame) > compat_ulong_t fpscr; > int i, err = 0; > > + /* This must not be called when the FP/SIMD is missing */ > + if (WARN_ON(!system_supports_fpsimd())) > + return -EINVAL; > + > __get_user_error(magic, &frame->magic, err); > __get_user_error(size, &frame->size, err); > > @@ -223,7 +230,7 @@ static int compat_restore_sigframe(struct pt_regs *regs, > err |= !valid_user_regs(®s->user_regs, current); > > aux = (struct compat_aux_sigframe __user *) sf->uc.uc_regspace; > - if (err == 0) > + if (err == 0 && system_supports_fpsimd()) > err |= compat_restore_vfp_context(&aux->vfp); > > return err; > @@ -419,7 +426,7 @@ static int compat_setup_sigframe(struct compat_sigframe __user *sf, > > aux = (struct compat_aux_sigframe __user *) sf->uc.uc_regspace; > > - if (err == 0) > + if (err == 0 && system_supports_fpsimd()) > err |= compat_preserve_vfp_context(&aux->vfp); > __put_user_error(0, &aux->end_magic, err); Same comments as for the native signals. -- Catalin _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2020-01-10 12:34 UTC|newest] Thread overview: 46+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-12-17 18:33 [PATCH v2 0/7] arm64: Fix support for no FP/SIMD Suzuki K Poulose 2019-12-17 18:33 ` Suzuki K Poulose 2019-12-17 18:33 ` [PATCH v2 1/7] arm64: Introduce system_capabilities_finalized() marker Suzuki K Poulose 2019-12-17 18:33 ` Suzuki K Poulose 2020-01-10 14:50 ` Catalin Marinas 2020-01-10 14:50 ` Catalin Marinas 2019-12-17 18:33 ` [PATCH v2 2/7] arm64: fpsimd: Make sure SVE setup is complete before SIMD is used Suzuki K Poulose 2019-12-17 18:33 ` Suzuki K Poulose 2020-01-10 11:51 ` Catalin Marinas 2020-01-10 11:51 ` Catalin Marinas 2020-01-10 18:41 ` Suzuki Kuruppassery Poulose 2020-01-10 18:41 ` Suzuki Kuruppassery Poulose 2019-12-17 18:33 ` [PATCH v2 3/7] arm64: cpufeature: Fix the type of no FP/SIMD capability Suzuki K Poulose 2019-12-17 18:33 ` Suzuki K Poulose 2020-01-10 14:50 ` Catalin Marinas 2020-01-10 14:50 ` Catalin Marinas 2019-12-17 18:33 ` [PATCH v2 4/7] arm64: cpufeature: Set the FP/SIMD compat HWCAP bits properly Suzuki K Poulose 2019-12-17 18:33 ` Suzuki K Poulose 2020-01-10 14:51 ` Catalin Marinas 2020-01-10 14:51 ` Catalin Marinas 2019-12-17 18:34 ` [PATCH v2 5/7] arm64: ptrace: nofpsimd: Fail FP/SIMD regset operations Suzuki K Poulose 2019-12-17 18:34 ` Suzuki K Poulose 2020-01-10 15:12 ` Catalin Marinas 2020-01-10 15:12 ` Catalin Marinas 2020-01-10 18:42 ` Suzuki Kuruppassery Poulose 2020-01-10 18:42 ` Suzuki Kuruppassery Poulose 2019-12-17 18:34 ` [PATCH v2 6/7] arm64: signal: nofpsimd: Handle fp/simd context for signal frames Suzuki K Poulose 2019-12-17 18:34 ` Suzuki K Poulose 2020-01-10 12:34 ` Catalin Marinas [this message] 2020-01-10 12:34 ` Catalin Marinas 2019-12-17 18:34 ` [PATCH v2 7/7] arm64: nofpsmid: Handle TIF_FOREIGN_FPSTATE flag cleanly Suzuki K Poulose 2019-12-17 18:34 ` Suzuki K Poulose 2019-12-17 19:05 ` Marc Zyngier 2019-12-17 19:05 ` Marc Zyngier 2019-12-18 11:42 ` Suzuki Kuruppassery Poulose 2019-12-18 11:42 ` Suzuki Kuruppassery Poulose 2019-12-18 11:56 ` Marc Zyngier 2019-12-18 11:56 ` Marc Zyngier 2019-12-18 12:00 ` Suzuki Kuruppassery Poulose 2019-12-18 12:00 ` Suzuki Kuruppassery Poulose 2020-01-10 15:21 ` Marc Zyngier 2020-01-10 15:21 ` Marc Zyngier 2020-01-13 10:28 ` Suzuki Kuruppassery Poulose 2020-01-13 10:28 ` Suzuki Kuruppassery Poulose 2020-01-10 14:49 ` Catalin Marinas 2020-01-10 14:49 ` Catalin Marinas
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=20200110123441.GB8786@arrakis.emea.arm.com \ --to=catalin.marinas@arm.com \ --cc=ard.biesheuvel@linaro.org \ --cc=christoffer.dall@arm.com \ --cc=dave.martin@arm.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=mark.rutland@arm.com \ --cc=maz@kernel.org \ --cc=suzuki.poulose@arm.com \ --cc=will.deacon@arm.com \ --cc=will@kernel.org \ /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.