> From: Michael S. Tsirkin <[email protected]>
> Sent: Wednesday, February 24, 2021 12:33 PM
> 
> > +
> > +   features = vdev->config->get_features(vdev);
> > +   if ((features & (1ULL << VIRTIO_NET_F_MQ)) == 0)
> > +           return 0;
> > +
> > +   if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_CFG_MAX_VQP,
> > +                   config->max_virtqueue_pairs))
> > +           return -EMSGSIZE;
> > +   if (nla_put_u8(msg,
> VDPA_ATTR_DEV_NET_CFG_RSS_MAX_KEY_LEN,
> > +                  config->rss_max_key_size))
> 
> Why is it ok to poke at RSS fields without checking the relevant feature bits?
> 
It should be checked.
I relied on _MQ feature bit but yes, even with MQ, RSS can be off.
Will fix to read RSS feature bit.

> > +           return -EMSGSIZE;
> 
> Did you check this with sparse?
> max_virtqueue_pairs is __virtio16.
I will check and go through the types.

> 
> > +
> > +   rss_field = le16_to_cpu(config->rss_max_key_size);
> > +   if (nla_put_u16(msg,
> VDPA_ATTR_DEV_NET_CFG_RSS_MAX_IT_LEN, rss_field))
> > +           return -EMSGSIZE;
> > +
> > +   hash_types = le32_to_cpu(config->supported_hash_types);
> 
> unused variable
Will remove.

> 
> > +   if (nla_put_u32(msg, VDPA_ATTR_DEV_NET_CFG_RSS_HASH_TYPES,
> > +                   config->supported_hash_types))
> > +           return -EMSGSIZE;
> > +   return 0;
> > +}
> > +
> > +static int vdpa_dev_net_config_fill(struct vdpa_device *vdev, struct
> > +sk_buff *msg) {
> > +   struct virtio_net_config config = {};
> > +
> > +   vdev->config->get_config(vdev, 0, &config, sizeof(config));
> > +   if (nla_put(msg, VDPA_ATTR_DEV_NET_CFG_MACADDR,
> sizeof(config.mac), config.mac))
> > +           return -EMSGSIZE;
> > +   if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_STATUS, config.status))
> > +           return -EMSGSIZE;
> > +   if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_CFG_MTU,
> config.mtu))
> > +           return -EMSGSIZE;
> > +   if (nla_put_u32(msg, VDPA_ATTR_DEV_NET_CFG_SPEED,
> config.speed))
> > +           return -EMSGSIZE;
> 
> looks like a bunch of endian-ness/sparse errors to me, and a bunch of fields
> checked without checking the feature bits.
Yes, few are missed out.
Fixing them in v2.
_______________________________________________
Virtualization mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Reply via email to