| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Fri, 15 Apr 2022 13:55:06 -0700
From: Tony Nguyen <anthony.l.nguyen@...el.com>
To: Maciej Fijalkowski <maciej.fijalkowski@...el.com>,
Ivan Vecera <ivecera@...hat.com>
CC: Fei Liu <feliu@...hat.com>, <netdev@...r.kernel.org>,
<mschmidt@...hat.com>, Brett Creeley <brett@...sando.io>,
open list <linux-kernel@...r.kernel.org>,
"moderated list:INTEL ETHERNET DRIVERS"
<intel-wired-lan@...ts.osuosl.org>,
Jakub Kicinski <kuba@...nel.org>,
"Paolo Abeni" <pabeni@...hat.com>,
"David S. Miller" <davem@...emloft.net>
Subject: Re: [Intel-wired-lan] [PATCH net] ice: Protect vf_state check by
cfg_lock in ice_vc_process_vf_msg()
On 4/15/2022 4:57 AM, Maciej Fijalkowski wrote:
<snip>
>>> diff --git a/drivers/net/ethernet/intel/ice/ice_virtchnl.c b/drivers/net/ethernet/intel/ice/ice_virtchnl.c
>>> index 5612c032f15a..553287a75b50 100644
>>> --- a/drivers/net/ethernet/intel/ice/ice_virtchnl.c
>>> +++ b/drivers/net/ethernet/intel/ice/ice_virtchnl.c
>>> @@ -3625,44 +3625,39 @@ void ice_vc_process_vf_msg(struct ice_pf *pf, struct ice_rq_event_info *event)
>>> return;
>>> }
>>>
>>> + mutex_lock(&vf->cfg_lock);
>>> +
>>> /* Check if VF is disabled. */
>>> if (test_bit(ICE_VF_STATE_DIS, vf->vf_states)) {
>>> err = -EPERM;
>>> - goto error_handler;
>>> - }
>>> -
>>> - ops = vf->virtchnl_ops;
>>> -
>>> - /* Perform basic checks on the msg */
>>> - err = virtchnl_vc_validate_vf_msg(&vf->vf_ver, v_opcode, msg, msglen);
>>> - if (err) {
>>> - if (err == VIRTCHNL_STATUS_ERR_PARAM)
>>> - err = -EPERM;
>>> - else
>>> - err = -EINVAL;
>>> + } else {
>>> + /* Perform basic checks on the msg */
>>> + err = virtchnl_vc_validate_vf_msg(&vf->vf_ver, v_opcode, msg,
>>> + msglen);
>>> + if (err) {
>>> + if (err == VIRTCHNL_STATUS_ERR_PARAM)
>>> + err = -EPERM;
>>> + else
>>> + err = -EINVAL;
>>> + }
>> The chunk above feels a bit like unnecessary churn, no?
>> Couldn't this patch be simply focused only on extending critical section?
Agree, this doesn't seem related to the fix.
Thanks,
Tony
>>> }
>>> -
>>> -error_handler:
>>> if (err) {
>>> ice_vc_send_msg_to_vf(vf, v_opcode, VIRTCHNL_STATUS_ERR_PARAM,
>>> NULL, 0);
>>> dev_err(dev, "Invalid message from VF %d, opcode %d, len %d, error %d\n",
>>> vf_id, v_opcode, msglen, err);
>>> - ice_put_vf(vf);
>>> - return;
>>> + goto finish;
>>> }
>>>
>>> - 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;
>>> + goto finish;
>>> }
>>>
>>> + ops = vf->virtchnl_ops;
>>> +
>>> switch (v_opcode) {
>>> case VIRTCHNL_OP_VERSION:
>>> err = ops->get_ver_msg(vf, msg);
>>> @@ -3773,6 +3768,7 @@ void ice_vc_process_vf_msg(struct ice_pf *pf, struct ice_rq_event_info *event)
>>> vf_id, v_opcode, err);
>>> }
>>>
>>> +finish:
>>> mutex_unlock(&vf->cfg_lock);
>>> ice_put_vf(vf);
>>> }
>>> --
>>> 2.35.1
>>>
>>> _______________________________________________
>>> Intel-wired-lan mailing list
>>> Intel-wired-lan@...osl.org
>>> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
> _______________________________________________
> Intel-wired-lan mailing list
> Intel-wired-lan@...osl.org
> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
Powered by blists - more mailing lists