Re: [PATCH v5 net-next 4/9] vhost-net: allow configuring extended features

[Simon Horman <horms@kernel.org>]

On Fri, Jun 20, 2025 at 07:39:48PM +0200, Paolo Abeni wrote:
> Use the extended feature type for 'acked_features' and implement
> two new ioctls operation allowing the user-space to set/query an
> unbounded amount of features.
> 
> The actual number of processed features is limited by VIRTIO_FEATURES_MAX
> and attempts to set features above such limit fail with
> EOPNOTSUPP.
> 
> Note that: the legacy ioctls implicitly truncate the negotiated
> features to the lower 64 bits range and the 'acked_backend_features'
> field don't need conversion, as the only negotiated feature there
> is in the low 64 bit range.
> 
> Acked-by: Jason Wang <jasowang@redhat.com>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>

...

> +	case VHOST_GET_FEATURES_ARRAY:
> +		if (get_user(count, featurep))
> +			return -EFAULT;
> +
> +		/* Copy the net features, up to the user-provided buffer size */
> +		argp += sizeof(u64);
> +		copied = min(count, VIRTIO_FEATURES_DWORDS);
> +		if (copy_to_user(argp, vhost_net_features,
> +				 copied * sizeof(u64)))
> +			return -EFAULT;
> +
> +		/* Zero the trailing space provided by user-space, if any */
> +		if (clear_user(argp, (count - copied) * sizeof(u64)))

Hi Paolo,

Smatch warns to "check for integer overflow 'count'" on the line above.

Perhaps it is wrong. Or my analyais is. But it seems to me that an overflow
could occur if count is very large, say such that (count - copied) is more
than 2^64 / 8.  As then (count - copied) * sizeof(u64) would overflow 64
bits.

By the same reasoning this could overflow 32 bits on systems where an
unsigned long, type type of the 2nd parameter of clear_user, is 32 bits.

> +			return -EFAULT;
> +		return 0;

...