On 11/17/25 11:16 AM, Simon Horman wrote:
> On Wed, Nov 12, 2025 at 01:34:28PM -0600, Daniel Jurgens wrote:
>
> ...
>
>> +static int virtnet_ff_init(struct virtnet_ff *ff, struct virtio_device
>> *vdev)
>> +{
>> + size_t ff_mask_size = sizeof(struct virtio_net_ff_cap_mask_data) +
>> + sizeof(struct virtio_net_ff_selector) *
>> + VIRTIO_NET_FF_MASK_TYPE_MAX;
>> + struct virtio_admin_cmd_query_cap_id_result *cap_id_list;
>> + struct virtio_net_ff_selector *sel;
>> + size_t real_ff_mask_size;
>> + int err;
>> + int i;
>> +
>> + if (!vdev->config->admin_cmd_exec)
>> + return -EOPNOTSUPP;
>> +
>> + cap_id_list = kzalloc(sizeof(*cap_id_list), GFP_KERNEL);
>> + if (!cap_id_list)
>> + return -ENOMEM;
>> +
>> + err = virtio_admin_cap_id_list_query(vdev, cap_id_list);
>> + if (err)
>> + goto err_cap_list;
>> +
>> + if (!(VIRTIO_CAP_IN_LIST(cap_id_list,
>> + VIRTIO_NET_FF_RESOURCE_CAP) &&
>> + VIRTIO_CAP_IN_LIST(cap_id_list,
>> + VIRTIO_NET_FF_SELECTOR_CAP) &&
>> + VIRTIO_CAP_IN_LIST(cap_id_list,
>> + VIRTIO_NET_FF_ACTION_CAP))) {
>> + err = -EOPNOTSUPP;
>> + goto err_cap_list;
>> + }
>> +
>> + ff->ff_caps = kzalloc(sizeof(*ff->ff_caps), GFP_KERNEL);
>> + if (!ff->ff_caps) {
>> + err = -ENOMEM;
>> + goto err_cap_list;
>> + }
>> +
>> + err = virtio_admin_cap_get(vdev,
>> + VIRTIO_NET_FF_RESOURCE_CAP,
>> + ff->ff_caps,
>> + sizeof(*ff->ff_caps));
>> +
>> + if (err)
>> + goto err_ff;
>> +
>> + /* VIRTIO_NET_FF_MASK_TYPE start at 1 */
>> + for (i = 1; i <= VIRTIO_NET_FF_MASK_TYPE_MAX; i++)
>> + ff_mask_size += get_mask_size(i);
>> +
>> + ff->ff_mask = kzalloc(ff_mask_size, GFP_KERNEL);
>> + if (!ff->ff_mask) {
>> + err = -ENOMEM;
>> + goto err_ff;
>> + }
>> +
>> + err = virtio_admin_cap_get(vdev,
>> + VIRTIO_NET_FF_SELECTOR_CAP,
>> + ff->ff_mask,
>> + ff_mask_size);
>> +
>> + if (err)
>> + goto err_ff_mask;
>> +
>> + ff->ff_actions = kzalloc(sizeof(*ff->ff_actions) +
>> + VIRTIO_NET_FF_ACTION_MAX,
>> + GFP_KERNEL);
>> + if (!ff->ff_actions) {
>> + err = -ENOMEM;
>> + goto err_ff_mask;
>> + }
>> +
>> + err = virtio_admin_cap_get(vdev,
>> + VIRTIO_NET_FF_ACTION_CAP,
>> + ff->ff_actions,
>> + sizeof(*ff->ff_actions) +
>> VIRTIO_NET_FF_ACTION_MAX);
>> +
>> + if (err)
>> + goto err_ff_action;
>> +
>> + err = virtio_admin_cap_set(vdev,
>> + VIRTIO_NET_FF_RESOURCE_CAP,
>> + ff->ff_caps,
>> + sizeof(*ff->ff_caps));
>> + if (err)
>> + goto err_ff_action;
>> +
>> + real_ff_mask_size = sizeof(struct virtio_net_ff_cap_mask_data);
>> + sel = (void *)&ff->ff_mask->selectors[0];
>> +
>> + for (i = 0; i < ff->ff_mask->count; i++) {
>> + if (sel->length > MAX_SEL_LEN) {
>> + err = -EINVAL;
>> + goto err_ff_action;
>> + }
>> + real_ff_mask_size += sizeof(struct virtio_net_ff_selector) +
>> sel->length;
>> + sel = (void *)sel + sizeof(*sel) + sel->length;
>> + }
>
> Hi Daniel,
>
> I'm not sure that the bounds checking in the loop above is adequate.
> For example, if ff->ff_mask->count is larger than expected.
> Or sel->length returns MAX_SEL_LEN each time then it seems
> than sel could overrun the space allocated for ff->ff_mask.
>
> Flagged by Claude Code with https://github.com/masoncl/review-prompts/
>
I can also bound the loop by VIRTIO_NET_FF_MASK_TYPE_MAX. I'll also
address your comments about classifier and rules limits on patch 7 here,
by checking the rules and classifier limits are > 0.
I'll wait to push a new version until I hear back from Michael about the
threading comment he made on the cover letter.
