linux-kernel - Re: [PATCH v3 2/4] fsi: occ: Store the SBEFIFO FFDC in the user response buffer

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, 15 Oct 2021 05:05:27 +0000
From:   Joel Stanley <joel@....id.au>
To:     Eddie James <eajames@...ux.ibm.com>,
        Amitay Isaacs <amitay@...abs.org>
Cc:     linux-fsi@...ts.ozlabs.org,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-hwmon@...r.kernel.org, Jeremy Kerr <jk@...abs.org>,
        Alistair Popple <alistair@...ple.id.au>,
        Guenter Roeck <linux@...ck-us.net>,
        Jean Delvare <jdelvare@...e.com>
Subject: Re: [PATCH v3 2/4] fsi: occ: Store the SBEFIFO FFDC in the user
 response buffer

On Mon, 27 Sept 2021 at 15:59, Eddie James <eajames@...ux.ibm.com> wrote:
>
> If the SBEFIFO response indicates an error, store the response in the
> user buffer and return an error. Previously, the user had no way of
> obtaining the SBEFIFO FFDC.

How does this look for userspace?

Will existing userspace handle this?

>
> Signed-off-by: Eddie James <eajames@...ux.ibm.com>
> ---
> Changes since v1:
>  - Don't store any magic value; only return non-zero resp_len in the error
>    case if there is FFDC
>
>  drivers/fsi/fsi-occ.c | 66 ++++++++++++++++++++++++++++++-------------
>  1 file changed, 47 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/fsi/fsi-occ.c b/drivers/fsi/fsi-occ.c
> index ace3ec7767e5..1d5f6fdc2a34 100644
> --- a/drivers/fsi/fsi-occ.c
> +++ b/drivers/fsi/fsi-occ.c
> @@ -55,6 +55,9 @@ struct occ {
>         int idx;
>         u8 sequence_number;
>         void *buffer;
> +       void *client_buffer;
> +       size_t client_buffer_size;
> +       size_t client_response_size;
>         enum versions version;
>         struct miscdevice mdev;
>         struct mutex occ_lock;
> @@ -217,6 +220,20 @@ static const struct file_operations occ_fops = {
>         .release = occ_release,
>  };
>
> +static void occ_save_ffdc(struct occ *occ, __be32 *resp, size_t parsed_len,
> +                         size_t resp_len)
> +{
> +       size_t dh = resp_len - parsed_len;

Is there any chance that parsed_len is larger than resp_len?

> +       size_t ffdc_len = (dh - 1) * 4;
> +       __be32 *ffdc = &resp[resp_len - dh];

Should you be checking that this number is sensible?

> +
> +       if (ffdc_len > occ->client_buffer_size)
> +               ffdc_len = occ->client_buffer_size;
> +
> +       memcpy(occ->client_buffer, ffdc, ffdc_len);
> +       occ->client_response_size = ffdc_len;
> +}