2020-06-08 07:10:17

by Zhang, Qiang

[permalink] [raw]
Subject: [PATCH v2] usb: gadget: function: printer: fix use-after-free in __lock_acquire

From: Zqiang <[email protected]>

Increase the reference count of the printer dev through kref to avoid
being released by other tasks when in use.

BUG: KASAN: use-after-free in __lock_acquire+0x3fd4/0x4180
kernel/locking/lockdep.c:3831
Read of size 8 at addr ffff8880683b0018 by task syz-executor.0/3377

CPU: 1 PID: 3377 Comm: syz-executor.0 Not tainted 5.6.11 #1
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0xce/0x128 lib/dump_stack.c:118
print_address_description.constprop.4+0x21/0x3c0 mm/kasan/report.c:374
__kasan_report+0x131/0x1b0 mm/kasan/report.c:506
kasan_report+0x12/0x20 mm/kasan/common.c:641
__asan_report_load8_noabort+0x14/0x20 mm/kasan/generic_report.c:135
__lock_acquire+0x3fd4/0x4180 kernel/locking/lockdep.c:3831
lock_acquire+0x127/0x350 kernel/locking/lockdep.c:4488
__raw_spin_lock_irqsave include/linux/spinlock_api_smp.h:110 [inline]
_raw_spin_lock_irqsave+0x35/0x50 kernel/locking/spinlock.c:159
printer_ioctl+0x4a/0x110 drivers/usb/gadget/function/f_printer.c:723
vfs_ioctl fs/ioctl.c:47 [inline]
ksys_ioctl+0xfb/0x130 fs/ioctl.c:763
__do_sys_ioctl fs/ioctl.c:772 [inline]
__se_sys_ioctl fs/ioctl.c:770 [inline]
__x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:770
do_syscall_64+0x9e/0x510 arch/x86/entry/common.c:294
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x4531a9
Code: ed 60 fc ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48
89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d
01 f0 ff ff 0f 83 bb 60 fc ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007fd14ad72c78 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
RAX: ffffffffffffffda RBX: 000000000073bfa8 RCX: 00000000004531a9
RDX: fffffffffffffff9 RSI: 000000000000009e RDI: 0000000000000003
RBP: 0000000000000003 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 00000000004bbd61
R13: 00000000004d0a98 R14: 00007fd14ad736d4 R15: 00000000ffffffff

Allocated by task 2393:
save_stack+0x21/0x90 mm/kasan/common.c:72
set_track mm/kasan/common.c:80 [inline]
__kasan_kmalloc.constprop.3+0xa7/0xd0 mm/kasan/common.c:515
kasan_kmalloc+0x9/0x10 mm/kasan/common.c:529
kmem_cache_alloc_trace+0xfa/0x2d0 mm/slub.c:2813
kmalloc include/linux/slab.h:555 [inline]
kzalloc include/linux/slab.h:669 [inline]
gprinter_alloc+0xa1/0x870 drivers/usb/gadget/function/f_printer.c:1416
usb_get_function+0x58/0xc0 drivers/usb/gadget/functions.c:61
config_usb_cfg_link+0x1ed/0x3e0 drivers/usb/gadget/configfs.c:444
configfs_symlink+0x527/0x11d0 fs/configfs/symlink.c:202
vfs_symlink+0x33d/0x5b0 fs/namei.c:4201
do_symlinkat+0x11b/0x1d0 fs/namei.c:4228
__do_sys_symlinkat fs/namei.c:4242 [inline]
__se_sys_symlinkat fs/namei.c:4239 [inline]
__x64_sys_symlinkat+0x73/0xb0 fs/namei.c:4239
do_syscall_64+0x9e/0x510 arch/x86/entry/common.c:294
entry_SYSCALL_64_after_hwframe+0x49/0xbe

Freed by task 3368:
save_stack+0x21/0x90 mm/kasan/common.c:72
set_track mm/kasan/common.c:80 [inline]
kasan_set_free_info mm/kasan/common.c:337 [inline]
__kasan_slab_free+0x135/0x190 mm/kasan/common.c:476
kasan_slab_free+0xe/0x10 mm/kasan/common.c:485
slab_free_hook mm/slub.c:1444 [inline]
slab_free_freelist_hook mm/slub.c:1477 [inline]
slab_free mm/slub.c:3034 [inline]
kfree+0xf7/0x410 mm/slub.c:3995
gprinter_free+0x49/0xd0 drivers/usb/gadget/function/f_printer.c:1353
usb_put_function+0x38/0x50 drivers/usb/gadget/functions.c:87
config_usb_cfg_unlink+0x2db/0x3b0 drivers/usb/gadget/configfs.c:485
configfs_unlink+0x3b9/0x7f0 fs/configfs/symlink.c:250
vfs_unlink+0x287/0x570 fs/namei.c:4073
do_unlinkat+0x4f9/0x620 fs/namei.c:4137
__do_sys_unlink fs/namei.c:4184 [inline]
__se_sys_unlink fs/namei.c:4182 [inline]
__x64_sys_unlink+0x42/0x50 fs/namei.c:4182
do_syscall_64+0x9e/0x510 arch/x86/entry/common.c:294
entry_SYSCALL_64_after_hwframe+0x49/0xbe

The buggy address belongs to the object at ffff8880683b0000
which belongs to the cache kmalloc-1k of size 1024
The buggy address is located 24 bytes inside of
1024-byte region [ffff8880683b0000, ffff8880683b0400)
The buggy address belongs to the page:
page:ffffea0001a0ec00 refcount:1 mapcount:0 mapping:ffff88806c00e300
index:0xffff8880683b1800 compound_mapcount: 0
flags: 0x100000000010200(slab|head)
raw: 0100000000010200 0000000000000000 0000000600000001 ffff88806c00e300
raw: ffff8880683b1800 000000008010000a 00000001ffffffff 0000000000000000
page dumped because: kasan: bad access detected

Reported-by: Kyungtae Kim <[email protected]>
Signed-off-by: Zqiang <[email protected]>
---
v1->v2:
Commit information modification.

drivers/usb/gadget/function/f_printer.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/function/f_printer.c b/drivers/usb/gadget/function/f_printer.c
index 9c7ed2539ff7..8ed1295d7e35 100644
--- a/drivers/usb/gadget/function/f_printer.c
+++ b/drivers/usb/gadget/function/f_printer.c
@@ -31,6 +31,7 @@
#include <linux/types.h>
#include <linux/ctype.h>
#include <linux/cdev.h>
+#include <linux/kref.h>

#include <asm/byteorder.h>
#include <linux/io.h>
@@ -64,7 +65,7 @@ struct printer_dev {
struct usb_gadget *gadget;
s8 interface;
struct usb_ep *in_ep, *out_ep;
-
+ struct kref kref;
struct list_head rx_reqs; /* List of free RX structs */
struct list_head rx_reqs_active; /* List of Active RX xfers */
struct list_head rx_buffers; /* List of completed xfers */
@@ -218,6 +219,13 @@ static inline struct usb_endpoint_descriptor *ep_desc(struct usb_gadget *gadget,

/*-------------------------------------------------------------------------*/

+static void printer_dev_free(struct kref *kref)
+{
+ struct printer_dev *dev = container_of(kref, struct printer_dev, kref);
+
+ kfree(dev);
+}
+
static struct usb_request *
printer_req_alloc(struct usb_ep *ep, unsigned len, gfp_t gfp_flags)
{
@@ -348,6 +356,7 @@ printer_open(struct inode *inode, struct file *fd)

spin_unlock_irqrestore(&dev->lock, flags);

+ kref_get(&dev->kref);
DBG(dev, "printer_open returned %x\n", ret);
return ret;
}
@@ -365,6 +374,7 @@ printer_close(struct inode *inode, struct file *fd)
dev->printer_status &= ~PRINTER_SELECTED;
spin_unlock_irqrestore(&dev->lock, flags);

+ kref_put(&dev->kref, printer_dev_free);
DBG(dev, "printer_close\n");

return 0;
@@ -1350,7 +1360,8 @@ static void gprinter_free(struct usb_function *f)
struct f_printer_opts *opts;

opts = container_of(f->fi, struct f_printer_opts, func_inst);
- kfree(dev);
+
+ kref_put(&dev->kref, printer_dev_free);
mutex_lock(&opts->lock);
--opts->refcnt;
mutex_unlock(&opts->lock);
@@ -1419,6 +1430,7 @@ static struct usb_function *gprinter_alloc(struct usb_function_instance *fi)
return ERR_PTR(-ENOMEM);
}

+ kref_init(&dev->kref);
++opts->refcnt;
dev->minor = opts->minor;
dev->pnp_string = opts->pnp_string;
--
2.24.1


2020-06-08 07:35:17

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v2] usb: gadget: function: printer: fix use-after-free in __lock_acquire

On Mon, Jun 08, 2020 at 03:16:22PM +0800, [email protected] wrote:
> From: Zqiang <[email protected]>
>
> Increase the reference count of the printer dev through kref to avoid
> being released by other tasks when in use.
>
> BUG: KASAN: use-after-free in __lock_acquire+0x3fd4/0x4180
> kernel/locking/lockdep.c:3831
> Read of size 8 at addr ffff8880683b0018 by task syz-executor.0/3377
>
> CPU: 1 PID: 3377 Comm: syz-executor.0 Not tainted 5.6.11 #1
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
> Call Trace:
> __dump_stack lib/dump_stack.c:77 [inline]
> dump_stack+0xce/0x128 lib/dump_stack.c:118
> print_address_description.constprop.4+0x21/0x3c0 mm/kasan/report.c:374
> __kasan_report+0x131/0x1b0 mm/kasan/report.c:506
> kasan_report+0x12/0x20 mm/kasan/common.c:641
> __asan_report_load8_noabort+0x14/0x20 mm/kasan/generic_report.c:135
> __lock_acquire+0x3fd4/0x4180 kernel/locking/lockdep.c:3831
> lock_acquire+0x127/0x350 kernel/locking/lockdep.c:4488
> __raw_spin_lock_irqsave include/linux/spinlock_api_smp.h:110 [inline]
> _raw_spin_lock_irqsave+0x35/0x50 kernel/locking/spinlock.c:159
> printer_ioctl+0x4a/0x110 drivers/usb/gadget/function/f_printer.c:723
> vfs_ioctl fs/ioctl.c:47 [inline]
> ksys_ioctl+0xfb/0x130 fs/ioctl.c:763
> __do_sys_ioctl fs/ioctl.c:772 [inline]
> __se_sys_ioctl fs/ioctl.c:770 [inline]
> __x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:770
> do_syscall_64+0x9e/0x510 arch/x86/entry/common.c:294
> entry_SYSCALL_64_after_hwframe+0x49/0xbe
> RIP: 0033:0x4531a9
> Code: ed 60 fc ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48
> 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d
> 01 f0 ff ff 0f 83 bb 60 fc ff c3 66 2e 0f 1f 84 00 00 00 00
> RSP: 002b:00007fd14ad72c78 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
> RAX: ffffffffffffffda RBX: 000000000073bfa8 RCX: 00000000004531a9
> RDX: fffffffffffffff9 RSI: 000000000000009e RDI: 0000000000000003
> RBP: 0000000000000003 R08: 0000000000000000 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000246 R12: 00000000004bbd61
> R13: 00000000004d0a98 R14: 00007fd14ad736d4 R15: 00000000ffffffff
>
> Allocated by task 2393:
> save_stack+0x21/0x90 mm/kasan/common.c:72
> set_track mm/kasan/common.c:80 [inline]
> __kasan_kmalloc.constprop.3+0xa7/0xd0 mm/kasan/common.c:515
> kasan_kmalloc+0x9/0x10 mm/kasan/common.c:529
> kmem_cache_alloc_trace+0xfa/0x2d0 mm/slub.c:2813
> kmalloc include/linux/slab.h:555 [inline]
> kzalloc include/linux/slab.h:669 [inline]
> gprinter_alloc+0xa1/0x870 drivers/usb/gadget/function/f_printer.c:1416
> usb_get_function+0x58/0xc0 drivers/usb/gadget/functions.c:61
> config_usb_cfg_link+0x1ed/0x3e0 drivers/usb/gadget/configfs.c:444
> configfs_symlink+0x527/0x11d0 fs/configfs/symlink.c:202
> vfs_symlink+0x33d/0x5b0 fs/namei.c:4201
> do_symlinkat+0x11b/0x1d0 fs/namei.c:4228
> __do_sys_symlinkat fs/namei.c:4242 [inline]
> __se_sys_symlinkat fs/namei.c:4239 [inline]
> __x64_sys_symlinkat+0x73/0xb0 fs/namei.c:4239
> do_syscall_64+0x9e/0x510 arch/x86/entry/common.c:294
> entry_SYSCALL_64_after_hwframe+0x49/0xbe
>
> Freed by task 3368:
> save_stack+0x21/0x90 mm/kasan/common.c:72
> set_track mm/kasan/common.c:80 [inline]
> kasan_set_free_info mm/kasan/common.c:337 [inline]
> __kasan_slab_free+0x135/0x190 mm/kasan/common.c:476
> kasan_slab_free+0xe/0x10 mm/kasan/common.c:485
> slab_free_hook mm/slub.c:1444 [inline]
> slab_free_freelist_hook mm/slub.c:1477 [inline]
> slab_free mm/slub.c:3034 [inline]
> kfree+0xf7/0x410 mm/slub.c:3995
> gprinter_free+0x49/0xd0 drivers/usb/gadget/function/f_printer.c:1353
> usb_put_function+0x38/0x50 drivers/usb/gadget/functions.c:87
> config_usb_cfg_unlink+0x2db/0x3b0 drivers/usb/gadget/configfs.c:485
> configfs_unlink+0x3b9/0x7f0 fs/configfs/symlink.c:250
> vfs_unlink+0x287/0x570 fs/namei.c:4073
> do_unlinkat+0x4f9/0x620 fs/namei.c:4137
> __do_sys_unlink fs/namei.c:4184 [inline]
> __se_sys_unlink fs/namei.c:4182 [inline]
> __x64_sys_unlink+0x42/0x50 fs/namei.c:4182
> do_syscall_64+0x9e/0x510 arch/x86/entry/common.c:294
> entry_SYSCALL_64_after_hwframe+0x49/0xbe
>
> The buggy address belongs to the object at ffff8880683b0000
> which belongs to the cache kmalloc-1k of size 1024
> The buggy address is located 24 bytes inside of
> 1024-byte region [ffff8880683b0000, ffff8880683b0400)
> The buggy address belongs to the page:
> page:ffffea0001a0ec00 refcount:1 mapcount:0 mapping:ffff88806c00e300
> index:0xffff8880683b1800 compound_mapcount: 0
> flags: 0x100000000010200(slab|head)
> raw: 0100000000010200 0000000000000000 0000000600000001 ffff88806c00e300
> raw: ffff8880683b1800 000000008010000a 00000001ffffffff 0000000000000000
> page dumped because: kasan: bad access detected
>
> Reported-by: Kyungtae Kim <[email protected]>
> Signed-off-by: Zqiang <[email protected]>
> ---
> v1->v2:
> Commit information modification.
>
> drivers/usb/gadget/function/f_printer.c | 16 ++++++++++++++--
> 1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/gadget/function/f_printer.c b/drivers/usb/gadget/function/f_printer.c
> index 9c7ed2539ff7..8ed1295d7e35 100644
> --- a/drivers/usb/gadget/function/f_printer.c
> +++ b/drivers/usb/gadget/function/f_printer.c
> @@ -31,6 +31,7 @@
> #include <linux/types.h>
> #include <linux/ctype.h>
> #include <linux/cdev.h>
> +#include <linux/kref.h>
>
> #include <asm/byteorder.h>
> #include <linux/io.h>
> @@ -64,7 +65,7 @@ struct printer_dev {
> struct usb_gadget *gadget;
> s8 interface;
> struct usb_ep *in_ep, *out_ep;
> -
> + struct kref kref;

Now you have two referenced counted objects in the same exact structure.
Which one is the "correct" reference count?

You can't do this, that way lies madness and a sure set of nasty bugs
that will haunt you for forever.

Only use one reference count, either fix the existing one, or remove it
and use this kref.

But why not just use a struct device? Putting a kref at the end of a
device chain feels really really odd, don't you think?



> struct list_head rx_reqs; /* List of free RX structs */
> struct list_head rx_reqs_active; /* List of Active RX xfers */
> struct list_head rx_buffers; /* List of completed xfers */
> @@ -218,6 +219,13 @@ static inline struct usb_endpoint_descriptor *ep_desc(struct usb_gadget *gadget,
>
> /*-------------------------------------------------------------------------*/
>
> +static void printer_dev_free(struct kref *kref)
> +{
> + struct printer_dev *dev = container_of(kref, struct printer_dev, kref);
> +
> + kfree(dev);
> +}
> +
> static struct usb_request *
> printer_req_alloc(struct usb_ep *ep, unsigned len, gfp_t gfp_flags)
> {
> @@ -348,6 +356,7 @@ printer_open(struct inode *inode, struct file *fd)
>
> spin_unlock_irqrestore(&dev->lock, flags);
>
> + kref_get(&dev->kref);
> DBG(dev, "printer_open returned %x\n", ret);
> return ret;
> }
> @@ -365,6 +374,7 @@ printer_close(struct inode *inode, struct file *fd)
> dev->printer_status &= ~PRINTER_SELECTED;
> spin_unlock_irqrestore(&dev->lock, flags);
>
> + kref_put(&dev->kref, printer_dev_free);
> DBG(dev, "printer_close\n");
>
> return 0;
> @@ -1350,7 +1360,8 @@ static void gprinter_free(struct usb_function *f)
> struct f_printer_opts *opts;
>
> opts = container_of(f->fi, struct f_printer_opts, func_inst);
> - kfree(dev);
> +
> + kref_put(&dev->kref, printer_dev_free);
> mutex_lock(&opts->lock);
> --opts->refcnt;
> mutex_unlock(&opts->lock);
> @@ -1419,6 +1430,7 @@ static struct usb_function *gprinter_alloc(struct usb_function_instance *fi)
> return ERR_PTR(-ENOMEM);
> }
>
> + kref_init(&dev->kref);
> ++opts->refcnt;
> dev->minor = opts->minor;
> dev->pnp_string = opts->pnp_string;

All you are doing here is mirroring the existing cdev reference count,
what does this really solve? Are you sure it is correct? If so, why?

thanks,

greg k-h

2020-06-09 10:50:32

by Zhang, Qiang

[permalink] [raw]
Subject: 回复: [PATCH v2] usb: gadget: function: print er: fix use-after-free in __lock_acquire

Thank you for your suggestion
two referenced counted objects in the same exact structure. another referenced is
"dev->printer_cdev_open"?

________________________________________
??????: Greg KH <[email protected]>
????ʱ??: 2020??6??8?? 15:33
?ռ???: Zhang, Qiang
????: [email protected]; [email protected]; [email protected]
????: Re: [PATCH v2] usb: gadget: function: printer: fix use-after-free in __lock_acquire

On Mon, Jun 08, 2020 at 03:16:22PM +0800, [email protected] wrote:
> From: Zqiang <[email protected]>
>
> Increase the reference count of the printer dev through kref to avoid
> being released by other tasks when in use.
>
> BUG: KASAN: use-after-free in __lock_acquire+0x3fd4/0x4180
> kernel/locking/lockdep.c:3831
> Read of size 8 at addr ffff8880683b0018 by task syz-executor.0/3377
>
> CPU: 1 PID: 3377 Comm: syz-executor.0 Not tainted 5.6.11 #1
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
> Call Trace:
> __dump_stack lib/dump_stack.c:77 [inline]
> dump_stack+0xce/0x128 lib/dump_stack.c:118
> print_address_description.constprop.4+0x21/0x3c0 mm/kasan/report.c:374
> __kasan_report+0x131/0x1b0 mm/kasan/report.c:506
> kasan_report+0x12/0x20 mm/kasan/common.c:641
> __asan_report_load8_noabort+0x14/0x20 mm/kasan/generic_report.c:135
> __lock_acquire+0x3fd4/0x4180 kernel/locking/lockdep.c:3831
> lock_acquire+0x127/0x350 kernel/locking/lockdep.c:4488
> __raw_spin_lock_irqsave include/linux/spinlock_api_smp.h:110 [inline]
> _raw_spin_lock_irqsave+0x35/0x50 kernel/locking/spinlock.c:159
> printer_ioctl+0x4a/0x110 drivers/usb/gadget/function/f_printer.c:723
> vfs_ioctl fs/ioctl.c:47 [inline]
> ksys_ioctl+0xfb/0x130 fs/ioctl.c:763
> __do_sys_ioctl fs/ioctl.c:772 [inline]
> __se_sys_ioctl fs/ioctl.c:770 [inline]
> __x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:770
> do_syscall_64+0x9e/0x510 arch/x86/entry/common.c:294
> entry_SYSCALL_64_after_hwframe+0x49/0xbe
> RIP: 0033:0x4531a9
> Code: ed 60 fc ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48
> 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d
> 01 f0 ff ff 0f 83 bb 60 fc ff c3 66 2e 0f 1f 84 00 00 00 00
> RSP: 002b:00007fd14ad72c78 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
> RAX: ffffffffffffffda RBX: 000000000073bfa8 RCX: 00000000004531a9
> RDX: fffffffffffffff9 RSI: 000000000000009e RDI: 0000000000000003
> RBP: 0000000000000003 R08: 0000000000000000 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000246 R12: 00000000004bbd61
> R13: 00000000004d0a98 R14: 00007fd14ad736d4 R15: 00000000ffffffff
>
> Allocated by task 2393:
> save_stack+0x21/0x90 mm/kasan/common.c:72
> set_track mm/kasan/common.c:80 [inline]
> __kasan_kmalloc.constprop.3+0xa7/0xd0 mm/kasan/common.c:515
> kasan_kmalloc+0x9/0x10 mm/kasan/common.c:529
> kmem_cache_alloc_trace+0xfa/0x2d0 mm/slub.c:2813
> kmalloc include/linux/slab.h:555 [inline]
> kzalloc include/linux/slab.h:669 [inline]
> gprinter_alloc+0xa1/0x870 drivers/usb/gadget/function/f_printer.c:1416
> usb_get_function+0x58/0xc0 drivers/usb/gadget/functions.c:61
> config_usb_cfg_link+0x1ed/0x3e0 drivers/usb/gadget/configfs.c:444
> configfs_symlink+0x527/0x11d0 fs/configfs/symlink.c:202
> vfs_symlink+0x33d/0x5b0 fs/namei.c:4201
> do_symlinkat+0x11b/0x1d0 fs/namei.c:4228
> __do_sys_symlinkat fs/namei.c:4242 [inline]
> __se_sys_symlinkat fs/namei.c:4239 [inline]
> __x64_sys_symlinkat+0x73/0xb0 fs/namei.c:4239
> do_syscall_64+0x9e/0x510 arch/x86/entry/common.c:294
> entry_SYSCALL_64_after_hwframe+0x49/0xbe
>
> Freed by task 3368:
> save_stack+0x21/0x90 mm/kasan/common.c:72
> set_track mm/kasan/common.c:80 [inline]
> kasan_set_free_info mm/kasan/common.c:337 [inline]
> __kasan_slab_free+0x135/0x190 mm/kasan/common.c:476
> kasan_slab_free+0xe/0x10 mm/kasan/common.c:485
> slab_free_hook mm/slub.c:1444 [inline]
> slab_free_freelist_hook mm/slub.c:1477 [inline]
> slab_free mm/slub.c:3034 [inline]
> kfree+0xf7/0x410 mm/slub.c:3995
> gprinter_free+0x49/0xd0 drivers/usb/gadget/function/f_printer.c:1353
> usb_put_function+0x38/0x50 drivers/usb/gadget/functions.c:87
> config_usb_cfg_unlink+0x2db/0x3b0 drivers/usb/gadget/configfs.c:485
> configfs_unlink+0x3b9/0x7f0 fs/configfs/symlink.c:250
> vfs_unlink+0x287/0x570 fs/namei.c:4073
> do_unlinkat+0x4f9/0x620 fs/namei.c:4137
> __do_sys_unlink fs/namei.c:4184 [inline]
> __se_sys_unlink fs/namei.c:4182 [inline]
> __x64_sys_unlink+0x42/0x50 fs/namei.c:4182
> do_syscall_64+0x9e/0x510 arch/x86/entry/common.c:294
> entry_SYSCALL_64_after_hwframe+0x49/0xbe
>
> The buggy address belongs to the object at ffff8880683b0000
> which belongs to the cache kmalloc-1k of size 1024
> The buggy address is located 24 bytes inside of
> 1024-byte region [ffff8880683b0000, ffff8880683b0400)
> The buggy address belongs to the page:
> page:ffffea0001a0ec00 refcount:1 mapcount:0 mapping:ffff88806c00e300
> index:0xffff8880683b1800 compound_mapcount: 0
> flags: 0x100000000010200(slab|head)
> raw: 0100000000010200 0000000000000000 0000000600000001 ffff88806c00e300
> raw: ffff8880683b1800 000000008010000a 00000001ffffffff 0000000000000000
> page dumped because: kasan: bad access detected
>
> Reported-by: Kyungtae Kim <[email protected]>
> Signed-off-by: Zqiang <[email protected]>
> ---
> v1->v2:
> Commit information modification.
>
> drivers/usb/gadget/function/f_printer.c | 16 ++++++++++++++--
> 1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/gadget/function/f_printer.c b/drivers/usb/gadget/function/f_printer.c
> index 9c7ed2539ff7..8ed1295d7e35 100644
> --- a/drivers/usb/gadget/function/f_printer.c
> +++ b/drivers/usb/gadget/function/f_printer.c
> @@ -31,6 +31,7 @@
> #include <linux/types.h>
> #include <linux/ctype.h>
> #include <linux/cdev.h>
> +#include <linux/kref.h>
>
> #include <asm/byteorder.h>
> #include <linux/io.h>
> @@ -64,7 +65,7 @@ struct printer_dev {
> struct usb_gadget *gadget;
> s8 interface;
> struct usb_ep *in_ep, *out_ep;
> -
> + struct kref kref;

Now you have two referenced counted objects in the same exact structure.
Which one is the "correct" reference count?

You can't do this, that way lies madness and a sure set of nasty bugs
that will haunt you for forever.

Only use one reference count, either fix the existing one, or remove it
and use this kref.

But why not just use a struct device? Putting a kref at the end of a
device chain feels really really odd, don't you think?



> struct list_head rx_reqs; /* List of free RX structs */
> struct list_head rx_reqs_active; /* List of Active RX xfers */
> struct list_head rx_buffers; /* List of completed xfers */
> @@ -218,6 +219,13 @@ static inline struct usb_endpoint_descriptor *ep_desc(struct usb_gadget *gadget,
>
> /*-------------------------------------------------------------------------*/
>
> +static void printer_dev_free(struct kref *kref)
> +{
> + struct printer_dev *dev = container_of(kref, struct printer_dev, kref);
> +
> + kfree(dev);
> +}
> +
> static struct usb_request *
> printer_req_alloc(struct usb_ep *ep, unsigned len, gfp_t gfp_flags)
> {
> @@ -348,6 +356,7 @@ printer_open(struct inode *inode, struct file *fd)
>
> spin_unlock_irqrestore(&dev->lock, flags);
>
> + kref_get(&dev->kref);
> DBG(dev, "printer_open returned %x\n", ret);
> return ret;
> }
> @@ -365,6 +374,7 @@ printer_close(struct inode *inode, struct file *fd)
> dev->printer_status &= ~PRINTER_SELECTED;
> spin_unlock_irqrestore(&dev->lock, flags);
>
> + kref_put(&dev->kref, printer_dev_free);
> DBG(dev, "printer_close\n");
>
> return 0;
> @@ -1350,7 +1360,8 @@ static void gprinter_free(struct usb_function *f)
> struct f_printer_opts *opts;
>
> opts = container_of(f->fi, struct f_printer_opts, func_inst);
> - kfree(dev);
> +
> + kref_put(&dev->kref, printer_dev_free);
> mutex_lock(&opts->lock);
> --opts->refcnt;
> mutex_unlock(&opts->lock);
> @@ -1419,6 +1430,7 @@ static struct usb_function *gprinter_alloc(struct usb_function_instance *fi)
> return ERR_PTR(-ENOMEM);
> }
>
> + kref_init(&dev->kref);
> ++opts->refcnt;
> dev->minor = opts->minor;
> dev->pnp_string = opts->pnp_string;

All you are doing here is mirroring the existing cdev reference count,
what does this really solve? Are you sure it is correct? If so, why?

thanks,

greg k-h