linux-kernel - Re: [PATCH] media:vivid: add sanity check to avoid divide error and set value to 1 if 0.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <a3d0ddaf-cd84-73c5-a3fc-92c3d47dc5b7@gmail.com>
Date:   Thu, 27 Jun 2019 13:50:14 +0530
From:   Vandana BN <bnvandana@...il.com>
To:     Hans Verkuil <hverkuil@...all.nl>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        linux-media@...r.kernel.org, linux-kernel@...r.kernel.org
Cc:     skhan@...uxfoundation.org, gregkh@...uxfoundation.org,
        linux-kernel-mentees@...ts.linuxfoundation.org
Subject: Re: [PATCH] media:vivid: add sanity check to avoid divide error and
 set value to 1 if 0.


On 27/06/19 12:58 PM, Hans Verkuil wrote:
> On 6/26/19 8:01 AM, Vandana BN wrote:
>> Syzbot reported divide error in vivid_thread_vid_cap, which has been
>> seen only once and doesnot have a reproducer.
>> This patch sanity checks for the denominator value with WARN_ON if it is 0 and replaces it with 1.
>>
>> Reported-by: syz...@...kaller.appspotmail.com
>>
>> divide error: 0000 [#1] PREEMPT SMP KASAN
>> kobject: 'tx-0' (0000000017161f7f): kobject_uevent_env
>> CPU: 0 PID: 23689 Comm: vivid-003-vid-c Not tainted 5.0.0-rc4+ #58
>> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
>> Google 01/01/2011
>> RIP: 0010:vivid_cap_update_frame_period
>> drivers/media/platform/vivid/vivid-kthread-cap.c:661 [inline]
>> RIP: 0010:vivid_thread_vid_cap+0x221/0x284
>> drivers/media/platform/vivid/vivid-kthread-cap.c:789
>> Code: 48 c1 e9 03 0f b6 0c 11 48 89 f2 48 69 c0 00 ca 9a 3b 83 c2 03 38
>> ca
>> 7c 08 84 c9 0f 85 f0 1e 00 00 41 8b 8f 24 64 00 00 31 d2 <48> f7 f1 49
>> 89
>> c4 48 89 c3 49 8d 87 28 64 00 00 48 89 c2 48 89 45
>> RSP: 0018:ffff88808b4afd68 EFLAGS: 00010246
>> kobject: 'tx-0' (0000000017161f7f): fill_kobj_path: path
>> = '/devices/virtual/net/gre0/queues/tx-0'
>> RAX: 000000de5a6f8e00 RBX: 0000000100047b22 RCX: 0000000000000000
>> RDX: 0000000000000000 RSI: 0000000000000004 RDI: 0000000000000004
>> RBP: ffff88808b4aff00 R08: ffff88804862e1c0 R09: ffffffff89997008
>> R10: ffffffff89997010 R11: 0000000000000001 R12: 00000000fffffffc
>> R13: ffff8880a17e0500 R14: ffff88803e40f760 R15: ffff8882182b0140
>> FS:  0000000000000000(0000) GS:ffff8880ae800000(0000)
>> knlGS:0000000000000000
>> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> CR2: 00000000004cdc90 CR3: 000000005d827000 CR4: 00000000001426f0
>> Call Trace:
>> kobject: 'gretap0' (00000000d7549098): kobject_add_internal: parent:
>> 'net',
>> set: 'devices'
>> kobject: 'loop2' (0000000094ed4ee4): kobject_uevent_env
>> kobject: 'loop2' (0000000094ed4ee4): fill_kobj_path: path
>> = '/devices/virtual/block/loop2'
>>   kthread+0x357/0x430 kernel/kthread.c:246
>> kobject: 'gretap0' (00000000d7549098): kobject_uevent_env
>>   ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:352
>> Modules linked in:
>> kobject: 'gretap0' (00000000d7549098): fill_kobj_path: path
>> = '/devices/virtual/net/gretap0'
>> ---[ end trace bc5c8b25b64d768f ]---
>> kobject: 'loop1' (0000000032036b86): kobject_uevent_env
>> RIP: 0010:vivid_cap_update_frame_period
>> drivers/media/platform/vivid/vivid-kthread-cap.c:661 [inline]
>> RIP: 0010:vivid_thread_vid_cap+0x221/0x2840
>> drivers/media/platform/vivid/vivid-kthread-cap.c:789
>> kobject: 'loop1' (0000000032036b86): fill_kobj_path: path
>> = '/devices/virtual/block/loop1'
>> Code: 48 c1 e9 03 0f b6 0c 11 48 89 f2 48 69 c0 00 ca 9a 3b 83 c2 03 38
>> ca
>> 7c 08 84 c9 0f 85 f0 1e 00 00 41 8b 8f 24 64 00 00 31 d2 <48> f7 f1 49
>> 89
>> c4 48 89 c3 49 8d 87 28 64 00 00 48 89 c2 48 89 45
>> kobject: 'loop0' (00000000dd9927c3): kobject_uevent_env
>> RSP: 0018:ffff88808b4afd68 EFLAGS: 00010246
>> RAX: 000000de5a6f8e00 RBX: 0000000100047b22 RCX: 0000000000000000
>> kobject: 'queues' (000000007ed20666): kobject_add_internal:
>> parent: 'gretap0', set: '<NULL>'
>> RDX: 0000000000000000 RSI: 0000000000000004 RDI: 0000000000000004
>> RBP: ffff88808b4aff00 R08: ffff88804862e1c0 R09: ffffffff89997008
>> kobject: 'loop0' (00000000dd9927c3): fill_kobj_path: path
>> = '/devices/virtual/block/loop0'
>> R10: ffffffff89997010 R11: 0000000000000001 R12: 00000000fffffffc
>> kobject: 'queues' (000000007ed20666): kobject_uevent_env
>> R13: ffff8880a17e0500 R14: ffff88803e40f760 R15: ffff8882182b0140
>> FS:  0000000000000000(0000) GS:ffff8880ae800000(0000)
>> knlGS:0000000000000000
>> kobject: 'loop5' (00000000a41f9e79): kobject_uevent_env
>> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> kobject: 'queues' (000000007ed20666): kobject_uevent_env: filter
>> function
>> caused the event to drop!
>> CR2: 00000000004cdc90 CR3: 000000005d827000 CR4: 00000000001426f0
>> kobject: 'loop5' (00000000a41f9e79): fill_kobj_path: path
>> = '/devices/virtual/block/loop5'
>>
>> Signed-off-by: Vandana BN <bnvandana@...il.com>
>> ---
>>  drivers/media/platform/vivid/vivid-kthread-cap.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/media/platform/vivid/vivid-kthread-cap.c b/drivers/media/platform/vivid/vivid-kthread-cap.c
>> index f8006a30c12f..025a8c68bd1c 100644
>> --- a/drivers/media/platform/vivid/vivid-kthread-cap.c
>> +++ b/drivers/media/platform/vivid/vivid-kthread-cap.c
>> @@ -658,6 +658,8 @@ static void vivid_cap_update_frame_period(struct vivid_dev *dev)
>>  	u64 f_period;
>>  
>>  	f_period = (u64)dev->timeperframe_vid_cap.numerator * 1000000000;
>> +	if(WARN_ON(dev->timeperframe_vid_cap.denominator == 0))
>> +		dev->timeperframe_vid_cap.denominator = 1;
>>  	do_div(f_period, dev->timeperframe_vid_cap.denominator);
>>  	if (dev->field_cap == V4L2_FIELD_ALTERNATE)
>>  		f_period >>= 1;
>>
> Always run the patch over 'checkpatch.pl --strict'. It gives me:
>
> WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per line)
> #83:
> This patch sanity checks for the denominator value with WARN_ON if it is 0 and replaces it with 1.
>
> ERROR: space required before the open parenthesis '('
> #175: FILE: drivers/media/platform/vivid/vivid-kthread-cap.c:661:
> +       if(WARN_ON(dev->timeperframe_vid_cap.denominator == 0))
>
> total: 1 errors, 1 warnings, 0 checks, 8 lines checked
>
> NOTE: For some of the reported defects, checkpatch may be able to
>       mechanically convert to the typical style using --fix or --fix-inplace.
>
> /home/hans/x.eml has style problems, please review.
>
> NOTE: If any of the errors are false positives, please report
>       them to the maintainer, see CHECKPATCH in MAINTAINERS.
>
> Regards,
>
> 	Hans

Thanks Hans,

Sorry had missed running checkpatch, Will send a updated patch.

Regards,

Vandana.