| Message ID | 20200622162141.279716-5-imbrenda@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Minor fixes, improvements, and cleanup | expand |
On 22/06/2020 18.21, Claudio Imbrenda wrote: > Add an overflow check for calloc to prevent potential multiplication overflow. > > Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com> > --- > lib/alloc.c | 36 +++++++++++++++++++++++++++++++++++- > 1 file changed, 35 insertions(+), 1 deletion(-) > > diff --git a/lib/alloc.c b/lib/alloc.c > index ed8f5f9..f4aa87a 100644 > --- a/lib/alloc.c > +++ b/lib/alloc.c > @@ -6,9 +6,43 @@ void *malloc(size_t size) > return memalign(sizeof(long), size); > } > > +static bool mult_overflow(size_t a, size_t b) > +{ > +#if BITS_PER_LONG == 32 > + /* 32 bit system, easy case: just use u64 */ > + return (u64)a * (u64)b >= (1ULL << 32); > +#else > +#ifdef __SIZEOF_INT128__ > + /* if __int128 is available use it (like the u64 case above) */ > + unsigned __int128 res = a; > + res *= b; > + res >>= 64; > + return res != 0; > +#else > + u64 tmp; > + > + if ((a >> 32) && (b >> 32)) > + return true; > + if (!(a >> 32) && !(b >> 32)) > + return false; > + tmp = (u32)a; > + tmp *= (u32)b; > + tmp >>= 32; > + if (a < b) > + tmp += a * (b >> 32); > + else > + tmp += b * (a >> 32); > + return tmp >> 32; > +#endif /* __SIZEOF_INT128__ */ > +#endif /* BITS_PER_LONG == 32 */ > +} This caused a new regression in the CI: https://gitlab.com/huth/kvm-unit-tests/-/jobs/606930599#L1754 https://travis-ci.com/github/huth/kvm-unit-tests/jobs/352515116#L546 lib/alloc.c: In function 'mult_overflow': lib/alloc.c:24:9: error: right shift count >= width of type [-Werror=shift-count-overflow] 24 | if ((a >> 32) && (b >> 32)) | ^~ lib/alloc.c:24:22: error: right shift count >= width of type [-Werror=shift-count-overflow] 24 | if ((a >> 32) && (b >> 32)) | ^~ lib/alloc.c:26:10: error: right shift count >= width of type [-Werror=shift-count-overflow] 26 | if (!(a >> 32) && !(b >> 32)) | ^~ lib/alloc.c:26:24: error: right shift count >= width of type [-Werror=shift-count-overflow] 26 | if (!(a >> 32) && !(b >> 32)) | ^~ lib/alloc.c:32:17: error: right shift count >= width of type [-Werror=shift-count-overflow] 32 | tmp += a * (b >> 32); | ^~ lib/alloc.c:34:17: error: right shift count >= width of type [-Werror=shift-count-overflow] 34 | tmp += b * (a >> 32); | ^~ cc1: all warnings being treated as errors Claudio, any ideas how to fix that? Paolo, could you maybe push your staging branches to Githab or Gitlab first, to see whether there are any regressions in Travis or Gitlab-CI before you push the branch to the main repo? ... almost all of the recent updates broke something, and analyzing the problems afterwards is a little bit cumbersome... Thomas
On 23/06/20 07:57, Thomas Huth wrote: > cc1: all warnings being treated as errors > > Claudio, any ideas how to fix that? I guess it's just diff --git a/lib/alloc.c b/lib/alloc.c index f4aa87a..6c89f98 100644 --- a/lib/alloc.c +++ b/lib/alloc.c @@ -1,5 +1,6 @@ #include "alloc.h" #include "asm/page.h" +#include "bitops.h" void *malloc(size_t size) { > Paolo, could you maybe push your staging branches to Githab or Gitlab > first, to see whether there are any regressions in Travis or Gitlab-CI > before you push the branch to the main repo? ... almost all of the > recent updates broke something, and analyzing the problems afterwards is > a little bit cumbersome... Ok, I'll catch you on IRC to discuss this. Paolo
diff --git a/lib/alloc.c b/lib/alloc.c index ed8f5f9..f4aa87a 100644 --- a/lib/alloc.c +++ b/lib/alloc.c @@ -6,9 +6,43 @@ void *malloc(size_t size) return memalign(sizeof(long), size); } +static bool mult_overflow(size_t a, size_t b) +{ +#if BITS_PER_LONG == 32 + /* 32 bit system, easy case: just use u64 */ + return (u64)a * (u64)b >= (1ULL << 32); +#else +#ifdef __SIZEOF_INT128__ + /* if __int128 is available use it (like the u64 case above) */ + unsigned __int128 res = a; + res *= b; + res >>= 64; + return res != 0; +#else + u64 tmp; + + if ((a >> 32) && (b >> 32)) + return true; + if (!(a >> 32) && !(b >> 32)) + return false; + tmp = (u32)a; + tmp *= (u32)b; + tmp >>= 32; + if (a < b) + tmp += a * (b >> 32); + else + tmp += b * (a >> 32); + return tmp >> 32; +#endif /* __SIZEOF_INT128__ */ +#endif /* BITS_PER_LONG == 32 */ +} + void *calloc(size_t nmemb, size_t size) { - void *ptr = malloc(nmemb * size); + void *ptr; + + assert(!mult_overflow(nmemb, size)); + ptr = malloc(nmemb * size); if (ptr) memset(ptr, 0, nmemb * size); return ptr;
Add an overflow check for calloc to prevent potential multiplication overflow. Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com> --- lib/alloc.c | 36 +++++++++++++++++++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-)