[PATCH] MIPS: prctl: Disallow FRE without FR with PR_SET_FP_MODE requests

[PATCH] MIPS: prctl: Disallow FRE without FR with PR_SET_FP_MODE requests

[Maciej W. Rozycki]

Having PR_FP_MODE_FRE (i.e. Config5.FRE) set without PR_FP_MODE_FR (i.e. 
Status.FR) is not supported as the lone purpose of Config5.FRE is to 
emulate Status.FR=0 handling on FPU hardware that has Status.FR=1 
hardwired[1][2].  Also we do not handle this case elsewhere, and assume 
throughout our code that TIF_HYBRID_FPREGS and TIF_32BIT_FPREGS cannot 
be set both at once for a task, leading to inconsistent behaviour if 
this does happen.

Return unsuccessfully then from prctl(2) PR_SET_FP_MODE calls requesting 
PR_FP_MODE_FRE to be set with PR_FP_MODE_FR clear.  This corresponds to 
modes allowed by `mips_set_personality_fp'.

References:

[1] "MIPS Architecture For Programmers, Vol. III: MIPS32 / microMIPS32
    Privileged Resource Architecture", Imagination Technologies,
    Document Number: MD00090, Revision 6.02, July 10, 2015, Table 9.69 
    "Config5 Register Field Descriptions", p. 262

[2] "MIPS Architecture For Programmers, Volume III: MIPS64 / microMIPS64 
    Privileged Resource Architecture", Imagination Technologies, 
    Document Number: MD00091, Revision 6.03, December 22, 2015, Table 
    9.72 "Config5 Register Field Descriptions", p. 288

Cc: stable@vger.kernel.org # 4.0+
Fixes: 9791554b45a2 ("MIPS,prctl: add PR_[GS]ET_FP_MODE prctl options for MIPS")
Signed-off-by: Maciej W. Rozycki <macro@mips.com>
---
 arch/mips/kernel/process.c |    4 ++++
 1 file changed, 4 insertions(+)

linux-mips-set-process-fp-mode-fr-fre.diff
Index: linux/arch/mips/kernel/process.c
===================================================================
--- linux.orig/arch/mips/kernel/process.c	2018-05-12 22:52:11.000000000 +0100
+++ linux/arch/mips/kernel/process.c	2018-05-12 23:07:15.147112000 +0100
@@ -721,6 +721,10 @@ int mips_set_process_fp_mode(struct task
 	if (value & ~known_bits)
 		return -EOPNOTSUPP;
 
+	/* Setting FRE without FR is not supported.  */
+	if ((value & (PR_FP_MODE_FR | PR_FP_MODE_FRE)) == PR_FP_MODE_FRE)
+		return -EOPNOTSUPP;
+
 	/* Avoid inadvertently triggering emulation */
 	if ((value & PR_FP_MODE_FR) && raw_cpu_has_fpu &&
 	    !(raw_current_cpu_data.fpu_id & MIPS_FPIR_F64))

Re: [PATCH] MIPS: prctl: Disallow FRE without FR with PR_SET_FP_MODE requests

[Paul Burton]

Hi Maciej,

On Tue, May 15, 2018 at 11:04:44PM +0100, Maciej W. Rozycki wrote:
> Having PR_FP_MODE_FRE (i.e. Config5.FRE) set without PR_FP_MODE_FR (i.e. 
> Status.FR) is not supported as the lone purpose of Config5.FRE is to 
> emulate Status.FR=0 handling on FPU hardware that has Status.FR=1 
> hardwired[1][2].  Also we do not handle this case elsewhere, and assume 
> throughout our code that TIF_HYBRID_FPREGS and TIF_32BIT_FPREGS cannot 
> be set both at once for a task, leading to inconsistent behaviour if 
> this does happen.

Reviewing the code I think we should actually end up with FR=1 in this
case, because neither __own_fpu() nor the FPU emulator depend on the
value of TIF_32BIT_FPREGS if TIF_HYBRID_FPREGS is set. So it's not too
awful & I don't see the kernel doing anything too crazy, but it
definitely isn't what the user asked for.

> Return unsuccessfully then from prctl(2) PR_SET_FP_MODE calls requesting 
> PR_FP_MODE_FRE to be set with PR_FP_MODE_FR clear.  This corresponds to 
> modes allowed by `mips_set_personality_fp'.

Looks good to me:

  Reviewed-by: Paul Burton <paul.burton@mips.com>

Thanks,
  Paul

> References:
> 
> [1] "MIPS Architecture For Programmers, Vol. III: MIPS32 / microMIPS32
>     Privileged Resource Architecture", Imagination Technologies,
>     Document Number: MD00090, Revision 6.02, July 10, 2015, Table 9.69 
>     "Config5 Register Field Descriptions", p. 262
> 
> [2] "MIPS Architecture For Programmers, Volume III: MIPS64 / microMIPS64 
>     Privileged Resource Architecture", Imagination Technologies, 
>     Document Number: MD00091, Revision 6.03, December 22, 2015, Table 
>     9.72 "Config5 Register Field Descriptions", p. 288
> 
> Cc: stable@vger.kernel.org # 4.0+
> Fixes: 9791554b45a2 ("MIPS,prctl: add PR_[GS]ET_FP_MODE prctl options for MIPS")
> Signed-off-by: Maciej W. Rozycki <macro@mips.com>
> ---
>  arch/mips/kernel/process.c |    4 ++++
>  1 file changed, 4 insertions(+)
> 
> linux-mips-set-process-fp-mode-fr-fre.diff
> Index: linux/arch/mips/kernel/process.c
> ===================================================================
> --- linux.orig/arch/mips/kernel/process.c	2018-05-12 22:52:11.000000000 +0100
> +++ linux/arch/mips/kernel/process.c	2018-05-12 23:07:15.147112000 +0100
> @@ -721,6 +721,10 @@ int mips_set_process_fp_mode(struct task
>  	if (value & ~known_bits)
>  		return -EOPNOTSUPP;
>  
> +	/* Setting FRE without FR is not supported.  */
> +	if ((value & (PR_FP_MODE_FR | PR_FP_MODE_FRE)) == PR_FP_MODE_FRE)
> +		return -EOPNOTSUPP;
> +
>  	/* Avoid inadvertently triggering emulation */
>  	if ((value & PR_FP_MODE_FR) && raw_cpu_has_fpu &&
>  	    !(raw_current_cpu_data.fpu_id & MIPS_FPIR_F64))
> 

Re: [PATCH] MIPS: prctl: Disallow FRE without FR with PR_SET_FP_MODE requests

[Maciej W. Rozycki]

Hi Paul,

> > Having PR_FP_MODE_FRE (i.e. Config5.FRE) set without PR_FP_MODE_FR (i.e. 
> > Status.FR) is not supported as the lone purpose of Config5.FRE is to 
> > emulate Status.FR=0 handling on FPU hardware that has Status.FR=1 
> > hardwired[1][2].  Also we do not handle this case elsewhere, and assume 
> > throughout our code that TIF_HYBRID_FPREGS and TIF_32BIT_FPREGS cannot 
> > be set both at once for a task, leading to inconsistent behaviour if 
> > this does happen.
> 
> Reviewing the code I think we should actually end up with FR=1 in this
> case, because neither __own_fpu() nor the FPU emulator depend on the
> value of TIF_32BIT_FPREGS if TIF_HYBRID_FPREGS is set. So it's not too
> awful & I don't see the kernel doing anything too crazy, but it
> definitely isn't what the user asked for.

 However `arch_ptrace' does check TIF_32BIT_FPREGS and gets things wrong 
if TIF_HYBRID_FPREGS is also set.  Which is actually how I discovered 
this issue.

> > Return unsuccessfully then from prctl(2) PR_SET_FP_MODE calls requesting 
> > PR_FP_MODE_FRE to be set with PR_FP_MODE_FR clear.  This corresponds to 
> > modes allowed by `mips_set_personality_fp'.
> 
> Looks good to me:
> 
>   Reviewed-by: Paul Burton <paul.burton@mips.com>

 Thanks for your review.

  Maciej

Re: [PATCH] MIPS: prctl: Disallow FRE without FR with PR_SET_FP_MODE requests

[James Hogan]

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

On Tue, May 15, 2018 at 11:04:44PM +0100, Maciej W. Rozycki wrote:
> Having PR_FP_MODE_FRE (i.e. Config5.FRE) set without PR_FP_MODE_FR (i.e. 
> Status.FR) is not supported as the lone purpose of Config5.FRE is to 
> emulate Status.FR=0 handling on FPU hardware that has Status.FR=1 
> hardwired[1][2].  Also we do not handle this case elsewhere, and assume 
> throughout our code that TIF_HYBRID_FPREGS and TIF_32BIT_FPREGS cannot 
> be set both at once for a task, leading to inconsistent behaviour if 
> this does happen.
> 
> Return unsuccessfully then from prctl(2) PR_SET_FP_MODE calls requesting 
> PR_FP_MODE_FRE to be set with PR_FP_MODE_FR clear.  This corresponds to 
> modes allowed by `mips_set_personality_fp'.
> 
> References:
> 
> [1] "MIPS Architecture For Programmers, Vol. III: MIPS32 / microMIPS32
>     Privileged Resource Architecture", Imagination Technologies,
>     Document Number: MD00090, Revision 6.02, July 10, 2015, Table 9.69 
>     "Config5 Register Field Descriptions", p. 262
> 
> [2] "MIPS Architecture For Programmers, Volume III: MIPS64 / microMIPS64 
>     Privileged Resource Architecture", Imagination Technologies, 
>     Document Number: MD00091, Revision 6.03, December 22, 2015, Table 
>     9.72 "Config5 Register Field Descriptions", p. 288
> 
> Cc: stable@vger.kernel.org # 4.0+
> Fixes: 9791554b45a2 ("MIPS,prctl: add PR_[GS]ET_FP_MODE prctl options for MIPS")
> Signed-off-by: Maciej W. Rozycki <macro@mips.com>

Thanks, applied to mips-fixes, hopefully for 4.17 (but if it misses
tomorrows linux-next it may have to wait 'til 4.18).

Cheers
James

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