Coverity reported the following:
*** CID 1530573: (UNINIT)
drivers/net/phy/phy-c45.c:1036 in genphy_c45_plca_set_cfg()
1030 return ret;
1031
1032 val = ret;
1033 }
1034
1035 if (plca_cfg->node_cnt >= 0)
vvv CID 1530573: (UNINIT)
vvv Using uninitialized value "val".
1036 val = (val & ~MDIO_OATC14_PLCA_NCNT) |
1037 (plca_cfg->node_cnt << 8);
1038
1039 if (plca_cfg->node_id >= 0)
1040 val = (val & ~MDIO_OATC14_PLCA_ID) |
1041 (plca_cfg->node_id);
drivers/net/phy/phy-c45.c:1076 in genphy_c45_plca_set_cfg()
1070 return ret;
1071
1072 val = ret;
1073 }
1074
1075 if (plca_cfg->burst_cnt >= 0)
vvv CID 1530573: (UNINIT)
vvv Using uninitialized value "val".
1076 val = (val & ~MDIO_OATC14_PLCA_MAXBC) |
1077 (plca_cfg->burst_cnt << 8);
1078
1079 if (plca_cfg->burst_tmr >= 0)
1080 val = (val & ~MDIO_OATC14_PLCA_BTMR) |
1081 (plca_cfg->burst_tmr);
This is not actually creating a real problem because the path leading to
'val' being used uninitialized will eventually override the full content
of that variable before actually using it for writing the register.
However, the fix is simple and comes at basically no cost.
Signed-off-by: Piergiorgio Beruto <[email protected]>
Reported-by: coverity-bot <[email protected]>
Addresses-Coverity-ID: 1530573 ("UNINIT")
Fixes: 493323416fed ("drivers/net/phy: add helpers to get/set PLCA configuration")
---
drivers/net/phy/phy-c45.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c
index cff83220595c..9f9565a4819d 100644
--- a/drivers/net/phy/phy-c45.c
+++ b/drivers/net/phy/phy-c45.c
@@ -999,8 +999,8 @@ EXPORT_SYMBOL_GPL(genphy_c45_plca_get_cfg);
int genphy_c45_plca_set_cfg(struct phy_device *phydev,
const struct phy_plca_cfg *plca_cfg)
{
+ u16 val = 0;
int ret;
- u16 val;
// PLCA IDVER is read-only
if (plca_cfg->version >= 0)
--
2.37.4
On 1/17/2023 1:47 PM, Piergiorgio Beruto wrote:
> Coverity reported the following:
>
> *** CID 1530573: (UNINIT)
> drivers/net/phy/phy-c45.c:1036 in genphy_c45_plca_set_cfg()
> 1030 return ret;
> 1031
> 1032 val = ret;
> 1033 }
> 1034
> 1035 if (plca_cfg->node_cnt >= 0)
> vvv CID 1530573: (UNINIT)
> vvv Using uninitialized value "val".
> 1036 val = (val & ~MDIO_OATC14_PLCA_NCNT) |
> 1037 (plca_cfg->node_cnt << 8);
> 1038
> 1039 if (plca_cfg->node_id >= 0)
> 1040 val = (val & ~MDIO_OATC14_PLCA_ID) |
> 1041 (plca_cfg->node_id);
> drivers/net/phy/phy-c45.c:1076 in genphy_c45_plca_set_cfg()
> 1070 return ret;
> 1071
> 1072 val = ret;
> 1073 }
> 1074
> 1075 if (plca_cfg->burst_cnt >= 0)
> vvv CID 1530573: (UNINIT)
> vvv Using uninitialized value "val".
> 1076 val = (val & ~MDIO_OATC14_PLCA_MAXBC) |
> 1077 (plca_cfg->burst_cnt << 8);
> 1078
> 1079 if (plca_cfg->burst_tmr >= 0)
> 1080 val = (val & ~MDIO_OATC14_PLCA_BTMR) |
> 1081 (plca_cfg->burst_tmr);
>
> This is not actually creating a real problem because the path leading to
> 'val' being used uninitialized will eventually override the full content
> of that variable before actually using it for writing the register.
> However, the fix is simple and comes at basically no cost.
>
Makes sense, and its better to be clear, and prevent the introduction of
a bug later if somehow it refactored such that the value is not
re-initialized before use in that case.
Reviewed-by: Jacob Keller <[email protected]>
> Signed-off-by: Piergiorgio Beruto <[email protected]>
> Reported-by: coverity-bot <[email protected]>
> Addresses-Coverity-ID: 1530573 ("UNINIT")
> Fixes: 493323416fed ("drivers/net/phy: add helpers to get/set PLCA configuration")
> ---
> drivers/net/phy/phy-c45.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c
> index cff83220595c..9f9565a4819d 100644
> --- a/drivers/net/phy/phy-c45.c
> +++ b/drivers/net/phy/phy-c45.c
> @@ -999,8 +999,8 @@ EXPORT_SYMBOL_GPL(genphy_c45_plca_get_cfg);
> int genphy_c45_plca_set_cfg(struct phy_device *phydev,
> const struct phy_plca_cfg *plca_cfg)
> {
> + u16 val = 0;
> int ret;
> - u16 val;
>
> // PLCA IDVER is read-only
> if (plca_cfg->version >= 0)
On Tue, 17 Jan 2023 22:47:53 +0100 Piergiorgio Beruto wrote:
> Subject: [PATCH net-next 1/1] drivers/phylib: fix coverity issue
The title of the patch should refer to the bug rather than which tool
found it.
here, for eaxmple:
net: phy: fix use of uninit variable when setting PLCA config
> Coverity reported the following:
>
> *** CID 1530573: (UNINIT)
> drivers/net/phy/phy-c45.c:1036 in genphy_c45_plca_set_cfg()
> 1030 return ret;
> 1031
> 1032 val = ret;
> 1033 }
> 1034
> 1035 if (plca_cfg->node_cnt >= 0)
[snip]
> Signed-off-by: Piergiorgio Beruto <[email protected]>
> Reported-by: coverity-bot <[email protected]>
> Addresses-Coverity-ID: 1530573 ("UNINIT")
> Fixes: 493323416fed ("drivers/net/phy: add helpers to get/set PLCA configuration")
nit: the tags are in somewhat unnatural order. Since you'll need to
respin for the subject change, this would be better:
Reported-by: coverity-bot <[email protected]>
Fixes: 493323416fed ("drivers/net/phy: add helpers to get/set PLCA configuration")
Signed-off-by: Piergiorgio Beruto <[email protected]>
(Yes, the custom coverity tag can go meet its friends in the bin)
On Tue, Jan 17, 2023 at 07:26:04PM -0800, Jakub Kicinski wrote:
> On Tue, 17 Jan 2023 22:47:53 +0100 Piergiorgio Beruto wrote:
> > Subject: [PATCH net-next 1/1] drivers/phylib: fix coverity issue
>
> The title of the patch should refer to the bug rather than which tool
> found it.
>
> here, for eaxmple:
>
> net: phy: fix use of uninit variable when setting PLCA config
>
> > Coverity reported the following:
> >
> > *** CID 1530573: (UNINIT)
> > drivers/net/phy/phy-c45.c:1036 in genphy_c45_plca_set_cfg()
> > 1030 return ret;
> > 1031
> > 1032 val = ret;
> > 1033 }
> > 1034
> > 1035 if (plca_cfg->node_cnt >= 0)
> [snip]
> > Signed-off-by: Piergiorgio Beruto <[email protected]>
> > Reported-by: coverity-bot <[email protected]>
> > Addresses-Coverity-ID: 1530573 ("UNINIT")
> > Fixes: 493323416fed ("drivers/net/phy: add helpers to get/set PLCA configuration")
>
> nit: the tags are in somewhat unnatural order. Since you'll need to
> respin for the subject change, this would be better:
>
> Reported-by: coverity-bot <[email protected]>
> Fixes: 493323416fed ("drivers/net/phy: add helpers to get/set PLCA configuration")
> Signed-off-by: Piergiorgio Beruto <[email protected]>
>
> (Yes, the custom coverity tag can go meet its friends in the bin)
Thanks Jakub,
I just fixed that.
Kind Regards,
Piergiorgio