All of lore.kernel.org
 help / color / mirror / Atom feed
From: Conor Dooley <conor@kernel.org>
To: kvm-riscv@lists.infradead.org
Subject: [PATCH -next v14 12/19] riscv: signal: check fp-reserved words unconditionally
Date: Wed, 1 Mar 2023 17:56:56 +0000	[thread overview]
Message-ID: <Y/+R6NfBHSGGI3MX@spud> (raw)
In-Reply-To: <20230224170118.16766-13-andy.chiu@sifive.com>

On Fri, Feb 24, 2023 at 05:01:11PM +0000, Andy Chiu wrote:
> In order to let kernel/user locate and identify an extension context on
> the existing sigframe, we are going to utilize reserved space of fp and
> encode the information there. And since the sigcontext has already
> preserved a space for fp context w or w/o CONFIG_FPU, we move those
> reserved words checking/setting routine back into generic code.
> 
> This commit also undone an additional logical change carried by the
> refactor commit 007f5c3589578
> ("Refactor FPU code in signal setup/return procedures"). Originally we
> did not restore fp context if restoring of gpr have failed. And it was
> fine on the other side. In such way the kernel could keep the regfiles
> intact, and potentially react at the failing point of restore.
> 
> Signed-off-by: Andy Chiu <andy.chiu@sifive.com>
> ---

> @@ -90,11 +66,29 @@ static long restore_sigcontext(struct pt_regs *regs,
>  	struct sigcontext __user *sc)
>  {
>  	long err;
> +	size_t i;
> +
>  	/* sc_regs is structured the same as the start of pt_regs */
>  	err = __copy_from_user(regs, &sc->sc_regs, sizeof(sc->sc_regs));
> +	if (unlikely(err))
> +		return err;
>  	/* Restore the floating-point state. */

Please preserve the newline after return from where you moved this from.

> -	if (has_fpu())
> -		err |= restore_fp_state(regs, &sc->sc_fpregs);
> +	if (has_fpu()) {
> +		err = restore_fp_state(regs, &sc->sc_fpregs);
> +		if (unlikely(err))
> +			return err;
> +	}

> @@ -145,11 +139,16 @@ static long setup_sigcontext(struct rt_sigframe __user *frame,
>  {
>  	struct sigcontext __user *sc = &frame->uc.uc_mcontext;
>  	long err;
> +	size_t i;
> +
>  	/* sc_regs is structured the same as the start of pt_regs */
>  	err = __copy_to_user(&sc->sc_regs, regs, sizeof(sc->sc_regs));
>  	/* Save the floating-point state. */
>  	if (has_fpu())
>  		err |= save_fp_state(regs, &sc->sc_fpregs);
> +	/* We support no other extension state at this time. */
> +	for (i = 0; i < ARRAY_SIZE(sc->sc_fpregs.q.reserved); i++)
> +		err |= __put_user(0, &sc->sc_fpregs.q.reserved[i]);
>  	return err;

And one before the return here would not go amiss. Those are nitpick
bits though, so:
Acked-by: Conor Dooley <conor.dooley@microchip.com>

Cheers,
Conor.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/kvm-riscv/attachments/20230301/8f551d7f/attachment-0001.sig>

WARNING: multiple messages have this Message-ID (diff)
From: Conor Dooley <conor@kernel.org>
To: Andy Chiu <andy.chiu@sifive.com>
Cc: linux-riscv@lists.infradead.org, palmer@dabbelt.com,
	anup@brainfault.org, atishp@atishpatra.org,
	kvm-riscv@lists.infradead.org, kvm@vger.kernel.org,
	vineetg@rivosinc.com, greentime.hu@sifive.com,
	guoren@linux.alibaba.com,
	Paul Walmsley <paul.walmsley@sifive.com>,
	Albert Ou <aou@eecs.berkeley.edu>,
	Vincent Chen <vincent.chen@sifive.com>,
	Guo Ren <guoren@kernel.org>,
	Conor Dooley <conor.dooley@microchip.com>,
	Andrew Bresticker <abrestic@rivosinc.com>
Subject: Re: [PATCH -next v14 12/19] riscv: signal: check fp-reserved words unconditionally
Date: Wed, 1 Mar 2023 17:56:56 +0000	[thread overview]
Message-ID: <Y/+R6NfBHSGGI3MX@spud> (raw)
In-Reply-To: <20230224170118.16766-13-andy.chiu@sifive.com>


[-- Attachment #1.1: Type: text/plain, Size: 2328 bytes --]

On Fri, Feb 24, 2023 at 05:01:11PM +0000, Andy Chiu wrote:
> In order to let kernel/user locate and identify an extension context on
> the existing sigframe, we are going to utilize reserved space of fp and
> encode the information there. And since the sigcontext has already
> preserved a space for fp context w or w/o CONFIG_FPU, we move those
> reserved words checking/setting routine back into generic code.
> 
> This commit also undone an additional logical change carried by the
> refactor commit 007f5c3589578
> ("Refactor FPU code in signal setup/return procedures"). Originally we
> did not restore fp context if restoring of gpr have failed. And it was
> fine on the other side. In such way the kernel could keep the regfiles
> intact, and potentially react at the failing point of restore.
> 
> Signed-off-by: Andy Chiu <andy.chiu@sifive.com>
> ---

> @@ -90,11 +66,29 @@ static long restore_sigcontext(struct pt_regs *regs,
>  	struct sigcontext __user *sc)
>  {
>  	long err;
> +	size_t i;
> +
>  	/* sc_regs is structured the same as the start of pt_regs */
>  	err = __copy_from_user(regs, &sc->sc_regs, sizeof(sc->sc_regs));
> +	if (unlikely(err))
> +		return err;
>  	/* Restore the floating-point state. */

Please preserve the newline after return from where you moved this from.

> -	if (has_fpu())
> -		err |= restore_fp_state(regs, &sc->sc_fpregs);
> +	if (has_fpu()) {
> +		err = restore_fp_state(regs, &sc->sc_fpregs);
> +		if (unlikely(err))
> +			return err;
> +	}

> @@ -145,11 +139,16 @@ static long setup_sigcontext(struct rt_sigframe __user *frame,
>  {
>  	struct sigcontext __user *sc = &frame->uc.uc_mcontext;
>  	long err;
> +	size_t i;
> +
>  	/* sc_regs is structured the same as the start of pt_regs */
>  	err = __copy_to_user(&sc->sc_regs, regs, sizeof(sc->sc_regs));
>  	/* Save the floating-point state. */
>  	if (has_fpu())
>  		err |= save_fp_state(regs, &sc->sc_fpregs);
> +	/* We support no other extension state at this time. */
> +	for (i = 0; i < ARRAY_SIZE(sc->sc_fpregs.q.reserved); i++)
> +		err |= __put_user(0, &sc->sc_fpregs.q.reserved[i]);
>  	return err;

And one before the return here would not go amiss. Those are nitpick
bits though, so:
Acked-by: Conor Dooley <conor.dooley@microchip.com>

Cheers,
Conor.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

WARNING: multiple messages have this Message-ID (diff)
From: Conor Dooley <conor@kernel.org>
To: Andy Chiu <andy.chiu@sifive.com>
Cc: linux-riscv@lists.infradead.org, palmer@dabbelt.com,
	anup@brainfault.org, atishp@atishpatra.org,
	kvm-riscv@lists.infradead.org, kvm@vger.kernel.org,
	vineetg@rivosinc.com, greentime.hu@sifive.com,
	guoren@linux.alibaba.com,
	Paul Walmsley <paul.walmsley@sifive.com>,
	Albert Ou <aou@eecs.berkeley.edu>,
	Vincent Chen <vincent.chen@sifive.com>,
	Guo Ren <guoren@kernel.org>,
	Conor Dooley <conor.dooley@microchip.com>,
	Andrew Bresticker <abrestic@rivosinc.com>
Subject: Re: [PATCH -next v14 12/19] riscv: signal: check fp-reserved words unconditionally
Date: Wed, 1 Mar 2023 17:56:56 +0000	[thread overview]
Message-ID: <Y/+R6NfBHSGGI3MX@spud> (raw)
In-Reply-To: <20230224170118.16766-13-andy.chiu@sifive.com>

[-- Attachment #1: Type: text/plain, Size: 2328 bytes --]

On Fri, Feb 24, 2023 at 05:01:11PM +0000, Andy Chiu wrote:
> In order to let kernel/user locate and identify an extension context on
> the existing sigframe, we are going to utilize reserved space of fp and
> encode the information there. And since the sigcontext has already
> preserved a space for fp context w or w/o CONFIG_FPU, we move those
> reserved words checking/setting routine back into generic code.
> 
> This commit also undone an additional logical change carried by the
> refactor commit 007f5c3589578
> ("Refactor FPU code in signal setup/return procedures"). Originally we
> did not restore fp context if restoring of gpr have failed. And it was
> fine on the other side. In such way the kernel could keep the regfiles
> intact, and potentially react at the failing point of restore.
> 
> Signed-off-by: Andy Chiu <andy.chiu@sifive.com>
> ---

> @@ -90,11 +66,29 @@ static long restore_sigcontext(struct pt_regs *regs,
>  	struct sigcontext __user *sc)
>  {
>  	long err;
> +	size_t i;
> +
>  	/* sc_regs is structured the same as the start of pt_regs */
>  	err = __copy_from_user(regs, &sc->sc_regs, sizeof(sc->sc_regs));
> +	if (unlikely(err))
> +		return err;
>  	/* Restore the floating-point state. */

Please preserve the newline after return from where you moved this from.

> -	if (has_fpu())
> -		err |= restore_fp_state(regs, &sc->sc_fpregs);
> +	if (has_fpu()) {
> +		err = restore_fp_state(regs, &sc->sc_fpregs);
> +		if (unlikely(err))
> +			return err;
> +	}

> @@ -145,11 +139,16 @@ static long setup_sigcontext(struct rt_sigframe __user *frame,
>  {
>  	struct sigcontext __user *sc = &frame->uc.uc_mcontext;
>  	long err;
> +	size_t i;
> +
>  	/* sc_regs is structured the same as the start of pt_regs */
>  	err = __copy_to_user(&sc->sc_regs, regs, sizeof(sc->sc_regs));
>  	/* Save the floating-point state. */
>  	if (has_fpu())
>  		err |= save_fp_state(regs, &sc->sc_fpregs);
> +	/* We support no other extension state at this time. */
> +	for (i = 0; i < ARRAY_SIZE(sc->sc_fpregs.q.reserved); i++)
> +		err |= __put_user(0, &sc->sc_fpregs.q.reserved[i]);
>  	return err;

And one before the return here would not go amiss. Those are nitpick
bits though, so:
Acked-by: Conor Dooley <conor.dooley@microchip.com>

Cheers,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

  reply	other threads:[~2023-03-01 17:56 UTC|newest]

Thread overview: 162+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-24 17:00 [PATCH -next v14 00/19] riscv: Add vector ISA support Andy Chiu
2023-02-24 17:00 ` Andy Chiu
2023-02-24 17:00 ` Andy Chiu
2023-02-24 17:01 ` [PATCH -next v14 01/19] riscv: Rename __switch_to_aux -> fpu Andy Chiu
2023-02-24 17:01   ` Andy Chiu
2023-02-24 17:01   ` Andy Chiu
2023-02-28 21:56   ` Conor Dooley
2023-02-28 21:56     ` Conor Dooley
2023-02-28 21:56     ` Conor Dooley
2023-02-24 17:01 ` [PATCH -next v14 02/19] riscv: Extending cpufeature.c to detect V-extension Andy Chiu
2023-02-24 17:01   ` Andy Chiu
2023-02-24 17:01   ` Andy Chiu
2023-02-28 22:07   ` Conor Dooley
2023-02-28 22:07     ` Conor Dooley
2023-02-28 22:07     ` Conor Dooley
2023-02-24 17:01 ` [PATCH -next v14 03/19] riscv: Add new csr defines related to vector extension Andy Chiu
2023-02-24 17:01   ` Andy Chiu
2023-02-24 17:01   ` Andy Chiu
2023-02-28 22:31   ` Conor Dooley
2023-02-28 22:31     ` Conor Dooley
2023-02-28 22:31     ` Conor Dooley
2023-02-24 17:01 ` [PATCH -next v14 04/19] riscv: Clear vector regfile on bootup Andy Chiu
2023-02-24 17:01   ` Andy Chiu
2023-02-24 17:01   ` Andy Chiu
2023-02-28 22:17   ` Conor Dooley
2023-02-28 22:17     ` Conor Dooley
2023-02-28 22:17     ` Conor Dooley
2023-02-24 17:01 ` [PATCH -next v14 05/19] riscv: Disable Vector Instructions for kernel itself Andy Chiu
2023-02-24 17:01   ` Andy Chiu
2023-02-24 17:01   ` Andy Chiu
2023-02-24 17:01 ` [PATCH -next v14 06/19] riscv: Introduce Vector enable/disable helpers Andy Chiu
2023-02-24 17:01   ` Andy Chiu
2023-02-24 17:01   ` Andy Chiu
2023-02-28 22:36   ` Conor Dooley
2023-02-28 22:36     ` Conor Dooley
2023-02-28 22:36     ` Conor Dooley
2023-02-24 17:01 ` [PATCH -next v14 07/19] riscv: Introduce riscv_v_vsize to record size of Vector context Andy Chiu
2023-02-24 17:01   ` Andy Chiu
2023-02-24 17:01   ` Andy Chiu
2023-02-28 22:38   ` Conor Dooley
2023-02-28 22:39     ` Conor Dooley
2023-02-28 22:38     ` Conor Dooley
2023-02-24 17:01 ` [PATCH -next v14 08/19] riscv: Introduce struct/helpers to save/restore per-task Vector state Andy Chiu
2023-02-24 17:01   ` Andy Chiu
2023-02-24 17:01   ` Andy Chiu
2023-02-28 23:00   ` Conor Dooley
2023-02-28 23:00     ` Conor Dooley
2023-02-28 23:00     ` Conor Dooley
2023-03-15  4:00     ` Andy Chiu
2023-03-15  4:00       ` Andy Chiu
2023-03-15  4:00       ` Andy Chiu
2023-03-02 11:12   ` Björn Töpel
2023-03-02 11:12     ` Björn Töpel
2023-03-02 11:12     ` Björn Töpel
2023-03-15  4:05     ` Andy Chiu
2023-03-15  4:05       ` Andy Chiu
2023-03-15  4:05       ` Andy Chiu
2023-02-24 17:01 ` [PATCH -next v14 09/19] riscv: Add task switch support for vector Andy Chiu
2023-02-24 17:01   ` Andy Chiu
2023-02-24 17:01   ` Andy Chiu
2023-03-01 16:41   ` Conor Dooley
2023-03-01 16:41     ` Conor Dooley
2023-03-01 16:41     ` Conor Dooley
2023-03-01 16:57     ` Björn Töpel
2023-03-01 16:57       ` Björn Töpel
2023-03-01 16:57       ` Björn Töpel
2023-03-02 11:07   ` Björn Töpel
2023-03-02 11:07     ` Björn Töpel
2023-03-02 11:07     ` Björn Töpel
2023-02-24 17:01 ` [PATCH -next v14 10/19] riscv: Allocate user's vector context in the first-use trap Andy Chiu
2023-02-24 17:01   ` Andy Chiu
2023-02-24 17:01   ` Andy Chiu
2023-03-01 16:53   ` Conor Dooley
2023-03-01 16:53     ` Conor Dooley
2023-03-01 16:53     ` Conor Dooley
2023-02-24 17:01 ` [PATCH -next v14 11/19] riscv: Add ptrace vector support Andy Chiu
2023-02-24 17:01   ` Andy Chiu
2023-02-24 17:01   ` Andy Chiu
2023-03-01 17:29   ` Conor Dooley
2023-03-01 17:29     ` Conor Dooley
2023-03-01 17:29     ` Conor Dooley
2023-03-02 11:27   ` Björn Töpel
2023-03-02 11:27     ` Björn Töpel
2023-03-02 11:27     ` Björn Töpel
2023-03-14 10:39     ` Andy Chiu
2023-03-14 10:39       ` Andy Chiu
2023-03-14 10:39       ` Andy Chiu
2023-03-14 10:44       ` Conor Dooley
2023-03-14 10:44         ` Conor Dooley
2023-03-14 10:44         ` Conor Dooley
2023-02-24 17:01 ` [PATCH -next v14 12/19] riscv: signal: check fp-reserved words unconditionally Andy Chiu
2023-02-24 17:01   ` Andy Chiu
2023-02-24 17:01   ` Andy Chiu
2023-03-01 17:56   ` Conor Dooley [this message]
2023-03-01 17:56     ` Conor Dooley
2023-03-01 17:56     ` Conor Dooley
2023-02-24 17:01 ` [PATCH -next v14 13/19] riscv: signal: Add sigcontext save/restore for vector Andy Chiu
2023-02-24 17:01   ` Andy Chiu
2023-02-24 17:01   ` Andy Chiu
2023-03-01 18:27   ` Conor Dooley
2023-03-01 18:27     ` Conor Dooley
2023-03-01 18:27     ` Conor Dooley
2023-03-02 12:42   ` Björn Töpel
2023-03-02 12:42     ` Björn Töpel
2023-03-02 12:42     ` Björn Töpel
2023-02-24 17:01 ` [PATCH -next v14 14/19] riscv: signal: Report signal frame size to userspace via auxv Andy Chiu
2023-02-24 17:01   ` Andy Chiu
2023-02-24 17:01   ` Andy Chiu
2023-03-01 19:21   ` Conor Dooley
2023-03-01 19:21     ` Conor Dooley
2023-03-01 19:21     ` Conor Dooley
2023-03-02 12:47   ` Björn Töpel
2023-03-02 12:47     ` Björn Töpel
2023-03-02 12:47     ` Björn Töpel
2023-02-24 17:01 ` [PATCH -next v14 15/19] riscv: signal: validate altstack to reflect Vector Andy Chiu
2023-02-24 17:01   ` Andy Chiu
2023-02-24 17:01   ` Andy Chiu
2023-03-01 21:00   ` Conor Dooley
2023-03-01 21:00     ` Conor Dooley
2023-03-01 21:00     ` Conor Dooley
2023-02-24 17:01 ` [PATCH -next v14 16/19] riscv: prevent stack corruption by reserving task_pt_regs(p) early Andy Chiu
2023-02-24 17:01   ` Andy Chiu
2023-02-24 17:01   ` Andy Chiu
2023-03-01 21:34   ` Conor Dooley
2023-03-01 21:34     ` Conor Dooley
2023-03-01 21:34     ` Conor Dooley
2023-02-24 17:01 ` [PATCH -next v14 17/19] riscv: kvm: Add V extension to KVM ISA Andy Chiu
2023-02-24 17:01   ` Andy Chiu
2023-02-24 17:01   ` Andy Chiu
2023-03-01 21:38   ` Conor Dooley
2023-03-01 21:38     ` Conor Dooley
2023-03-01 21:38     ` Conor Dooley
2023-02-24 17:01 ` [PATCH -next v14 18/19] riscv: KVM: Add vector lazy save/restore support Andy Chiu
2023-02-24 17:01   ` Andy Chiu
2023-02-24 17:01   ` Andy Chiu
2023-02-24 17:01 ` [PATCH -next v14 19/19] riscv: Enable Vector code to be built Andy Chiu
2023-02-24 17:01   ` Andy Chiu
2023-02-24 17:01   ` Andy Chiu
2023-02-24 21:35   ` kernel test robot
2023-02-24 21:35     ` kernel test robot
2023-02-24 21:35     ` kernel test robot
2023-02-25  1:33   ` kernel test robot
2023-02-25  1:34     ` kernel test robot
2023-02-25  1:33     ` kernel test robot
2023-03-01 18:00     ` Nathan Chancellor
2023-03-01 18:00       ` Nathan Chancellor
2023-03-01 18:00       ` Nathan Chancellor
2023-03-01 18:44       ` Conor Dooley
2023-03-01 18:44         ` Conor Dooley
2023-03-01 18:44         ` Conor Dooley
2023-02-25  8:28   ` kernel test robot
2023-02-25  8:28     ` kernel test robot
2023-02-25  8:28     ` kernel test robot
2023-02-27 10:18   ` Conor Dooley
2023-02-27 10:19     ` Conor Dooley
2023-02-27 10:18     ` Conor Dooley
2023-02-27 13:40     ` Darius Rad
2023-02-27 13:40       ` Darius Rad
2023-02-27 13:40       ` Darius Rad
2023-02-27 13:58       ` Conor Dooley
2023-02-27 13:59         ` Conor Dooley
2023-02-27 13:58         ` Conor Dooley

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=Y/+R6NfBHSGGI3MX@spud \
    --to=conor@kernel.org \
    --cc=kvm-riscv@lists.infradead.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: link
Be 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.