| Message ID | 20250120170026.1880-1-vulab@iscas.ac.cn (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | net: usb: qmi_wwan: Add error handling for usbnet_get_ethernet_addr | expand |
On Tue, Jan 21, 2025 at 01:00:26AM +0800, Wentao Liang wrote: > In qmi_wwan_bind(), usbnet_get_ethernet_addr() is called > without error handling, risking unnoticed failures and > inconsistent behavior compared to other parts of the code. > > Fix this issue by adding an error handling for the > usbnet_get_ethernet_addr(), improving code robustness. > > Fixes: 423ce8caab7e ("net: usb: qmi_wwan: New driver for Huawei QMI based WWAN devices") > Signed-off-by: Wentao Liang <vulab@iscas.ac.cn> Reviewed-by: Breno Leitao <leitao@debian.org>
Wentao Liang <vulab@iscas.ac.cn> writes: > In qmi_wwan_bind(), usbnet_get_ethernet_addr() is called > without error handling, risking unnoticed failures and > inconsistent behavior compared to other parts of the code. > > Fix this issue by adding an error handling for the > usbnet_get_ethernet_addr(), improving code robustness. > > Fixes: 423ce8caab7e ("net: usb: qmi_wwan: New driver for Huawei QMI based WWAN devices") > Signed-off-by: Wentao Liang <vulab@iscas.ac.cn> > --- > drivers/net/usb/qmi_wwan.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c > index e9208a8d2bfa..7aa576bfe76b 100644 > --- a/drivers/net/usb/qmi_wwan.c > +++ b/drivers/net/usb/qmi_wwan.c > @@ -779,7 +779,9 @@ static int qmi_wwan_bind(struct usbnet *dev, struct usb_interface *intf) > /* errors aren't fatal - we can live with the dynamic address */ > if (cdc_ether && cdc_ether->wMaxSegmentSize) { > dev->hard_mtu = le16_to_cpu(cdc_ether->wMaxSegmentSize); > - usbnet_get_ethernet_addr(dev, cdc_ether->iMACAddress); > + status = usbnet_get_ethernet_addr(dev, cdc_ether->iMACAddress); > + if (status < 0) > + goto err; > } > > /* claim data interface and set it up */ Did you read the comment? This intentinonally ignores any errors. I don't know how to make it clear anough for these AI bots to understand. Any advice there? NAK (and why weren't I CCed? Noticed this by chance only...) Bjørn
On 20.01.25 18:00, Wentao Liang wrote: > In qmi_wwan_bind(), usbnet_get_ethernet_addr() is called > without error handling, risking unnoticed failures and > inconsistent behavior compared to other parts of the code. Hi, unfortunately this makes no difference between a genuine IO error and the device simply providing no string. Does the device need to provide a string for the MAC in order to function? Frankly, this patch does not look like a good idea, as it disregards the reasoning expressed in the comment seen in the code. Regards Oliver
diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c index e9208a8d2bfa..7aa576bfe76b 100644 --- a/drivers/net/usb/qmi_wwan.c +++ b/drivers/net/usb/qmi_wwan.c @@ -779,7 +779,9 @@ static int qmi_wwan_bind(struct usbnet *dev, struct usb_interface *intf) /* errors aren't fatal - we can live with the dynamic address */ if (cdc_ether && cdc_ether->wMaxSegmentSize) { dev->hard_mtu = le16_to_cpu(cdc_ether->wMaxSegmentSize); - usbnet_get_ethernet_addr(dev, cdc_ether->iMACAddress); + status = usbnet_get_ethernet_addr(dev, cdc_ether->iMACAddress); + if (status < 0) + goto err; } /* claim data interface and set it up */
In qmi_wwan_bind(), usbnet_get_ethernet_addr() is called without error handling, risking unnoticed failures and inconsistent behavior compared to other parts of the code. Fix this issue by adding an error handling for the usbnet_get_ethernet_addr(), improving code robustness. Fixes: 423ce8caab7e ("net: usb: qmi_wwan: New driver for Huawei QMI based WWAN devices") Signed-off-by: Wentao Liang <vulab@iscas.ac.cn> --- drivers/net/usb/qmi_wwan.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)