* [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 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-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 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-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 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-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* [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-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
* 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-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
* 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-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