All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] media: go7007: code improvment and bug fixes
@ 2021-06-20 19:44 ` Pavel Skripkin
  0 siblings, 0 replies; 8+ messages in thread
From: Pavel Skripkin @ 2021-06-20 19:44 UTC (permalink / raw)
  To: hverkuil-cisco, mchehab, oneukum, gregkh
  Cc: linux-media, linux-kernel, linux-kernel-mentees, Pavel Skripkin

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(-)

-- 
2.32.0


^ permalink raw reply	[flat|nested] 8+ messages in thread
* [PATCH 0/2] media: go7007: code improvment and bug fixes
@ 2021-06-20 19:44 ` Pavel Skripkin
  0 siblings, 0 replies; 8+ messages in thread
From: Pavel Skripkin @ 2021-06-20 19:44 UTC (permalink / raw)
  To: hverkuil-cisco, mchehab, oneukum, gregkh
  Cc: linux-kernel-mentees, linux-kernel, linux-media

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(-)

-- 
2.32.0

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply	[flat|nested] 8+ messages in thread
* [PATCH 1/2] media: go7007: fix memory leak in go7007_usb_probe
  2021-06-20 19:44 ` Pavel Skripkin
@ 2021-06-20 19:45   ` Pavel Skripkin
  -1 siblings, 0 replies; 8+ messages in thread
From: Pavel Skripkin @ 2021-06-20 19:45 UTC (permalink / raw)
  To: hverkuil-cisco, mchehab, oneukum, gregkh
  Cc: linux-media, linux-kernel, linux-kernel-mentees, Pavel Skripkin

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

Fixes: 137641287eb4 ("go7007: add sanity checking for endpoints")
Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
---
 drivers/media/usb/go7007/go7007-usb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/usb/go7007/go7007-usb.c b/drivers/media/usb/go7007/go7007-usb.c
index dbf0455d5d50..eeb85981e02b 100644
--- a/drivers/media/usb/go7007/go7007-usb.c
+++ b/drivers/media/usb/go7007/go7007-usb.c
@@ -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);
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread
* [PATCH 1/2] media: go7007: fix memory leak in go7007_usb_probe
@ 2021-06-20 19:45   ` Pavel Skripkin
  0 siblings, 0 replies; 8+ messages in thread
From: Pavel Skripkin @ 2021-06-20 19:45 UTC (permalink / raw)
  To: hverkuil-cisco, mchehab, oneukum, gregkh
  Cc: linux-kernel-mentees, linux-kernel, linux-media

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

Fixes: 137641287eb4 ("go7007: add sanity checking for endpoints")
Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
---
 drivers/media/usb/go7007/go7007-usb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/usb/go7007/go7007-usb.c b/drivers/media/usb/go7007/go7007-usb.c
index dbf0455d5d50..eeb85981e02b 100644
--- a/drivers/media/usb/go7007/go7007-usb.c
+++ b/drivers/media/usb/go7007/go7007-usb.c
@@ -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);
-- 
2.32.0

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply related	[flat|nested] 8+ messages in thread
* [PATCH 2/2] media: go7007: remove redundant initialization
  2021-06-20 19:44 ` Pavel Skripkin
@ 2021-06-20 19:45   ` Pavel Skripkin
  -1 siblings, 0 replies; 8+ messages in thread
From: Pavel Skripkin @ 2021-06-20 19:45 UTC (permalink / raw)
  To: hverkuil-cisco, mchehab, oneukum, gregkh
  Cc: linux-media, linux-kernel, linux-kernel-mentees, Pavel Skripkin

In 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();

Fixes: 866b8695d67e8 ("Staging: add the go7007 video driver")
Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
---
 drivers/media/usb/go7007/go7007-driver.c | 26 ------------------------
 1 file changed, 26 deletions(-)

diff --git a/drivers/media/usb/go7007/go7007-driver.c b/drivers/media/usb/go7007/go7007-driver.c
index f1767be9d868..6650eab913d8 100644
--- a/drivers/media/usb/go7007/go7007-driver.c
+++ b/drivers/media/usb/go7007/go7007-driver.c
@@ -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;
 }
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread
* [PATCH 2/2] media: go7007: remove redundant initialization
@ 2021-06-20 19:45   ` Pavel Skripkin
  0 siblings, 0 replies; 8+ messages in thread
From: Pavel Skripkin @ 2021-06-20 19:45 UTC (permalink / raw)
  To: hverkuil-cisco, mchehab, oneukum, gregkh
  Cc: linux-kernel-mentees, linux-kernel, linux-media

In 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();

Fixes: 866b8695d67e8 ("Staging: add the go7007 video driver")
Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
---
 drivers/media/usb/go7007/go7007-driver.c | 26 ------------------------
 1 file changed, 26 deletions(-)

diff --git a/drivers/media/usb/go7007/go7007-driver.c b/drivers/media/usb/go7007/go7007-driver.c
index f1767be9d868..6650eab913d8 100644
--- a/drivers/media/usb/go7007/go7007-driver.c
+++ b/drivers/media/usb/go7007/go7007-driver.c
@@ -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;
 }
-- 
2.32.0

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply related	[flat|nested] 8+ messages in thread
* Re: [PATCH 0/2] media: go7007: code improvment and bug fixes
  2021-06-20 19:44 ` Pavel Skripkin
@ 2021-07-06 17:15   ` Pavel Skripkin
  -1 siblings, 0 replies; 8+ messages in thread
From: Pavel Skripkin @ 2021-07-06 17:15 UTC (permalink / raw)
  To: hverkuil-cisco, mchehab, oneukum, gregkh
  Cc: linux-media, linux-kernel, linux-kernel-mentees

On Sun, 20 Jun 2021 22:44:33 +0300
Pavel Skripkin <paskripkin@gmail.com> 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.
> 
> 
> 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(-)
> 

Hi, media developers!

Gentle ping :)



With regards,
Pavel Skripkin

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH 0/2] media: go7007: code improvment and bug fixes
@ 2021-07-06 17:15   ` Pavel Skripkin
  0 siblings, 0 replies; 8+ messages in thread
From: Pavel Skripkin @ 2021-07-06 17:15 UTC (permalink / raw)
  To: hverkuil-cisco, mchehab, oneukum, gregkh
  Cc: linux-kernel-mentees, linux-kernel, linux-media

On Sun, 20 Jun 2021 22:44:33 +0300
Pavel Skripkin <paskripkin@gmail.com> 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.
> 
> 
> 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(-)
> 

Hi, media developers!

Gentle ping :)



With regards,
Pavel Skripkin
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply	[flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-07-06 17:15 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-20 19:44 [PATCH 0/2] media: go7007: code improvment and bug fixes Pavel Skripkin
2021-06-20 19:44 ` Pavel Skripkin
2021-06-20 19:45 ` [PATCH 1/2] media: go7007: fix memory leak in go7007_usb_probe Pavel Skripkin
2021-06-20 19:45   ` Pavel Skripkin
2021-06-20 19:45 ` [PATCH 2/2] media: go7007: remove redundant initialization Pavel Skripkin
2021-06-20 19:45   ` Pavel Skripkin
2021-07-06 17:15 ` [PATCH 0/2] media: go7007: code improvment and bug fixes Pavel Skripkin
2021-07-06 17:15   ` Pavel Skripkin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.