From: Kefeng Wang <wangkefeng.wang@huawei.com>
To: SeongJae Park <sj@kernel.org>
Cc: syzbot <syzbot+841a46899768ec7bec67@syzkaller.appspotmail.com>,
<akpm@linux-foundation.org>, <damon@lists.linux.dev>,
<linux-kernel@vger.kernel.org>, <linux-mm@kvack.org>,
<syzkaller-bugs@googlegroups.com>
Subject: Re: [syzbot] [damon?] divide error in damon_set_attrs
Date: Sat, 27 May 2023 09:15:01 +0800 [thread overview]
Message-ID: <c18dc1e9-1805-f366-9d16-30e9628ac14e@huawei.com> (raw)
In-Reply-To: <20230526185409.92039-1-sj@kernel.org>
On 2023/5/27 2:54, SeongJae Park wrote:
> Hi Kefeng and syzbot,
>
> On Fri, 26 May 2023 20:59:12 +0800 Kefeng Wang <wangkefeng.wang@huawei.com> wrote:
>
>>
>>
>> On 2023/5/26 19:51, syzbot wrote:
>>> Hello,
>>>
>>> syzbot found the following issue on:
>>>
>>> HEAD commit: 44c026a73be8 Linux 6.4-rc3
>>> git tree: upstream
>>> console output: https://syzkaller.appspot.com/x/log.txt?x=13a92b31280000
>>> kernel config: https://syzkaller.appspot.com/x/.config?x=f389ffdf4e9ba3f0
>>> dashboard link: https://syzkaller.appspot.com/bug?extid=841a46899768ec7bec67
>>> compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2
>>> userspace arch: i386
>>>
>>> Unfortunately, I don't have any reproducer for this issue yet.
>>>
>>> Downloadable assets:
>>> disk image: https://storage.googleapis.com/syzbot-assets/35f16ee05df7/disk-44c026a7.raw.xz
>>> vmlinux: https://storage.googleapis.com/syzbot-assets/10399498a570/vmlinux-44c026a7.xz
>>> kernel image: https://storage.googleapis.com/syzbot-assets/5c72201ea4ba/bzImage-44c026a7.xz
>>>
>>> IMPORTANT: if you fix the issue, please add the following tag to the commit:
>>> Reported-by: syzbot+841a46899768ec7bec67@syzkaller.appspotmail.com
>>>
>>> divide error: 0000 [#1] PREEMPT SMP KASAN
>>> CPU: 1 PID: 13527 Comm: syz-executor.1 Not tainted 6.4.0-rc3-syzkaller #0
>>> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 04/28/2023
>>> RIP: 0010:damon_nr_accesses_to_accesses_bp mm/damon/core.c:491 [inline]
>>> RIP: 0010:damon_nr_accesses_for_new_attrs mm/damon/core.c:497 [inline]
>>> RIP: 0010:damon_update_monitoring_result mm/damon/core.c:506 [inline]
>>> RIP: 0010:damon_update_monitoring_results mm/damon/core.c:534 [inline]
>>> RIP: 0010:damon_set_attrs+0x224/0x460 mm/damon/core.c:555
>
> Thank you for finding and reporting this bug!
>
> The code of the problem is as below:
>
> /* convert nr_accesses to access ratio in bp (per 10,000) */
> static unsigned int damon_nr_accesses_to_accesses_bp(
> unsigned int nr_accesses, struct damon_attrs *attrs)
> {
> unsigned int max_nr_accesses =
> attrs->aggr_interval / attrs->sample_interval;
>
> return nr_accesses * 10000 / max_nr_accesses;
> }
>
> The problem can happen when 'aggr_interval' is smaller than 'sample_interval',
> because 'max_nr_accesses' becomes zero in the case, and resulting in divide by
> zero.
>
> Same problem is in damon_accesses_bp_to_nr_accesses().
>
>>
>> make aggr_interval great than or equal sample_interval?
>>
>> diff --git a/mm/damon/core.c b/mm/damon/core.c
>> index d9ef62047bf5..6fe1960f3d6b 100644
>> --- a/mm/damon/core.c
>> +++ b/mm/damon/core.c
>> @@ -525,8 +525,8 @@ static void damon_update_monitoring_results(struct
>> damon_ctx *ctx,
>>
>> /* if any interval is zero, simply forgive conversion */
>> if (!old_attrs->sample_interval || !old_attrs->aggr_interval ||
>> - !new_attrs->sample_interval ||
>> - !new_attrs->aggr_interval)
>> + !new_attrs->sample_interval || !new_attrs->aggr_interval ||
>> + new_attrs->aggr_interval < new_attrs->sample_interval)
>> return;
>
> Nice and effective fix! Nevertheless, I think aggregation interval smaller
> than sample interval is just a wrong input. How about adding the check in
> damon_set_attrs()'s already existing attributes validation, like below?
Yes, move the check into damon_set_attrs() is better, and it seems that
we could move all the check into it, and drop the old_attrs check in
damon_update_monitoring_results(), what's you option?
diff --git a/mm/damon/core.c b/mm/damon/core.c
index d9ef62047bf5..1647f7f1f708 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -523,12 +523,6 @@ static void damon_update_monitoring_results(struct
damon_ctx *ctx,
struct damon_target *t;
struct damon_region *r;
- /* if any interval is zero, simply forgive conversion */
- if (!old_attrs->sample_interval || !old_attrs->aggr_interval ||
- !new_attrs->sample_interval ||
- !new_attrs->aggr_interval)
- return;
-
damon_for_each_target(t, ctx)
damon_for_each_region(r, t)
damon_update_monitoring_result(
@@ -551,6 +545,10 @@ int damon_set_attrs(struct damon_ctx *ctx, struct
damon_attrs *attrs)
return -EINVAL;
if (attrs->min_nr_regions > attrs->max_nr_regions)
return -EINVAL;
+ if (attrs->sample_interval > attrs->aggr_interval)
+ return -EINVAL;
+ if (!attrs->sample_interval || !attrs->aggr_interval)
+ return -EINVAL;
>
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -580,6 +580,8 @@ int damon_set_attrs(struct damon_ctx *ctx, struct damon_attrs *attrs)
> return -EINVAL;
> if (attrs->min_nr_regions > attrs->max_nr_regions)
> return -EINVAL;
> + if (attrs->aggr_interval < attrs->sample_interval)
> + return -EINVAL;
>
> damon_update_monitoring_results(ctx, attrs);
> ctx->attrs = *attrs;
>
> Thanks,
> SJ
next prev parent reply other threads:[~2023-05-27 1:34 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-05-26 11:51 [syzbot] [damon?] divide error in damon_set_attrs syzbot
2023-05-26 12:59 ` Kefeng Wang
2023-05-26 18:54 ` SeongJae Park
2023-05-26 19:35 ` SeongJae Park
2023-05-27 1:15 ` Kefeng Wang [this message]
2023-05-27 1:46 ` SeongJae Park
2023-05-27 2:02 ` Kefeng Wang
2023-05-27 2:08 ` SeongJae Park
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=c18dc1e9-1805-f366-9d16-30e9628ac14e@huawei.com \
--to=wangkefeng.wang@huawei.com \
--cc=akpm@linux-foundation.org \
--cc=damon@lists.linux.dev \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=sj@kernel.org \
--cc=syzbot+841a46899768ec7bec67@syzkaller.appspotmail.com \
--cc=syzkaller-bugs@googlegroups.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.