| Message ID | 20250104012037.159386-1-shr@devkernel.io (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v1] mm: fix div by zero in bdi_ratio_from_pages | expand |
On 04.01.25 02:20, Stefan Roesch wrote: > During testing it has been detected, that it is possible to get div by > zero error in bdi_set_min_bytes. The error is caused by the function > bdi_ratio_from_pages(). bdi_ratio_from_pages() calls > global_dirty_limits. If the dirty threshold is 0, the div by zero is > raised. This can happen if the root user is setting: > > echo 0 > /proc/sys/vm/dirty_ration. > > The following is a test case: > > echo 0 > /proc/sys/vm/dirty_ratio > cd /sys/class/bdi/<device> > echo 1 > strict_limit > echo 8192 > min_bytes > > ==> error is raised. > > The problem is addressed by returning -EINVAL if dirty_ratio or > dirty_bytes is set to 0. > > Reported-by: cheung wall <zzqq0103.hey@gmail.com> > Closes: https://lore.kernel.org/linux-mm/87pll35yd0.fsf@devkernel.io/T/#t > Signed-off-by: Stefan Roesch <shr@devkernel.io> > --- > mm/page-writeback.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/mm/page-writeback.c b/mm/page-writeback.c > index d213ead95675..91aa7a5c0078 100644 > --- a/mm/page-writeback.c > +++ b/mm/page-writeback.c > @@ -692,6 +692,8 @@ static unsigned long bdi_ratio_from_pages(unsigned long pages) > unsigned long ratio; > > global_dirty_limits(&background_thresh, &dirty_thresh); > + if (!dirty_thresh) > + return -EINVAL; > ratio = div64_u64(pages * 100ULL * BDI_RATIO_SCALE, dirty_thresh); > > return ratio; bdi_set_min_bytes() calls bdi_ratio_from_pages() and passes the result to __bdi_set_min_ratio(). __bdi_set_min_ratio() expects an "unsigned int min_ratio". I assume this will work because "max_ratio > 100 * BDI_RATIO_SCALE", but it is rather confusing ... Maybe we want something like: /* Use 101% to indicate "invalid" */ #define BDI_RATIO_INVALID (101 * BDI_RATIO_SCALE) Or alternatively, just handle it in the callers of bdi_ratio_from_pages(), checking for -EINVAL manually.
David Hildenbrand <david@redhat.com> writes: > On 04.01.25 02:20, Stefan Roesch wrote: >> During testing it has been detected, that it is possible to get div by >> zero error in bdi_set_min_bytes. The error is caused by the function >> bdi_ratio_from_pages(). bdi_ratio_from_pages() calls >> global_dirty_limits. If the dirty threshold is 0, the div by zero is >> raised. This can happen if the root user is setting: >> echo 0 > /proc/sys/vm/dirty_ration. >> The following is a test case: >> echo 0 > /proc/sys/vm/dirty_ratio >> cd /sys/class/bdi/<device> >> echo 1 > strict_limit >> echo 8192 > min_bytes >> ==> error is raised. >> The problem is addressed by returning -EINVAL if dirty_ratio or >> dirty_bytes is set to 0. >> Reported-by: cheung wall <zzqq0103.hey@gmail.com> >> Closes: https://lore.kernel.org/linux-mm/87pll35yd0.fsf@devkernel.io/T/#t >> Signed-off-by: Stefan Roesch <shr@devkernel.io> >> --- >> mm/page-writeback.c | 2 ++ >> 1 file changed, 2 insertions(+) >> diff --git a/mm/page-writeback.c b/mm/page-writeback.c >> index d213ead95675..91aa7a5c0078 100644 >> --- a/mm/page-writeback.c >> +++ b/mm/page-writeback.c >> @@ -692,6 +692,8 @@ static unsigned long bdi_ratio_from_pages(unsigned long pages) >> unsigned long ratio; >> global_dirty_limits(&background_thresh, &dirty_thresh); >> + if (!dirty_thresh) >> + return -EINVAL; >> ratio = div64_u64(pages * 100ULL * BDI_RATIO_SCALE, dirty_thresh); >> return ratio; > > bdi_set_min_bytes() calls bdi_ratio_from_pages() and passes the result to > __bdi_set_min_ratio(). > > __bdi_set_min_ratio() expects an "unsigned int min_ratio". I assume this will > work because "max_ratio > 100 * BDI_RATIO_SCALE", but it is rather confusing ... > > Maybe we want something like: > > /* Use 101% to indicate "invalid" */ > #define BDI_RATIO_INVALID (101 * BDI_RATIO_SCALE) > > Or alternatively, just handle it in the callers of bdi_ratio_from_pages(), > checking for -EINVAL manually. David, I prefer the second option, its a bit easier to follow.
diff --git a/mm/page-writeback.c b/mm/page-writeback.c index d213ead95675..91aa7a5c0078 100644 --- a/mm/page-writeback.c +++ b/mm/page-writeback.c @@ -692,6 +692,8 @@ static unsigned long bdi_ratio_from_pages(unsigned long pages) unsigned long ratio; global_dirty_limits(&background_thresh, &dirty_thresh); + if (!dirty_thresh) + return -EINVAL; ratio = div64_u64(pages * 100ULL * BDI_RATIO_SCALE, dirty_thresh); return ratio;
During testing it has been detected, that it is possible to get div by zero error in bdi_set_min_bytes. The error is caused by the function bdi_ratio_from_pages(). bdi_ratio_from_pages() calls global_dirty_limits. If the dirty threshold is 0, the div by zero is raised. This can happen if the root user is setting: echo 0 > /proc/sys/vm/dirty_ration. The following is a test case: echo 0 > /proc/sys/vm/dirty_ratio cd /sys/class/bdi/<device> echo 1 > strict_limit echo 8192 > min_bytes ==> error is raised. The problem is addressed by returning -EINVAL if dirty_ratio or dirty_bytes is set to 0. Reported-by: cheung wall <zzqq0103.hey@gmail.com> Closes: https://lore.kernel.org/linux-mm/87pll35yd0.fsf@devkernel.io/T/#t Signed-off-by: Stefan Roesch <shr@devkernel.io> --- mm/page-writeback.c | 2 ++ 1 file changed, 2 insertions(+) base-commit: 0bc21e701a6ffacfdde7f04f87d664d82e8a13bf