[dpdk-stable] [dpdk-dev] [PATCH] net/axgbe: fix double unlock coverity issue

[Ferruh Yigit]

On 10/21/2019 9:20 AM, Kumar, Ravi1 wrote:
>> 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: stable at dpdk.org
>>>>
>>>> Signed-off-by: Pallantla Poornima <pallantlax.poornima at 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.
>>
> Looks good to me. Ok to merge.

(Converting to explicit Ack)
Acked-by: Ravi Kumar <ravi1.kumar at amd.com>

Applied to dpdk-next-net/master, thanks.