>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. > Looks good to me. Ok to merge.
Regards, Ravi