Skip to content
  • Pavel Skripkin's avatar
    e02a3b94
    staging: rtl8712: fix memory leak in rtl871x_load_fw_cb · e02a3b94
    Pavel Skripkin authored
    There is a leak in rtl8712 driver.
    The problem was in non-freed adapter data if
    firmware load failed.
    
    This leak can be reproduced with this code:
    https://syzkaller.appspot.com/text?tag=ReproC&x=16612f02d00000
    
    ,
    Autoload must fail (to not hit memory leak reported by syzkaller)
    
    There are 2 possible ways how rtl871x_load_fw_cb() and
    r871xu_dev_remove() can be called (in case of fw load error).
    
    1st case:
    	r871xu_dev_remove() then rtl871x_load_fw_cb()
    
    	In this case r871xu_dev_remove() will wait for
    	completion and then will jump to the end, because
    	rtl871x_load_fw_cb() set intfdata to NULL:
    
    	if (pnetdev) {
    		struct _adapter *padapter = netdev_priv(pnetdev);
    
    		/* never exit with a firmware callback pending */
    		wait_for_completion(&padapter->rtl8712_fw_ready);
    		pnetdev = usb_get_intfdata(pusb_intf);
    		usb_set_intfdata(pusb_intf, NULL);
    		if (!pnetdev)
    			goto firmware_load_fail;
    
    		... clean up code here ...
    	}
    
    2nd case:
    	rtl871x_load_fw_cb() then r871xu_dev_remove()
    
    	In this case pnetdev (from code snippet above) will
    	be zero (because rtl871x_load_fw_cb() set it to NULL)
    	And clean up code won't be executed again.
    
    So, in all cases we need to free adapted data in rtl871x_load_fw_cb(),
    because disconnect function cannot take care of it. And there won't be
    any race conditions, because complete() call happens after setting
    intfdata to NULL.
    
    In previous patch I moved out free_netdev() from r8712_free_drv_sw()
    and that's why now it's possible to free adapter data and then call
    complete.
    
    Fixes: 8c213fa5 ("staging: r8712u: Use asynchronous firmware loading")
    Signed-off-by: default avatarPavel Skripkin <paskripkin@gmail.com>
    Link: https://lore.kernel.org/r/81e68fe0194499cc2e7692d35bc4dcf167827d8f.1623620630.git.paskripkin@gmail.com
    
    
    Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
    e02a3b94
    staging: rtl8712: fix memory leak in rtl871x_load_fw_cb
    Pavel Skripkin authored
    There is a leak in rtl8712 driver.
    The problem was in non-freed adapter data if
    firmware load failed.
    
    This leak can be reproduced with this code:
    https://syzkaller.appspot.com/text?tag=ReproC&x=16612f02d00000
    
    ,
    Autoload must fail (to not hit memory leak reported by syzkaller)
    
    There are 2 possible ways how rtl871x_load_fw_cb() and
    r871xu_dev_remove() can be called (in case of fw load error).
    
    1st case:
    	r871xu_dev_remove() then rtl871x_load_fw_cb()
    
    	In this case r871xu_dev_remove() will wait for
    	completion and then will jump to the end, because
    	rtl871x_load_fw_cb() set intfdata to NULL:
    
    	if (pnetdev) {
    		struct _adapter *padapter = netdev_priv(pnetdev);
    
    		/* never exit with a firmware callback pending */
    		wait_for_completion(&padapter->rtl8712_fw_ready);
    		pnetdev = usb_get_intfdata(pusb_intf);
    		usb_set_intfdata(pusb_intf, NULL);
    		if (!pnetdev)
    			goto firmware_load_fail;
    
    		... clean up code here ...
    	}
    
    2nd case:
    	rtl871x_load_fw_cb() then r871xu_dev_remove()
    
    	In this case pnetdev (from code snippet above) will
    	be zero (because rtl871x_load_fw_cb() set it to NULL)
    	And clean up code won't be executed again.
    
    So, in all cases we need to free adapted data in rtl871x_load_fw_cb(),
    because disconnect function cannot take care of it. And there won't be
    any race conditions, because complete() call happens after setting
    intfdata to NULL.
    
    In previous patch I moved out free_netdev() from r8712_free_drv_sw()
    and that's why now it's possible to free adapter data and then call
    complete.
    
    Fixes: 8c213fa5 ("staging: r8712u: Use asynchronous firmware loading")
    Signed-off-by: default avatarPavel Skripkin <paskripkin@gmail.com>
    Link: https://lore.kernel.org/r/81e68fe0194499cc2e7692d35bc4dcf167827d8f.1623620630.git.paskripkin@gmail.com
    
    
    Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Loading