re: vduse: add vq group support

re: vduse: add vq group support

[Colin King (gmail)]

[-- Attachment #1.1.1: Type: text/plain, Size: 1205 bytes --]

Hi,

Static analysis on linux-next has found an issue with the following commit:

commit ffc3634b66967445f3368c3b53a42bccc52b2c7f
Author: Eugenio Pérez <eperezma@redhat.com>
Date:   Thu Sep 25 11:13:32 2025 +0200

     vduse: add vq group support


This issue is as follows in function vhost_vdpa_vring_ioct:

         case VHOST_VDPA_GET_VRING_GROUP: {
                 u64 group;

                 if (!ops->get_vq_group)
                         return -EOPNOTSUPP;
                 s.index = idx;
                 group = ops->get_vq_group(vdpa, idx);
                 if (group >= vdpa->ngroups || group > U32_MAX || group < 0)
                         return -EIO;
                 else if (copy_to_user(argp, &s, sizeof(s)))
                         return -EFAULT;
                 s.num = group;
                 return 0;
         }


The copy_to_user of struct s is copying a partially initialized struct 
s, field s.num contains garbage data from the stack and this is being 
copied back to user space. Field s.num should be assigned some value 
before the copy_to_user call to avoid uninitialized data from the stack 
being leaked to user space.

Colin


[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 4901 bytes --]

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

Re: vduse: add vq group support

[Eugenio Perez Martin]

On Sat, Sep 27, 2025 at 4:22 PM Colin King (gmail)
<colin.i.king@gmail.com> wrote:
>
> Hi,
>
> Static analysis on linux-next has found an issue with the following commit:
>
> commit ffc3634b66967445f3368c3b53a42bccc52b2c7f
> Author: Eugenio Pérez <eperezma@redhat.com>
> Date:   Thu Sep 25 11:13:32 2025 +0200
>
>      vduse: add vq group support
>
>
> This issue is as follows in function vhost_vdpa_vring_ioct:
>
>          case VHOST_VDPA_GET_VRING_GROUP: {
>                  u64 group;
>
>                  if (!ops->get_vq_group)
>                          return -EOPNOTSUPP;
>                  s.index = idx;
>                  group = ops->get_vq_group(vdpa, idx);
>                  if (group >= vdpa->ngroups || group > U32_MAX || group < 0)
>                          return -EIO;
>                  else if (copy_to_user(argp, &s, sizeof(s)))
>                          return -EFAULT;
>                  s.num = group;
>                  return 0;
>          }
>
>
> The copy_to_user of struct s is copying a partially initialized struct
> s, field s.num contains garbage data from the stack and this is being
> copied back to user space. Field s.num should be assigned some value
> before the copy_to_user call to avoid uninitialized data from the stack
> being leaked to user space.
>

That's right! v5 of the patch fixes the issue.

Thanks!