On 10/9/2019 9:41 AM, Ferruh Yigit wrote:
> On 9/19/2019 12:01 PM, Pallantla Poornima wrote:
>> One issue caught by Coverity 340835
>> *unlock: axgbe_phy_set_mode unlocks pdata->phy_mutex
>> *double_unlock: axgbe_phy_sfp_detect unlocks pdata->phy_mutex
>> while it is unlocked.
>>
>> In axgbe_phy_sfp_detect()/axgbe_phy_set_redrv_mode(),
>> axgbe_phy_get_comm_ownership() and axgbe_phy_put_comm_ownership()
>> are invoked subsequently.
>>
>> Currently in axgbe_phy_get_comm_ownership(), during one of the case
>> 'phy_data->comm_owned' is not protected and before returning 0,
>> lock is not called and unlock is called in axgbe_phy_put_comm_ownership()
>> directly which is incorrect.
>>
>> Ideally, the variable 'phy_data->comm_owned' needs to be protected.
>> During success scenario, lock is called in axgbe_phy_get_comm_ownership()
>> followed by unlock in axgbe_phy_put_comm_ownership().
>> In failure case, unlock is invoked in axgbe_phy_get_comm_ownership()
>> itself appropriately.
>>
>> The fix is to protect 'phy_data->comm_owned' in the identified case
>> ensuring locks/unlocks properly exist.
>>
>> Coverity issue: 340835
>> Fixes: a5c7273771 ("net/axgbe: add phy programming APIs")
>> Cc: sta...@dpdk.org
>>
>> Signed-off-by: Pallantla Poornima <pallantlax.poorn...@intel.com>
>
> lgtm, 'axgbe_phy_put_comm_ownership()' expects
> 'axgbe_phy_get_comm_ownership()'
> gets the lock. Thanks for fixing the coverity issue.
>
> But still, Ravi can you please review/test the patch?
>
If there is no objection the patch will be merged soon.
