Messages in this thread | | | From | Alexander Lobakin <> | Subject | Re: [PATCH net v2] ice: Fix incorrect locking in ice_vc_process_vf_msg() | Date | Fri, 8 Apr 2022 15:47:14 +0200 |
| |
From: Ivan Vecera <ivecera@redhat.com> Date: Fri, 1 Apr 2022 12:40:52 +0200
> Usage of mutex_trylock() in ice_vc_process_vf_msg() is incorrect > because message sent from VF is ignored and never processed. > > Use mutex_lock() instead to fix the issue. It is safe because this > mutex is used to prevent races between VF related NDOs and > handlers processing request messages from VF and these handlers > are running in ice_service_task() context. Additionally move this > mutex lock prior ice_vc_is_opcode_allowed() call to avoid potential > races during allowlist acccess. > > Fixes: e6ba5273d4ed ("ice: Fix race conditions between virtchnl handling and VF ndo ops") > Signed-off-by: Ivan Vecera <ivecera@redhat.com>
Hey Tony,
I guess you missed this one due to being on a vacation previously. It's been previously reviewed IIRC, could you take it into net-queue?
> --- > drivers/net/ethernet/intel/ice/ice_virtchnl.c | 21 +++++++------------ > 1 file changed, 7 insertions(+), 14 deletions(-) > > diff --git a/drivers/net/ethernet/intel/ice/ice_virtchnl.c b/drivers/net/ethernet/intel/ice/ice_virtchnl.c > index 3f1a63815bac..a465f3743ffc 100644 > --- a/drivers/net/ethernet/intel/ice/ice_virtchnl.c > +++ b/drivers/net/ethernet/intel/ice/ice_virtchnl.c > @@ -3642,14 +3642,6 @@ void ice_vc_process_vf_msg(struct ice_pf *pf, struct ice_rq_event_info *event) > err = -EINVAL; > } > > - if (!ice_vc_is_opcode_allowed(vf, v_opcode)) { > - ice_vc_send_msg_to_vf(vf, v_opcode, > - VIRTCHNL_STATUS_ERR_NOT_SUPPORTED, NULL, > - 0); > - ice_put_vf(vf); > - return; > - } > - > error_handler: > if (err) { > ice_vc_send_msg_to_vf(vf, v_opcode, VIRTCHNL_STATUS_ERR_PARAM, > @@ -3660,12 +3652,13 @@ void ice_vc_process_vf_msg(struct ice_pf *pf, struct ice_rq_event_info *event) > return; > } > > - /* VF is being configured in another context that triggers a VFR, so no > - * need to process this message > - */ > - if (!mutex_trylock(&vf->cfg_lock)) { > - dev_info(dev, "VF %u is being configured in another context that will trigger a VFR, so there is no need to handle this message\n", > - vf->vf_id); > + mutex_lock(&vf->cfg_lock); > + > + if (!ice_vc_is_opcode_allowed(vf, v_opcode)) { > + ice_vc_send_msg_to_vf(vf, v_opcode, > + VIRTCHNL_STATUS_ERR_NOT_SUPPORTED, NULL, > + 0); > + mutex_unlock(&vf->cfg_lock); > ice_put_vf(vf); > return; > } > -- > 2.35.1
Thanks, Al
| |