[PATCH] MIPS: KVM: do not sign extend on unsigned MMIO load

[PATCH] MIPS: KVM: do not sign extend on unsigned MMIO load

[Nicholas Mc Guire]

Fix possible unintended sign extension in unsigned MMIO loads by casting
to uint16_t in the case of mmio_needed != 2.

Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org>
---

Thanks to James Hogan <james.hogan@imgtec.com> for the explaination of 
mmio_needed (there is not really any helpful comment in the code on this)
in this case (mmio_needed!=2) it should be unsigned.

Patch was only compile tested msp71xx_defconfig + CONFIG_KVM=m

Patch is against 4.1-rc2 (localversion-next is -next-20150506)

 arch/mips/kvm/emulate.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/mips/kvm/emulate.c b/arch/mips/kvm/emulate.c
index 6230f37..2f0fc60 100644
--- a/arch/mips/kvm/emulate.c
+++ b/arch/mips/kvm/emulate.c
@@ -2415,7 +2415,7 @@ enum emulation_result kvm_mips_complete_mmio_load(struct kvm_vcpu *vcpu,
 		if (vcpu->mmio_needed == 2)
 			*gpr = *(int16_t *) run->mmio.data;
 		else
-			*gpr = *(int16_t *) run->mmio.data;
+			*gpr = *(uint16_t *)run->mmio.data;
 
 		break;
 	case 1:
-- 
1.7.10.4

Re: [PATCH] MIPS: KVM: do not sign extend on unsigned MMIO load

[James Hogan]

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

On 07/05/15 13:47, Nicholas Mc Guire wrote:
> Fix possible unintended sign extension in unsigned MMIO loads by casting
> to uint16_t in the case of mmio_needed != 2.
> 
> Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org>

Looks good to me. I wrote an MMIO test to reproduce the issue, and this
fixes it.

Reviewed-by: James Hogan <james.hogan@imgtec.com>
Tested-by: James Hogan <james.hogan@imgtec.com>

It looks suitable for stable too (3.10+).

Cheers
James

> ---
> 
> Thanks to James Hogan <james.hogan@imgtec.com> for the explaination of 
> mmio_needed (there is not really any helpful comment in the code on this)
> in this case (mmio_needed!=2) it should be unsigned.
> 
> Patch was only compile tested msp71xx_defconfig + CONFIG_KVM=m
> 
> Patch is against 4.1-rc2 (localversion-next is -next-20150506)
> 
>  arch/mips/kvm/emulate.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/mips/kvm/emulate.c b/arch/mips/kvm/emulate.c
> index 6230f37..2f0fc60 100644
> --- a/arch/mips/kvm/emulate.c
> +++ b/arch/mips/kvm/emulate.c
> @@ -2415,7 +2415,7 @@ enum emulation_result kvm_mips_complete_mmio_load(struct kvm_vcpu *vcpu,
>  		if (vcpu->mmio_needed == 2)
>  			*gpr = *(int16_t *) run->mmio.data;
>  		else
> -			*gpr = *(int16_t *) run->mmio.data;
> +			*gpr = *(uint16_t *)run->mmio.data;
>  
>  		break;
>  	case 1:
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] MIPS: KVM: do not sign extend on unsigned MMIO load

[James Hogan]

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

Hi stable folk,

On 08/05/15 15:16, James Hogan wrote:
> On 07/05/15 13:47, Nicholas Mc Guire wrote:
>> Fix possible unintended sign extension in unsigned MMIO loads by casting
>> to uint16_t in the case of mmio_needed != 2.
>>
>> Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org>
> 
> Looks good to me. I wrote an MMIO test to reproduce the issue, and this
> fixes it.
> 
> Reviewed-by: James Hogan <james.hogan@imgtec.com>
> Tested-by: James Hogan <james.hogan@imgtec.com>
> 
> It looks suitable for stable too (3.10+).

This has reached mainline, commit ed9244e6c534612d2b5ae47feab2f55a0d4b4ced

Please could it be added to stable (3.10+).

Thanks
James


> 
> Cheers
> James
> 
>> ---
>>
>> Thanks to James Hogan <james.hogan@imgtec.com> for the explaination of 
>> mmio_needed (there is not really any helpful comment in the code on this)
>> in this case (mmio_needed!=2) it should be unsigned.
>>
>> Patch was only compile tested msp71xx_defconfig + CONFIG_KVM=m
>>
>> Patch is against 4.1-rc2 (localversion-next is -next-20150506)
>>
>>  arch/mips/kvm/emulate.c |    2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/mips/kvm/emulate.c b/arch/mips/kvm/emulate.c
>> index 6230f37..2f0fc60 100644
>> --- a/arch/mips/kvm/emulate.c
>> +++ b/arch/mips/kvm/emulate.c
>> @@ -2415,7 +2415,7 @@ enum emulation_result kvm_mips_complete_mmio_load(struct kvm_vcpu *vcpu,
>>  		if (vcpu->mmio_needed == 2)
>>  			*gpr = *(int16_t *) run->mmio.data;
>>  		else
>> -			*gpr = *(int16_t *) run->mmio.data;
>> +			*gpr = *(uint16_t *)run->mmio.data;
>>  
>>  		break;
>>  	case 1:
>>
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] MIPS: KVM: do not sign extend on unsigned MMIO load

[Jiri Slaby]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

On 06/08/2015, 10:33 AM, James Hogan wrote:
> Hi stable folk,
> 
> On 08/05/15 15:16, James Hogan wrote:
>> On 07/05/15 13:47, Nicholas Mc Guire wrote:
>>> Fix possible unintended sign extension in unsigned MMIO loads
>>> by casting to uint16_t in the case of mmio_needed != 2.
>>> 
>>> Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org>
>> 
>> Looks good to me. I wrote an MMIO test to reproduce the issue,
>> and this fixes it.
>> 
>> Reviewed-by: James Hogan <james.hogan@imgtec.com> Tested-by:
>> James Hogan <james.hogan@imgtec.com>
>> 
>> It looks suitable for stable too (3.10+).
> 
> This has reached mainline, commit
> ed9244e6c534612d2b5ae47feab2f55a0d4b4ced
> 
> Please could it be added to stable (3.10+).

Applied to 3.12. Thanks.

- -- 
js
suse labs
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBCAAGBQJVeDQcAAoJEL0lsQQGtHBJ7q4P/3Q7y1FHwPKDhsdIdyyRypR6
OXaH/6eNzpBhvSngP1gnx9MiyESTYihFVlRJsV6hYYzRcippnU0BP88dx9ntYrc1
Accbhj/PPYcMqfCnYdL80Kxt9EomeuxEDcCdbp8twnReTt44xNAGHePiNh9GrhjG
VKBMralyyjymtwyamCGb2W2aLNhxELIG3gXJTb7Q7E071LVeqQA6g+VNQ2QHwFYq
FkJexePsLu/j3zVxH+rsQPJA6E1oKfUJb3jQHAtZHAH95Um0r8T4jUVSgFhyk3r6
9tlkazL3P8Iui6lxbrV1vNCPAhhucY7PmX99uGhdroKJOKDCDPsVOKyJbxeHrUBR
3zrMpB9x2uXd6WpDLDfL+bI8bCG6NVXZPGgSd7P+r/UbNuZ6VBNSVdqlWUeoMWGR
ZS+HFMxVOiNplYudCTdLbJDLhm2XCWeW2lqszll/8Nk1c1FZkl8YbgpmdXfutKeU
LQfUfS4tr0AQ7BqXf3bPUGrSGZO7e1V5R4gAa+Yqo6ZjDOj20AjYvs0oW4ubgLg8
OJrHcJDLkEKrMDIZ7qpRZxyz56yrOgcfVbYB1fudXaV+e38t+kO0sujdNSJnHK8h
T3kfa96QW2gOi7Cys1o2OaQboY2wFxK3/YefX3Jn+N7tGedKUwF4IYHtj17YqX1/
8BkHSZZ9HQsJqyRAXBux
=qlvA
-----END PGP SIGNATURE-----

Re: [PATCH] MIPS: KVM: do not sign extend on unsigned MMIO load

[Luis Henriques]

On Mon, Jun 08, 2015 at 09:33:50AM +0100, James Hogan wrote:
> Hi stable folk,
> 
> On 08/05/15 15:16, James Hogan wrote:
> > On 07/05/15 13:47, Nicholas Mc Guire wrote:
> >> Fix possible unintended sign extension in unsigned MMIO loads by casting
> >> to uint16_t in the case of mmio_needed != 2.
> >>
> >> Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org>
> > 
> > Looks good to me. I wrote an MMIO test to reproduce the issue, and this
> > fixes it.
> > 
> > Reviewed-by: James Hogan <james.hogan@imgtec.com>
> > Tested-by: James Hogan <james.hogan@imgtec.com>
> > 
> > It looks suitable for stable too (3.10+).
> 
> This has reached mainline, commit ed9244e6c534612d2b5ae47feab2f55a0d4b4ced
> 
> Please could it be added to stable (3.10+).
> 
> Thanks
> James

Thanks, I'm queuing it for the 3.16 as well.

Cheers,
--
Luís

Re: [PATCH] MIPS: KVM: do not sign extend on unsigned MMIO load

[Greg KH]

On Mon, Jun 08, 2015 at 09:33:50AM +0100, James Hogan wrote:
> Hi stable folk,
> 
> On 08/05/15 15:16, James Hogan wrote:
> > On 07/05/15 13:47, Nicholas Mc Guire wrote:
> >> Fix possible unintended sign extension in unsigned MMIO loads by casting
> >> to uint16_t in the case of mmio_needed != 2.
> >>
> >> Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org>
> > 
> > Looks good to me. I wrote an MMIO test to reproduce the issue, and this
> > fixes it.
> > 
> > Reviewed-by: James Hogan <james.hogan@imgtec.com>
> > Tested-by: James Hogan <james.hogan@imgtec.com>
> > 
> > It looks suitable for stable too (3.10+).
> 
> This has reached mainline, commit ed9244e6c534612d2b5ae47feab2f55a0d4b4ced
> 
> Please could it be added to stable (3.10+).

It does not apply to 3.10 or 3.14-stable, so please provide a backport
if you want it there.

thanks,

greg k-h

[PATCH stable <3.17] MIPS: KVM: Do not sign extend on unsigned MMIO load

[James Hogan]

From: Nicholas Mc Guire <hofrat@osadl.org>

commit ed9244e6c534612d2b5ae47feab2f55a0d4b4ced upstream.

Fix possible unintended sign extension in unsigned MMIO loads by casting
to uint16_t in the case of mmio_needed != 2.

Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org>
Reviewed-by: James Hogan <james.hogan@imgtec.com>
Tested-by: James Hogan <james.hogan@imgtec.com>
Cc: Gleb Natapov <gleb@kernel.org>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: kvm@vger.kernel.org
Cc: linux-mips@linux-mips.org
Cc: linux-kernel@vger.kernel.org
Patchwork: https://patchwork.linux-mips.org/patch/9985/
Signed-off-by: Ralf Baechle <ralf@linux-mips.org>
Cc: <stable@vger.kernel.org> # v3.10+
Signed-off-by: James Hogan <james.hogan@imgtec.com>
---
This is a trivial backport (i.e. git cherry-pick, git format-patch) for
stable branches before v3.17, due to the commit d7d5b05faf16 ("MIPS:
KVM: Rename files to remove the prefix "kvm_" and "kvm_mips_"") which
renamed a bunch of files including this one.
---
 arch/mips/kvm/kvm_mips_emul.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/mips/kvm/kvm_mips_emul.c b/arch/mips/kvm/kvm_mips_emul.c
index e75ef8219caf..c76f297b7149 100644
--- a/arch/mips/kvm/kvm_mips_emul.c
+++ b/arch/mips/kvm/kvm_mips_emul.c
@@ -1626,7 +1626,7 @@ kvm_mips_complete_mmio_load(struct kvm_vcpu *vcpu, struct kvm_run *run)
 		if (vcpu->mmio_needed == 2)
 			*gpr = *(int16_t *) run->mmio.data;
 		else
-			*gpr = *(int16_t *) run->mmio.data;
+			*gpr = *(uint16_t *)run->mmio.data;
 
 		break;
 	case 1:
-- 
2.3.6