Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:5 as permitted sender) client-ip=2620:137:e000::3:5;
Date:   Mon, 6 Nov 2023 13:50:11 +0100
From:   Claudio Imbrenda <imbrenda@linux.ibm.com>
To:     Steffen Eiden <seiden@linux.ibm.com>
Cc:     linux-kernel@vger.kernel.org, linux-s390@vger.kernel.org,
        Janosch Frank <frankja@linux.ibm.com>
Subject: Re: [PATCH v1 1/1] s390/uvdevice: Report additional-data length for
 attestation
Message-ID: <20231106135011.07155487@p-imbrenda>
In-Reply-To: <20231106124922.3032370-1-seiden@linux.ibm.com>
References: <20231106124922.3032370-1-seiden@linux.ibm.com>
Organization: IBM
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Mon,  6 Nov 2023 13:49:22 +0100
Steffen Eiden <seiden@linux.ibm.com> wrote:

> Additional data length in the attestation request is an in/out variable.
> Software provides the capacity of the buffer. Upon successful request,
> firmware reports the actual bytes written to the additional data in that
> field. This information is lost, as the length field was not copied back
> to userspace before. Attestation might fail, if user space did not
> specify the exact amount of needed bytes required, as this length is
> part of the attestation measurement.
> 
> Signed-off-by: Steffen Eiden <seiden@linux.ibm.com>

Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>

> ---
>  drivers/s390/char/uvdevice.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/s390/char/uvdevice.c b/drivers/s390/char/uvdevice.c
> index 144cd2e03590..42c9f77f8da0 100644
> --- a/drivers/s390/char/uvdevice.c
> +++ b/drivers/s390/char/uvdevice.c
> @@ -109,6 +109,7 @@ static int uvio_copy_attest_result_to_user(struct uv_cb_attest *uvcb_attest,
>  					   struct uvio_attest *uvio_attest)
>  {
>  	struct uvio_attest __user *user_uvio_attest = (void __user *)uv_ioctl->argument_addr;
> +	u32 __user *user_buf_add_len = (u32 __user *)&user_uvio_attest->add_data_len;
>  	void __user *user_buf_add = (void __user *)uvio_attest->add_data_addr;
>  	void __user *user_buf_meas = (void __user *)uvio_attest->meas_addr;
>  	void __user *user_buf_uid = &user_uvio_attest->config_uid;
> @@ -117,6 +118,8 @@ static int uvio_copy_attest_result_to_user(struct uv_cb_attest *uvcb_attest,
>  		return -EFAULT;
>  	if (add_data && copy_to_user(user_buf_add, add_data, uvio_attest->add_data_len))
>  		return -EFAULT;
> +	if (put_user(uvio_attest->add_data_len, user_buf_add_len))
> +		return -EFAULT;
>  	if (copy_to_user(user_buf_uid, uvcb_attest->config_uid, sizeof(uvcb_attest->config_uid)))
>  		return -EFAULT;
>  	return 0;