2023-11-02 18:17:10

by Philipp Stanner

[permalink] [raw]
Subject: [PATCH 0/3] Use new wrappers to copy userspace arrays

Hi,

Linus recently merged [1] the wrapper functions memdup_array_user() and
vmemdup_array_user() in include/linux/string.h for Kernel v6.7

I am currently adding them to all places where (v)memdup_user() had been
used to copy arrays.

The wrapper is different to the wrapped functions only in that it might
return -EOVERFLOW. So this new error code might get pushed up to
userspace. I hope this is fine.

I felt that it might be a good idea to land those three patches here
with a single series, since they all touch something KVM-related.

Kind regards,
P.

[1] https://lore.kernel.org/all/[email protected]/

Philipp Stanner (3):
arch/x86/kvm: copy user-array with overflow-check
arch/s390/kvm: copy userspace-array safely
virt/kvm: copy userspace-array safely

arch/s390/kvm/guestdbg.c | 4 ++--
arch/x86/kvm/cpuid.c | 4 ++--
virt/kvm/kvm_main.c | 5 ++---
3 files changed, 6 insertions(+), 7 deletions(-)

--
2.41.0


2023-11-02 18:17:27

by Philipp Stanner

[permalink] [raw]
Subject: [PATCH 3/3] virt/kvm: copy userspace-array safely

kvm_main.c utilizes vmemdup_user() and array_size() to copy a userspace
array. Currently, this does not check for an overflow.

Use the new wrapper vmemdup_array_user() to copy the array more safely.

Suggested-by: Dave Airlie <[email protected]>
Signed-off-by: Philipp Stanner <[email protected]>
---
virt/kvm/kvm_main.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 486800a7024b..2a2f409c2a7d 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -4932,9 +4932,8 @@ static long kvm_vm_ioctl(struct file *filp,
goto out;
if (routing.nr) {
urouting = argp;
- entries = vmemdup_user(urouting->entries,
- array_size(sizeof(*entries),
- routing.nr));
+ entries = vmemdup_array_user(urouting->entries,
+ routing.nr, sizeof(*entries));
if (IS_ERR(entries)) {
r = PTR_ERR(entries);
goto out;
--
2.41.0

2023-12-01 01:52:51

by Sean Christopherson

[permalink] [raw]
Subject: Re: [PATCH 0/3] Use new wrappers to copy userspace arrays

On Thu, 02 Nov 2023 19:15:23 +0100, Philipp Stanner wrote:
> Linus recently merged [1] the wrapper functions memdup_array_user() and
> vmemdup_array_user() in include/linux/string.h for Kernel v6.7
>
> I am currently adding them to all places where (v)memdup_user() had been
> used to copy arrays.
>
> The wrapper is different to the wrapped functions only in that it might
> return -EOVERFLOW. So this new error code might get pushed up to
> userspace. I hope this is fine.
>
> [...]

Applied to kvm-x86 generic. Claudio (or anyone else from s390), holler if
you want to take the s390 patch through the s390 tree.

I massaged the shortlogs to use KVM's standard scopes, and to make it clear
that these are hardening patches, i.e. that there is no unsafe/buggy behavior
that is being fixed. I also added a note at the end of each changelog to call
out that KVM pre-checks the sizes before copying, again to make it clear that
using the safer helper isn't expected to actually change KVM's behavior.

[1/3] KVM: x86: Harden copying of userspace-array against overflow
https://github.com/kvm-x86/linux/commit/573cc0e5cf14
[2/3] KVM: s390: Harden copying of userspace-array against overflow
https://github.com/kvm-x86/linux/commit/8b81a8d7c6b7
[3/3] KVM: Harden copying of userspace-array against overflow
https://github.com/kvm-x86/linux/commit/bc2cad56094c

--
https://github.com/kvm-x86/linux/tree/next

2023-12-01 11:25:53

by Christian Borntraeger

[permalink] [raw]
Subject: Re: [PATCH 0/3] Use new wrappers to copy userspace arrays



Am 01.12.23 um 02:52 schrieb Sean Christopherson:
> On Thu, 02 Nov 2023 19:15:23 +0100, Philipp Stanner wrote:
>> Linus recently merged [1] the wrapper functions memdup_array_user() and
>> vmemdup_array_user() in include/linux/string.h for Kernel v6.7
>>
>> I am currently adding them to all places where (v)memdup_user() had been
>> used to copy arrays.
>>
>> The wrapper is different to the wrapped functions only in that it might
>> return -EOVERFLOW. So this new error code might get pushed up to
>> userspace. I hope this is fine.
>>
>> [...]
>
> Applied to kvm-x86 generic. Claudio (or anyone else from s390), holler if
> you want to take the s390 patch through the s390 tree.

I think this is fine via your tree.

Feel free to add
Acked-by: Christian Borntraeger <[email protected]>
to patch 2 if the commit id is not yet final.

2023-12-01 16:02:54

by Sean Christopherson

[permalink] [raw]
Subject: Re: [PATCH 0/3] Use new wrappers to copy userspace arrays

On Fri, Dec 01, 2023, Christian Borntraeger wrote:
>
>
> Am 01.12.23 um 02:52 schrieb Sean Christopherson:
> > On Thu, 02 Nov 2023 19:15:23 +0100, Philipp Stanner wrote:
> > > Linus recently merged [1] the wrapper functions memdup_array_user() and
> > > vmemdup_array_user() in include/linux/string.h for Kernel v6.7
> > >
> > > I am currently adding them to all places where (v)memdup_user() had been
> > > used to copy arrays.
> > >
> > > The wrapper is different to the wrapped functions only in that it might
> > > return -EOVERFLOW. So this new error code might get pushed up to
> > > userspace. I hope this is fine.
> > >
> > > [...]
> >
> > Applied to kvm-x86 generic. Claudio (or anyone else from s390), holler if
> > you want to take the s390 patch through the s390 tree.
>
> I think this is fine via your tree.
>
> Feel free to add
> Acked-by: Christian Borntraeger <[email protected]>
> to patch 2 if the commit id is not yet final.

Done, thanks much! New hashes if anyone cares:

[1/3] KVM: x86: Harden copying of userspace-array against overflow
https://github.com/kvm-x86/linux/commit/573cc0e5cf14
[2/3] KVM: s390: Harden copying of userspace-array against overflow
https://github.com/kvm-x86/linux/commit/8c4976772d9b
[3/3] KVM: Harden copying of userspace-array against overflow
https://github.com/kvm-x86/linux/commit/1f829359c8c3