Re: [PATCH] phy: phy-uclass: Add a missing error handling path

On 2023-07-02 20:47, Bhupesh Sharma wrote:

Since function 'phy_get_counts()' can return NULL, add handling for that error path inside callers of this function.

Do you have any example where this can/has happened?

Counts should never be NULL in a normal working call flow. I am a bit worried that this patch may hide some other bug or use of an uninitialized phy struct.

The phy struct is initialized in generic_phy_get_by_index_nodev. That function should fail when counts cannot be allocated.

Maybe generic_phy_get_by_index_nodev should unset phy->dev if it fails, or generic_phy_valid should be extended to also check phy->counts. That way generic_phy_valid would return false and these functions should return earlier before trying to use counts.

Regards, Jonas

Signed-off-by: Bhupesh Sharma bhupesh.sharma@linaro.org

drivers/phy/phy-uclass.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)

diff --git a/drivers/phy/phy-uclass.c b/drivers/phy/phy-uclass.c index 629ef3aa3d..c523a0b45e 100644 --- a/drivers/phy/phy-uclass.c +++ b/drivers/phy/phy-uclass.c @@ -229,6 +229,11 @@ int generic_phy_init(struct phy *phy) if (!generic_phy_valid(phy)) return 0; counts = phy_get_counts(phy);

• if (!counts) {

• debug("phy_get_counts returns NULL\n");
  

• return -ENODEV;
  

• }
• if (counts->init_count > 0) { counts->init_count++; return 0;

@@ -275,6 +280,11 @@ int generic_phy_exit(struct phy *phy) if (!generic_phy_valid(phy)) return 0; counts = phy_get_counts(phy);

• if (!counts) {

• debug("phy_get_counts returns NULL\n");
  

• return -ENODEV;
  

• }
• if (counts->init_count == 0) return 0; if (counts->init_count > 1) {

@@ -305,6 +315,11 @@ int generic_phy_power_on(struct phy *phy) if (!generic_phy_valid(phy)) return 0; counts = phy_get_counts(phy);

• if (!counts) {

• debug("phy_get_counts returns NULL\n");
  

• return -ENODEV;
  

• }
• if (counts->power_on_count > 0) { counts->power_on_count++; return 0;

@@ -341,6 +356,11 @@ int generic_phy_power_off(struct phy *phy) if (!generic_phy_valid(phy)) return 0; counts = phy_get_counts(phy);

• if (!counts) {

• debug("phy_get_counts returns NULL\n");
  

• return -ENODEV;
  

• }
• if (counts->power_on_count == 0) return 0; if (counts->power_on_count > 1) {