On 10/13/2026 7:58 AM, [email protected] wrote: > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) > to consider: > - [High] ice: memory leak of pkt_buf and prof in ice_vc_add_fdir_fltr error > path > -- > > commit 6477a5eb13b0a0dff01249d4a775aff5939e63e5 > Author: Aleksandr Loktionov <[email protected]> > > ice: validate VF FDIR queue and queue region actions > > This commit validates the VF-supplied queue index and queue region in > ice_vc_fdir_parse_action(), rejecting invalid values to prevent a VF from > steering matched packets to queues beyond its own range. > >> diff --git a/drivers/net/ethernet/intel/ice/virt/fdir.c >> b/drivers/net/ethernet/intel/ice/virt/fdir.c >> --- a/drivers/net/ethernet/intel/ice/virt/fdir.c >> +++ b/drivers/net/ethernet/intel/ice/virt/fdir.c > [ ... ] >> @@ -1176,11 +1181,25 @@ ice_vc_fdir_parse_action(struct ice_vf *vf, struct >> virtchnl_fdir_add *fltr, >> break; >> case VIRTCHNL_ACTION_QUEUE: >> dest_num++; >> + if (action->act_conf.queue.index >= vsi->num_rxq) { >> + dev_dbg(dev, "Invalid queue index %u for VF >> %d\n", >> + action->act_conf.queue.index, >> vf->vf_id); >> + return -EINVAL; >> + } > > [Severity: High] > This problem wasn't introduced by this patch, but does returning here leak > memory if conf->pkt_buf and conf->prof were previously allocated? > > In ice_vc_add_fdir_fltr(), if ice_vc_fdir_parse_action() fails, it jumps to > err_free_conf which frees the outer structure but not the inner ones: > > ice_vc_add_fdir_fltr() { > ... > ret = ice_vc_fdir_parse_action(vf, fltr, conf); > if (ret) { > ... > goto err_free_conf; > } > ... > err_free_conf: > devm_kfree(dev, conf); > ... > } > > Earlier in the request handling, if this was a raw flow, > ice_vc_fdir_parse_raw() allocates pkt_buf and prof and assigns them to the > conf structure: > > ice_vc_fdir_parse_raw() { > ... > pkt_buf = kzalloc(pkt_len, GFP_KERNEL); > ... > conf->pkt_buf = pkt_buf; > ... > } > > Could an untrusted VF exhaust host memory by repeatedly sending requests > with invalid queue parameters that trigger this return path? >
At the very least, this new error should perform cleanup, even if you don't take the time to fix every existing error path way that is broken. Can you please investigate? Thanks, Jake >> input->dest_ctl = >> ICE_FLTR_PRGM_DESC_DEST_DIRECT_PKT_QINDEX; >> input->q_index = action->act_conf.queue.index; >> break; > [ ... ] >
