On Wed, May 22, 2024 at 06:33:57AM -0700, Nikita Zhandarovich wrote: > On 5/20/24 10:18, Nam Cao wrote: > > On Mon, May 20, 2024 at 07:46:41AM -0700, Nikita Zhandarovich wrote: > >>> BUG: memory leak > >>> unreferenced object 0xffff888107a5c000 (size 4096): > >>> comm "kworker/1:0", pid 22, jiffies 4294943134 (age 18.720s) > >>> hex dump (first 32 bytes): > >>> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ > >>> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ > >>> backtrace: > >>> [<ffffffff816337cd>] kmemleak_alloc_recursive include/linux/kmemleak.h:42 [inline] > >>> [<ffffffff816337cd>] slab_post_alloc_hook mm/slab.h:766 [inline] > >>> [<ffffffff816337cd>] slab_alloc_node mm/slub.c:3478 [inline] > >>> [<ffffffff816337cd>] __kmem_cache_alloc_node+0x2dd/0x3f0 mm/slub.c:3517 > >>> [<ffffffff8157e625>] kmalloc_trace+0x25/0x90 mm/slab_common.c:1098 > >>> [<ffffffff83cee442>] kmalloc include/linux/slab.h:600 [inline] > >>> [<ffffffff83cee442>] _r8712_init_xmit_priv+0x2b2/0x6e0 drivers/staging/rtl8712/rtl871x_xmit.c:130 > >>> [<ffffffff83ce9033>] r8712_init_drv_sw+0xc3/0x290 drivers/staging/rtl8712/os_intfs.c:311 > >>> [<ffffffff83ce7ce6>] r871xu_drv_init+0x1c6/0x920 drivers/staging/rtl8712/usb_intf.c:386 > >>> [<ffffffff832d0f0b>] usb_probe_interface+0x16b/0x3a0 drivers/usb/core/driver.c:396 > >>> [<ffffffff82c3bb06>] call_driver_probe drivers/base/dd.c:579 [inline] > >> > >> I am inclined to think that this issue might be false positive. During > >> repro the device is initialized correctly, does some work and then > >> exits, calling all required functions to clean things up > >> (i.e. _free_xmit_priv()), including pxmitbuf->pallocated_buf. > >> Kmemleak triggers disappear if you set longer intervals between > >> scannning for them (obviously). And if all the things get cleared up > >> when the device disconnects, isn't that correct and expected > >> behaviour? Could the scanner just "lose track" of some of the objects > >> here? I think you may be right that this is false negative. I am guessing that kmemleak scans memory for pointers in block of 8-byte. However, this driver aligns the buffer from kmalloc() to 4 bytes, which is not necessary because pointers from kmalloc() is at least 8-byte-aligned. Then more pointers are stored in this 4-byte-aligned buffer. Thus, kmemleak misses these pointers, and falsely report memory leak. I never interacted with syzbot before, let's hope it can catch this: #syz test: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master diff --git a/drivers/staging/rtl8712/rtl871x_xmit.c b/drivers/staging/rtl8712/rtl871x_xmit.c index 6353dbe554d3..408616e9afcf 100644 --- a/drivers/staging/rtl8712/rtl871x_xmit.c +++ b/drivers/staging/rtl8712/rtl871x_xmit.c @@ -117,12 +117,9 @@ int _r8712_init_xmit_priv(struct xmit_priv *pxmitpriv, /*init xmit_buf*/ _init_queue(&pxmitpriv->free_xmitbuf_queue); _init_queue(&pxmitpriv->pending_xmitbuf_queue); - pxmitpriv->pallocated_xmitbuf = - kmalloc(NR_XMITBUFF * sizeof(struct xmit_buf) + 4, GFP_ATOMIC); - if (!pxmitpriv->pallocated_xmitbuf) + pxmitpriv->pxmitbuf = kmalloc(NR_XMITBUFF * sizeof(struct xmit_buf), GFP_ATOMIC); + if (!pxmitpriv->pxmitbuf) goto clean_up_frame_buf; - pxmitpriv->pxmitbuf = pxmitpriv->pallocated_xmitbuf + 4 - - ((addr_t)(pxmitpriv->pallocated_xmitbuf) & 3); pxmitbuf = (struct xmit_buf *)pxmitpriv->pxmitbuf; for (i = 0; i < NR_XMITBUFF; i++) { INIT_LIST_HEAD(&pxmitbuf->list); @@ -165,8 +162,8 @@ int _r8712_init_xmit_priv(struct xmit_priv *pxmitpriv, for (k = 0; k < 8; k++) /* delete xmit urb's */ usb_free_urb(pxmitbuf->pxmit_urb[k]); } - kfree(pxmitpriv->pallocated_xmitbuf); - pxmitpriv->pallocated_xmitbuf = NULL; + kfree(pxmitpriv->pxmitbuf); + pxmitpriv->pxmitbuf = NULL; clean_up_frame_buf: kfree(pxmitpriv->pallocated_frame_buf); pxmitpriv->pallocated_frame_buf = NULL; @@ -193,7 +190,7 @@ void _free_xmit_priv(struct xmit_priv *pxmitpriv) pxmitbuf++; } kfree(pxmitpriv->pallocated_frame_buf); - kfree(pxmitpriv->pallocated_xmitbuf); + kfree(pxmitpriv->pxmitbuf); free_hwxmits(padapter); } diff --git a/drivers/staging/rtl8712/rtl871x_xmit.h b/drivers/staging/rtl8712/rtl871x_xmit.h index cdcbc87a3cad..784172c385e3 100644 --- a/drivers/staging/rtl8712/rtl871x_xmit.h +++ b/drivers/staging/rtl8712/rtl871x_xmit.h @@ -244,7 +244,6 @@ struct xmit_priv { int cmdseq; struct __queue free_xmitbuf_queue; struct __queue pending_xmitbuf_queue; - u8 *pallocated_xmitbuf; u8 *pxmitbuf; uint free_xmitbuf_cnt; };
