Bug 188971
| Summary: | Function irda_usb_probe() does not set error code when the calls to kzalloc() fails | ||
|---|---|---|---|
| Product: | Drivers | Reporter: | bianpan (bianpan2010) |
| Component: | Network | Assignee: | drivers_network (drivers_network) |
| Status: | RESOLVED CODE_FIX | ||
| Severity: | normal | ||
| Priority: | P1 | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Kernel Version: | linux-4.9-rc6 | Subsystem: | |
| Regression: | No | Bisected commit-id: | |
| Attachments: | The patch fixes the bug | ||
Created attachment 256443 [details]
The patch fixes the bug
The patch has been merged into the latest version of the Linux kernel. So I will close the bug.
|
Function kzalloc() returns a NULL pointer if there is no enough memory. In function irda_usb_probe() defined in file drivers/net/irda/irda-usb.c, if the return values of the calls to kzalloc() (at lines 1726 and 1730) are NULL, the control flow jumps to labels "err_out_3" and "error_out_4", cleans allocated memory and returns variable ret. Because variable ret is check at line 1709, the value of variable ret may be 0. As a result, 0 (indicates success) may be returned even if the calls to kzalloc() fail. I think it is better to assign "-ENOMEM" to ret before the call to kzalloc(). Codes and comments related to this bug are summarised as follows. irda_usb_probe @@ drivers/net/irda/irda-usb.c 1597 static int irda_usb_probe(struct usb_interface *intf, 1598 const struct usb_device_id *id) 1599 { ... 1605 int ret = -ENOMEM; ... 1706 if (self->needspatch) { 1707 ret = usb_control_msg (self->usbdev, usb_sndctrlpipe (self->usbdev, 0), 1708 0x02, 0x40, 0, 0, NULL, 0, 500); 1709 if (ret < 0) { 1710 pr_debug("usb_control_msg failed %d\n", ret); 1711 goto err_out_3; 1712 } else { 1713 mdelay(10); 1714 } 1715 } 1716 1717 self->irda_desc = irda_desc; 1718 self->present = 1; 1719 self->netopen = 0; 1720 self->usbintf = intf; 1721 1722 /* Allocate the buffer for speed changes */ 1723 /* Don't change this buffer size and allocation without doing 1724 * some heavy and complete testing. Don't ask why :-( 1725 * Jean II */ // ret may takes value 0. Insert "ret = -ENOMEM;" here? 1726 self->speed_buff = kzalloc(IRDA_USB_SPEED_MTU, GFP_KERNEL); 1727 if (!self->speed_buff) 1728 goto err_out_3; 1729 1730 self->tx_buff = kzalloc(IRDA_SKB_MAX_MTU + self->header_length, 1731 GFP_KERNEL); 1732 if (!self->tx_buff) 1733 goto err_out_4; ... 1762 return 0; 1763 err_out_6: 1764 unregister_netdev(self->netdev); 1765 err_out_5: 1766 kfree(self->tx_buff); 1767 err_out_4: 1768 kfree(self->speed_buff); 1769 err_out_3: 1770 /* Free all urbs that we may have created */ 1771 usb_free_urb(self->speed_urb); 1772 err_out_2: 1773 usb_free_urb(self->tx_urb); 1774 err_out_1: 1775 for (i = 0; i < self->max_rx_urb; i++) 1776 usb_free_urb(self->rx_urb[i]); 1777 kfree(self->rx_urb); 1778 err_free_net: 1779 free_netdev(net); 1780 err_out: 1781 return ret; 1782 } Thanks very much!