| 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
| ||
|
Message-ID: <CAJaqyWdEZbURGZtmobrED_jBq34DnQEuC8kUoPMH5=p2K7NE0w@mail.gmail.com>
Date: Wed, 1 Oct 2025 15:26:13 +0200
From: Eugenio Perez Martin <eperezma@...hat.com>
To: Si-Wei Liu <si-wei.liu@...cle.com>
Cc: mst@...hat.com, jasowang@...hat.com, parav@...dia.com,
virtualization@...ts.linux-foundation.org, linux-kernel@...r.kernel.org,
Dragos Tatulea DE <dtatulea@...dia.com>, Maxime Coquelin <mcoqueli@...hat.com>
Subject: Re: [PATCH RESENT v4 4/6] vdpa: validate device feature provisioning
against supported class
On Tue, Feb 7, 2023 at 12:15 AM Si-Wei Liu <si-wei.liu@...cle.com> wrote:
>
> Today when device features are explicitly provisioned, the features
> user supplied may contain device class specific features that are
> not supported by the parent management device. On the other hand,
> when parent management device supports more than one class, the
> device features to provision may be ambiguous if none of the class
> specific attributes is provided at the same time. Validate these
> cases and prompt appropriate user errors accordingly.
>
> Signed-off-by: Si-Wei Liu <si-wei.liu@...cle.com>
> ---
> drivers/vdpa/vdpa.c | 59 +++++++++++++++++++++++++++++++++++++++++++++--------
> 1 file changed, 50 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
> index 1eba978..8da5120 100644
> --- a/drivers/vdpa/vdpa.c
> +++ b/drivers/vdpa/vdpa.c
> @@ -460,12 +460,28 @@ static int vdpa_nl_mgmtdev_handle_fill(struct sk_buff *msg, const struct vdpa_mg
> return 0;
> }
>
> +static u64 vdpa_mgmtdev_get_classes(const struct vdpa_mgmt_dev *mdev,
> + unsigned int *nclasses)
> +{
> + u64 supported_classes = 0;
> + unsigned int n = 0;
> +
> + for (int i = 0; mdev->id_table[i].device; i++) {
> + if (mdev->id_table[i].device > 63)
> + continue;
> + supported_classes |= BIT_ULL(mdev->id_table[i].device);
> + n++;
> + }
> + if (nclasses)
> + *nclasses = n;
> +
> + return supported_classes;
> +}
> +
> static int vdpa_mgmtdev_fill(const struct vdpa_mgmt_dev *mdev, struct sk_buff *msg,
> u32 portid, u32 seq, int flags)
> {
> - u64 supported_classes = 0;
> void *hdr;
> - int i = 0;
> int err;
>
> hdr = genlmsg_put(msg, portid, seq, &vdpa_nl_family, flags, VDPA_CMD_MGMTDEV_NEW);
> @@ -475,14 +491,9 @@ static int vdpa_mgmtdev_fill(const struct vdpa_mgmt_dev *mdev, struct sk_buff *m
> if (err)
> goto msg_err;
>
> - while (mdev->id_table[i].device) {
> - if (mdev->id_table[i].device <= 63)
> - supported_classes |= BIT_ULL(mdev->id_table[i].device);
> - i++;
> - }
> -
> if (nla_put_u64_64bit(msg, VDPA_ATTR_MGMTDEV_SUPPORTED_CLASSES,
> - supported_classes, VDPA_ATTR_UNSPEC)) {
> + vdpa_mgmtdev_get_classes(mdev, NULL),
> + VDPA_ATTR_UNSPEC)) {
> err = -EMSGSIZE;
> goto msg_err;
> }
> @@ -566,13 +577,25 @@ static int vdpa_nl_cmd_mgmtdev_get_doit(struct sk_buff *skb, struct genl_info *i
> BIT_ULL(VDPA_ATTR_DEV_NET_CFG_MTU) | \
> BIT_ULL(VDPA_ATTR_DEV_NET_CFG_MAX_VQP))
>
> +/*
> + * Bitmask for all per-device features: feature bits VIRTIO_TRANSPORT_F_START
> + * through VIRTIO_TRANSPORT_F_END are unset, i.e. 0xfffffc000fffffff for
> + * all 64bit features. If the features are extended beyond 64 bits, or new
> + * "holes" are reserved for other type of features than per-device, this
> + * macro would have to be updated.
> + */
> +#define VIRTIO_DEVICE_F_MASK (~0ULL << (VIRTIO_TRANSPORT_F_END + 1) | \
> + ((1ULL << VIRTIO_TRANSPORT_F_START) - 1))
> +
> static int vdpa_nl_cmd_dev_add_set_doit(struct sk_buff *skb, struct genl_info *info)
> {
> struct vdpa_dev_set_config config = {};
> struct nlattr **nl_attrs = info->attrs;
> struct vdpa_mgmt_dev *mdev;
> + unsigned int ncls = 0;
> const u8 *macaddr;
> const char *name;
> + u64 classes;
> int err = 0;
>
> if (!info->attrs[VDPA_ATTR_DEV_NAME])
> @@ -649,6 +672,24 @@ static int vdpa_nl_cmd_dev_add_set_doit(struct sk_buff *skb, struct genl_info *i
> goto err;
> }
>
> + classes = vdpa_mgmtdev_get_classes(mdev, &ncls);
> + if (config.mask & VDPA_DEV_NET_ATTRS_MASK &&
> + !(classes & BIT_ULL(VIRTIO_ID_NET))) {
> + NL_SET_ERR_MSG_MOD(info->extack,
> + "Network class attributes provided on unsupported management device");
> + err = -EINVAL;
> + goto err;
> + }
> + if (!(config.mask & VDPA_DEV_NET_ATTRS_MASK) &&
> + config.mask & BIT_ULL(VDPA_ATTR_DEV_FEATURES) &&
> + classes & BIT_ULL(VIRTIO_ID_NET) && ncls > 1 &&
> + config.device_features & VIRTIO_DEVICE_F_MASK) {
> + NL_SET_ERR_MSG_MOD(info->extack,
> + "Management device supports multi-class while device features specified are ambiguous");
> + err = -EINVAL;
> + goto err;
> + }
Hi! I need to question this last if() :). What's the point of error
when we specify features device-specific, from net or blk?
In the VDUSE case both blk and net are supported. I want to use
device_features to limit the net features that the VDUSE device
exports.
Also, why is this limited to only net devices? does this part:
classes & BIT_ULL(VIRTIO_ID_NET) && ncls > 1
Means that it is ok to specify more than one class as long as the set
does not contain net?
> +
> err = mdev->ops->dev_add(mdev, name, &config);
> err:
> up_write(&vdpa_dev_lock);
> --
> 1.8.3.1
>
Powered by blists - more mailing lists