| Message ID | 20240821131949.1465949-1-make24@iscas.ac.cn (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: stmmac: Check NULL ptr on lvts_data in qcom_ethqos_probe() | expand |
On Wed, Aug 21, 2024 at 09:19:49PM +0800, Ma Ke wrote: > of_device_get_match_data() can return NULL if of_match_device failed, and > the pointer 'data' was dereferenced without checking against NULL. Add > checking of pointer 'data' in qcom_ethqos_probe(). > > Cc: stable@vger.kernel.org > Fixes: a7c30e62d4b8 ("net: stmmac: Add driver for Qualcomm ethqos") > Signed-off-by: Ma Ke <make24@iscas.ac.cn> Hi Ma Ke, There is probably no need to repost just because of this. But as a fix for Networking code it should be targeted at the net tree. Subject: [PATCH net] ... > --- > drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c > index 901a3c1959fa..f18393fe58a4 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c > @@ -838,6 +838,9 @@ static int qcom_ethqos_probe(struct platform_device *pdev) > ethqos->mac_base = stmmac_res.addr; > > data = of_device_get_match_data(dev); > + if (!data) > + return -ENODEV; > + In this function dev_err_probe() is used, I assume in cases where a function that returns an error does not emit any logs. For consistency, perhaps that is appropriate here too? > ethqos->por = data->por; > ethqos->num_por = data->num_por; > ethqos->rgmii_config_loopback_en = data->rgmii_config_loopback_en; > -- > 2.25.1
On Wed, Aug 21, 2024 at 06:37:30PM +0100, Simon Horman wrote: > On Wed, Aug 21, 2024 at 09:19:49PM +0800, Ma Ke wrote: > > of_device_get_match_data() can return NULL if of_match_device failed, and > > the pointer 'data' was dereferenced without checking against NULL. Add > > checking of pointer 'data' in qcom_ethqos_probe(). > > > > Cc: stable@vger.kernel.org > > Fixes: a7c30e62d4b8 ("net: stmmac: Add driver for Qualcomm ethqos") > > Signed-off-by: Ma Ke <make24@iscas.ac.cn> > > Hi Ma Ke, > > There is probably no need to repost just because of this. > But as a fix for Networking code it should be targeted at the net tree. > > Subject: [PATCH net] ... > > > --- > > drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c > > index 901a3c1959fa..f18393fe58a4 100644 > > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c > > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c > > @@ -838,6 +838,9 @@ static int qcom_ethqos_probe(struct platform_device *pdev) > > ethqos->mac_base = stmmac_res.addr; > > > > data = of_device_get_match_data(dev); > > + if (!data) > > + return -ENODEV; > > + > > In this function dev_err_probe() is used, I assume in cases > where a function that returns an error does not emit any logs. > > For consistency, perhaps that is appropriate here too? > Unless I'm missing something here this function can only ever be invoked by a match against one of the entries in qcom_ethqos_match[], which all of them have a non-NULL data pointer. As such, if we somehow arrive here with data of NULL, the NULL pointer dereference on the next line will provide a welcome large splat and a callstack indicating that we have a problem. If there's some use case I'm missing, I would prefer if this was documented in the commit message. Regards, Bjorn > > ethqos->por = data->por; > > ethqos->num_por = data->num_por; > > ethqos->rgmii_config_loopback_en = data->rgmii_config_loopback_en; > > -- > > 2.25.1
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c index 901a3c1959fa..f18393fe58a4 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c @@ -838,6 +838,9 @@ static int qcom_ethqos_probe(struct platform_device *pdev) ethqos->mac_base = stmmac_res.addr; data = of_device_get_match_data(dev); + if (!data) + return -ENODEV; + ethqos->por = data->por; ethqos->num_por = data->num_por; ethqos->rgmii_config_loopback_en = data->rgmii_config_loopback_en;
of_device_get_match_data() can return NULL if of_match_device failed, and the pointer 'data' was dereferenced without checking against NULL. Add checking of pointer 'data' in qcom_ethqos_probe(). Cc: stable@vger.kernel.org Fixes: a7c30e62d4b8 ("net: stmmac: Add driver for Qualcomm ethqos") Signed-off-by: Ma Ke <make24@iscas.ac.cn> --- drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c | 3 +++ 1 file changed, 3 insertions(+)