linux-kernel - Re: [PATCH v9 24/27] virt: gunyah: Add proxy-scheduled vCPUs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 10 Feb 2023 09:09:46 -0800
From:   Elliot Berman <quic_eberman@...cinc.com>
To:     Srivatsa Vaddagiri <quic_svaddagi@...cinc.com>
CC:     Bjorn Andersson <quic_bjorande@...cinc.com>,
        Alex Elder <elder@...aro.org>,
        Murali Nalajala <quic_mnalajal@...cinc.com>,
        "Jonathan Corbet" <corbet@....net>,
        Catalin Marinas <catalin.marinas@....com>,
        "Will Deacon" <will@...nel.org>,
        Trilok Soni <quic_tsoni@...cinc.com>,
        "Carl van Schaik" <quic_cvanscha@...cinc.com>,
        Prakruthi Deepak Heragu <quic_pheragu@...cinc.com>,
        Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
        Arnd Bergmann <arnd@...db.de>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Rob Herring <robh+dt@...nel.org>,
        "Krzysztof Kozlowski" <krzysztof.kozlowski+dt@...aro.org>,
        Bagas Sanjaya <bagasdotme@...il.com>,
        Marc Zyngier <maz@...nel.org>,
        Jassi Brar <jassisinghbrar@...il.com>,
        Sudeep Holla <sudeep.holla@....com>,
        <linux-arm-msm@...r.kernel.org>, <devicetree@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, <linux-doc@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v9 24/27] virt: gunyah: Add proxy-scheduled vCPUs



On 2/9/2023 10:54 PM, Srivatsa Vaddagiri wrote:
> * Srivatsa Vaddagiri <quic_svaddagi@...cinc.com> [2023-02-09 16:10:06]:
> 
>> * Elliot Berman <quic_eberman@...cinc.com> [2023-01-20 14:46:23]:
>>
>>> +static void gh_handle_mmio_return(struct gunyah_vcpu *vcpu, u64 *state)
>>> +{
>>> +	if (!vcpu->vcpu_run->mmio.is_write)
>>> +		memcpy(&state[0], vcpu->vcpu_run->mmio.data, vcpu->vcpu_run->mmio.len);
>>
>> Would be good to do a bound check on length of memcpy I think (in case
>> vcpu_run_resp->state_data[1] is wrong for example).

I think this is the right place to ensure right value is passed. Right 
now, vcpu_run->mmio.len is user accessible and easily provides ability 
for buffer overrun if unchecked. I'm now preserving the length Gunyah 
requested in internal vcpu structure. Userspace can write whatever they 
want to the mmio request, but it won't change the number of bytes that 
we copy from user.

> 
> Sorry I meant to comment that for the other memcpy you had!
> 
> +               memcpy(vcpu->vcpu_run->mmio.data, &vcpu_run_resp->state_data[2],
> +                       vcpu_run_resp->state_data[1]);
> 
> 

Per Srini and Alex, we can trust the values hypervisor sends, so I 
think we can skip this check.

https://lore.kernel.org/all/4a584563-1fb7-22fa-5e16-e0cf5e88b76b@linaro.org/

Thanks,
Elliot