media: go7007: code improvment and bug fixes
Subsystems (2)
cover media: go7007: code improvment and bug fixes
In this patch series, I fixed memory leak reported by my local syzkaller instance and increased go7007_alloc() execution speed by removing reduntant zeroing members in kzalloc allocated structure.
Pavel Skripkin (2):
media: go7007: fix memory leak in go7007_usb_probe
media: go7007: remove redundant initialization
drivers/media/usb/go7007/go7007-driver.c | 26 ------------------------ drivers/media/usb/go7007/go7007-usb.c | 2 +- 2 files changed, 1 insertion(+), 27 deletions(-)
Signature · 1 line
2.32.0
PATCH 1/2 media: go7007: fix memory leak in go7007_usb_probe
In commit 137641287eb4 ("go7007: add sanity checking for endpoints") endpoint sanity check was introduced, but if check fails it simply returns with leaked pointers.
Cutted log from my local syzbot instance:
BUG: memory leak
unreferenced object 0xffff8880209f0000 (size 8192):
comm "kworker/0:4", pid 4916, jiffies 4295263583 (age 29.310s)
hex dump (first 32 bytes):
30 b0 27 22 80 88 ff ff 75 73 62 2d 64 75 6d 6d 0.'"....usb-dumm
79 5f 68 63 64 2e 33 2d 31 00 00 00 00 00 00 00 y_hcd.3-1.......
backtrace:
[<ffffffff860ca856>] kmalloc include/linux/slab.h:556 [inline]
[<ffffffff860ca856>] kzalloc include/linux/slab.h:686 [inline]
[<ffffffff860ca856>] go7007_alloc+0x46/0xb40 drivers/media/usb/go7007/go7007-driver.c:696
[<ffffffff860de74e>] go7007_usb_probe+0x13e/0x2200 drivers/media/usb/go7007/go7007-usb.c:1114
[<ffffffff854a5f74>] usb_probe_interface+0x314/0x7f0 drivers/usb/core/driver.c:396
[<ffffffff845a7151>] really_probe+0x291/0xf60 drivers/base/dd.c:576
BUG: memory leak
unreferenced object 0xffff88801e2f2800 (size 512):
comm "kworker/0:4", pid 4916, jiffies 4295263583 (age 29.310s)
hex dump (first 32 bytes):
00 87 40 8a ff ff ff ff 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:
[<ffffffff860de794>] kmalloc include/linux/slab.h:556 [inline]
[<ffffffff860de794>] kzalloc include/linux/slab.h:686 [inline]
[<ffffffff860de794>] go7007_usb_probe+0x184/0x2200 drivers/media/usb/go7007/go7007-usb.c:1118
[<ffffffff854a5f74>] usb_probe_interface+0x314/0x7f0 drivers/usb/core/driver.c:396
[<ffffffff845a7151>] really_probe+0x291/0xf60 drivers/base/dd.c:576--- drivers/media/usb/go7007/go7007-usb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/media/usb/go7007/go7007-usb.c +1 -1
@@ -1134,7 +1134,7 @@ static int go7007_usb_probe(struct usb_interface *intf, ep = usb->usbdev->ep_in[4 ];if (!ep) -return -ENODEV ; + goto allocfail;/* Allocate the URB and buffer for receiving incoming interrupts */ usb->intr_urb =usb_alloc_urb (0 ,GFP_KERNEL );
Signature · 1 line
2.32.0
PATCH 2/2 media: go7007: remove redundant initialization
In reply to Pavel SkripkinIn go7007_alloc() kzalloc() is used for struct go7007 allocation. It means that there is no need in zeroing any members, because kzalloc will take care of it.
Removing these reduntant initialization steps increases execution speed a lot:
Before:
+ 86.802 us | go7007_alloc();
After:
+ 29.595 us | go7007_alloc();
--- drivers/media/usb/go7007/go7007-driver.c | 26 ------------------------ 1 file changed, 26 deletions(-)
drivers/media/usb/go7007/go7007-driver.c +0 -26
@@ -691,49 +691,23 @@ struct go7007 *go7007_alloc(const struct go7007_board_info *board, struct device *dev) {struct go7007 *go; -int i; go =kzalloc (sizeof (struct go7007 ),GFP_KERNEL );if (go ==NULL )return NULL ; go->dev = dev; go->board_info = board; - go->board_id =0 ; go->tuner_type =-1 ; - go->channel_number =0 ; - go->name[0 ] =0 ;mutex_init (&go->hw_lock);init_waitqueue_head (&go->frame_waitq);spin_lock_init (&go->spinlock); go->status =STATUS_INIT ; -memset (&go->i2c_adapter,0 ,sizeof (go->i2c_adapter)); - go->i2c_adapter_online =0 ; - go->interrupt_available =0 ;init_waitqueue_head (&go->interrupt_waitq); - go->input =0 ;go7007_update_board (go); - go->encoder_h_halve =0 ; - go->encoder_v_halve =0 ; - go->encoder_subsample =0 ; go->format =V4L2_PIX_FMT_MJPEG ; go->bitrate =1500000 ; go->fps_scale =1 ; - go->pali =0 ; go->aspect_ratio =GO7007_RATIO_1_1 ; - go->gop_size =0 ; - go->ipb =0 ; - go->closed_gop =0 ; - go->repeat_seqhead =0 ; - go->seq_header_enable =0 ; - go->gop_header_enable =0 ; - go->dvd_mode =0 ; - go->interlace_coding =0 ; -for (i =0 ; i <4 ; ++i) - go->modet[i].enable =0 ; -for (i =0 ; i <1624 ; ++i) - go->modet_map[i] =0 ; - go->audio_deliver =NULL ; - go->audio_enabled =0 ;return go; }
Signature · 1 line
2.32.0
Quoted 6 lines · On Sun, 20 Jun 2021 22:44:33 +0300 Pavel Skripkin <[email protected]> wrote:
In this patch series, I fixed memory leak reported by my local syzkaller instance and increased
go7007_alloc()execution speed by removing reduntant zeroing members in kzalloc allocated structure.
Quoted 8 lines
Pavel Skripkin (2):
media: go7007: fix memory leak ingo7007_usb_probe
media: go7007: remove redundant initializationdrivers/media/usb/go7007/go7007-driver.c | 26------------------------
drivers/media/usb/go7007/go7007-usb.c|
2 +- 2 files changed, 1 insertion(+), 27 deletions(-)
Hi, media developers!
Gentle ping :)
With regards,
Pavel Skripkin