| Message ID | 20190625100518.30753-1-vkoul@kernel.org |
|---|---|
| State | Accepted |
| Commit | 8f9fab480c7a87b10bb5440b5555f370272a5d59 |
| Headers | show |
| Series | linux/kernel.h: fix overflow for DIV_ROUND_UP_ULL | expand |
On Tue, 25 Jun 2019 15:35:18 +0530 Vinod Koul <vkoul@kernel.org> wrote: > DIV_ROUND_UP_ULL adds the two arguments and then invokes > DIV_ROUND_DOWN_ULL. But on a 32bit system the addition of two 32 bit > values can overflow. DIV_ROUND_DOWN_ULL does it correctly and stashes > the addition into a unsigned long long so cast the result to unsigned > long long here to avoid the overflow condition. > > ... > > --- a/include/linux/kernel.h > +++ b/include/linux/kernel.h > @@ -93,7 +93,8 @@ > #define DIV_ROUND_DOWN_ULL(ll, d) \ > ({ unsigned long long _tmp = (ll); do_div(_tmp, d); _tmp; }) > > -#define DIV_ROUND_UP_ULL(ll, d) DIV_ROUND_DOWN_ULL((ll) + (d) - 1, (d)) > +#define DIV_ROUND_UP_ULL(ll, d) \ > + ({ DIV_ROUND_DOWN_ULL((unsigned long long)(ll) + (d) - 1, (d)) }) > This clearly wasn't tested :( fs/fs-writeback.c: In function wb_split_bdi_pages: ./include/linux/kernel.h:97:65: error: expected ; before } token ({ DIV_ROUND_DOWN_ULL((unsigned long long)(ll) + (d) - 1, (d)) }) ^ fs/fs-writeback.c:811:10: note: in expansion of macro DIV_ROUND_UP_ULL return DIV_ROUND_UP_ULL((u64)nr_pages * this_bw, tot_bw); From: Andrew Morton <akpm@linux-foundation.org> Subject: linux-kernelh-fix-overflow-for-div_round_up_ull-fix DIV_ROUND_UP_ULL must be an rval Cc: Bjorn Andersson <bjorn.andersson@linaro.org> Cc: Randy Dunlap <rdunlap@infradead.org> Cc: Vinod Koul <vkoul@kernel.org> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> --- include/linux/kernel.h | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) --- a/include/linux/kernel.h~linux-kernelh-fix-overflow-for-div_round_up_ull-fix +++ a/include/linux/kernel.h @@ -93,8 +93,10 @@ #define DIV_ROUND_DOWN_ULL(ll, d) \ ({ unsigned long long _tmp = (ll); do_div(_tmp, d); _tmp; }) -#define DIV_ROUND_UP_ULL(ll, d) \ - ({ DIV_ROUND_DOWN_ULL((unsigned long long)(ll) + (d) - 1, (d)) }) +#define DIV_ROUND_UP_ULL(ll, d) ({ \ + unsigned long long _tmp; \ + _tmp = DIV_ROUND_DOWN_ULL((unsigned long long)(ll) + (d) - 1, (d)); \ + _tmp; }) #if BITS_PER_LONG == 32 # define DIV_ROUND_UP_SECTOR_T(ll,d) DIV_ROUND_UP_ULL(ll, d)
On Tue, 25 Jun 2019 15:29:38 -0700 Andrew Morton <akpm@linux-foundation.org> wrote: > On Tue, 25 Jun 2019 15:35:18 +0530 Vinod Koul <vkoul@kernel.org> wrote: > > > DIV_ROUND_UP_ULL adds the two arguments and then invokes > > DIV_ROUND_DOWN_ULL. But on a 32bit system the addition of two 32 bit > > values can overflow. DIV_ROUND_DOWN_ULL does it correctly and stashes > > the addition into a unsigned long long so cast the result to unsigned > > long long here to avoid the overflow condition. > > > > ... > > > > --- a/include/linux/kernel.h > > +++ b/include/linux/kernel.h > > @@ -93,7 +93,8 @@ > > #define DIV_ROUND_DOWN_ULL(ll, d) \ > > ({ unsigned long long _tmp = (ll); do_div(_tmp, d); _tmp; }) > > > > -#define DIV_ROUND_UP_ULL(ll, d) DIV_ROUND_DOWN_ULL((ll) + (d) - 1, (d)) > > +#define DIV_ROUND_UP_ULL(ll, d) \ > > + ({ DIV_ROUND_DOWN_ULL((unsigned long long)(ll) + (d) - 1, (d)) }) > > > > This clearly wasn't tested :( > > fs/fs-writeback.c: In function wb_split_bdi_pages: > ./include/linux/kernel.h:97:65: error: expected ; before } token > ({ DIV_ROUND_DOWN_ULL((unsigned long long)(ll) + (d) - 1, (d)) }) > ^ > fs/fs-writeback.c:811:10: note: in expansion of macro DIV_ROUND_UP_ULL > return DIV_ROUND_UP_ULL((u64)nr_pages * this_bw, tot_bw); > > > From: Andrew Morton <akpm@linux-foundation.org> > Subject: linux-kernelh-fix-overflow-for-div_round_up_ull-fix > > DIV_ROUND_UP_ULL must be an rval > > Cc: Bjorn Andersson <bjorn.andersson@linaro.org> > Cc: Randy Dunlap <rdunlap@infradead.org> > Cc: Vinod Koul <vkoul@kernel.org> > Signed-off-by: Andrew Morton <akpm@linux-foundation.org> > --- > > include/linux/kernel.h | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > --- a/include/linux/kernel.h~linux-kernelh-fix-overflow-for-div_round_up_ull-fix > +++ a/include/linux/kernel.h > @@ -93,8 +93,10 @@ > #define DIV_ROUND_DOWN_ULL(ll, d) \ > ({ unsigned long long _tmp = (ll); do_div(_tmp, d); _tmp; }) > > -#define DIV_ROUND_UP_ULL(ll, d) \ > - ({ DIV_ROUND_DOWN_ULL((unsigned long long)(ll) + (d) - 1, (d)) }) > +#define DIV_ROUND_UP_ULL(ll, d) ({ \ > + unsigned long long _tmp; \ > + _tmp = DIV_ROUND_DOWN_ULL((unsigned long long)(ll) + (d) - 1, (d)); \ > + _tmp; }) Simpler: --- a/include/linux/kernel.h~linux-kernelh-fix-overflow-for-div_round_up_ull-fix +++ a/include/linux/kernel.h @@ -94,7 +94,7 @@ ({ unsigned long long _tmp = (ll); do_div(_tmp, d); _tmp; }) #define DIV_ROUND_UP_ULL(ll, d) \ - ({ DIV_ROUND_DOWN_ULL((unsigned long long)(ll) + (d) - 1, (d)) }) + DIV_ROUND_DOWN_ULL((unsigned long long)(ll) + (d) - 1, (d)) #if BITS_PER_LONG == 32 # define DIV_ROUND_UP_SECTOR_T(ll,d) DIV_ROUND_UP_ULL(ll, d)
On 25-06-19, 15:32, Andrew Morton wrote: > On Tue, 25 Jun 2019 15:29:38 -0700 Andrew Morton <akpm@linux-foundation.org> wrote: > > > On Tue, 25 Jun 2019 15:35:18 +0530 Vinod Koul <vkoul@kernel.org> wrote: > > > > > DIV_ROUND_UP_ULL adds the two arguments and then invokes > > > DIV_ROUND_DOWN_ULL. But on a 32bit system the addition of two 32 bit > > > values can overflow. DIV_ROUND_DOWN_ULL does it correctly and stashes > > > the addition into a unsigned long long so cast the result to unsigned > > > long long here to avoid the overflow condition. > > > > > > ... > > > > > > --- a/include/linux/kernel.h > > > +++ b/include/linux/kernel.h > > > @@ -93,7 +93,8 @@ > > > #define DIV_ROUND_DOWN_ULL(ll, d) \ > > > ({ unsigned long long _tmp = (ll); do_div(_tmp, d); _tmp; }) > > > > > > -#define DIV_ROUND_UP_ULL(ll, d) DIV_ROUND_DOWN_ULL((ll) + (d) - 1, (d)) > > > +#define DIV_ROUND_UP_ULL(ll, d) \ > > > + ({ DIV_ROUND_DOWN_ULL((unsigned long long)(ll) + (d) - 1, (d)) }) > > > > > > > This clearly wasn't tested :( Apologies for that, I did test and stash, but failed to amend the commit. I should have noticed while sending but :( Anyway I had the same conclusion as yous, so all is good. Thanks for fixing this Reviewed-by: Vinod Koul <vkoul@kernel.org> Tested-by: Vinod Koul <vkoul@kernel.org> > > > > fs/fs-writeback.c: In function wb_split_bdi_pages: > > ./include/linux/kernel.h:97:65: error: expected ; before } token > > ({ DIV_ROUND_DOWN_ULL((unsigned long long)(ll) + (d) - 1, (d)) }) > > ^ > > fs/fs-writeback.c:811:10: note: in expansion of macro DIV_ROUND_UP_ULL > > return DIV_ROUND_UP_ULL((u64)nr_pages * this_bw, tot_bw); > > > > > > From: Andrew Morton <akpm@linux-foundation.org> > > Subject: linux-kernelh-fix-overflow-for-div_round_up_ull-fix > > > > DIV_ROUND_UP_ULL must be an rval > > > > Cc: Bjorn Andersson <bjorn.andersson@linaro.org> > > Cc: Randy Dunlap <rdunlap@infradead.org> > > Cc: Vinod Koul <vkoul@kernel.org> > > Signed-off-by: Andrew Morton <akpm@linux-foundation.org> > > --- > > > > include/linux/kernel.h | 6 ++++-- > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > --- a/include/linux/kernel.h~linux-kernelh-fix-overflow-for-div_round_up_ull-fix > > +++ a/include/linux/kernel.h > > @@ -93,8 +93,10 @@ > > #define DIV_ROUND_DOWN_ULL(ll, d) \ > > ({ unsigned long long _tmp = (ll); do_div(_tmp, d); _tmp; }) > > > > -#define DIV_ROUND_UP_ULL(ll, d) \ > > - ({ DIV_ROUND_DOWN_ULL((unsigned long long)(ll) + (d) - 1, (d)) }) > > +#define DIV_ROUND_UP_ULL(ll, d) ({ \ > > + unsigned long long _tmp; \ > > + _tmp = DIV_ROUND_DOWN_ULL((unsigned long long)(ll) + (d) - 1, (d)); \ > > + _tmp; }) > > Simpler: > > --- a/include/linux/kernel.h~linux-kernelh-fix-overflow-for-div_round_up_ull-fix > +++ a/include/linux/kernel.h > @@ -94,7 +94,7 @@ > ({ unsigned long long _tmp = (ll); do_div(_tmp, d); _tmp; }) > > #define DIV_ROUND_UP_ULL(ll, d) \ > - ({ DIV_ROUND_DOWN_ULL((unsigned long long)(ll) + (d) - 1, (d)) }) > + DIV_ROUND_DOWN_ULL((unsigned long long)(ll) + (d) - 1, (d)) > > #if BITS_PER_LONG == 32 > # define DIV_ROUND_UP_SECTOR_T(ll,d) DIV_ROUND_UP_ULL(ll, d) > _ -- ~Vinod
diff --git a/include/linux/kernel.h b/include/linux/kernel.h index 74b1ee9027f5..1214fb48cfc8 100644 --- a/include/linux/kernel.h +++ b/include/linux/kernel.h @@ -93,7 +93,8 @@ #define DIV_ROUND_DOWN_ULL(ll, d) \ ({ unsigned long long _tmp = (ll); do_div(_tmp, d); _tmp; }) -#define DIV_ROUND_UP_ULL(ll, d) DIV_ROUND_DOWN_ULL((ll) + (d) - 1, (d)) +#define DIV_ROUND_UP_ULL(ll, d) \ + ({ DIV_ROUND_DOWN_ULL((unsigned long long)(ll) + (d) - 1, (d)) }) #if BITS_PER_LONG == 32 # define DIV_ROUND_UP_SECTOR_T(ll,d) DIV_ROUND_UP_ULL(ll, d)
DIV_ROUND_UP_ULL adds the two arguments and then invokes DIV_ROUND_DOWN_ULL. But on a 32bit system the addition of two 32 bit values can overflow. DIV_ROUND_DOWN_ULL does it correctly and stashes the addition into a unsigned long long so cast the result to unsigned long long here to avoid the overflow condition. Signed-off-by: Vinod Koul <vkoul@kernel.org> --- include/linux/kernel.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) -- 2.20.1