Re: ldapd warning
On Sun, Nov 29, 2020 at 08:41:50AM +0100, Theo Buehler wrote: > On Sun, Nov 29, 2020 at 08:02:45AM +0100, Emil Engler wrote: > > It can overflow! Please check for the positivity and width of size_t before! > > What can overflow? ret is guaranteed to be non-negative before the cast. > > As for the width (which would be about truncation, not overflow): while > the standard allows for size_t to be an unsigned integer type as small > as 16 bits, we generally assume that sizeof(size_t) >= sizeof(int). > I don't think I've ever seen a width check ensuring this in our sources. Maybe rummage arround in the openssl attic? There migth be code that checks for sizeof(size_t) changing during runtime :P -- I'm not entirely sure you are real.
Re: ldapd warning
On Sun, Nov 29, 2020 at 06:42:47PM +1000, Jonathan Matthew wrote: > On Sat, Nov 28, 2020 at 11:20:30PM +0100, Theo Buehler wrote: > > /usr/src/usr.sbin/ldapd/util.c:46:21: warning: comparison of integers of > > different signs: > > 'int' and 'size_t' (aka 'unsigned long') [-Wsign-compare] > > if (ret < 0 || ret >= size) > >~~~ ^ > > > > This has been around for a while. I forgot that I had this patch in my > > tree. > > 'size' was cast to int before r1.11 of util.c, I'm not sure why the cast was > removed. smtpd also has a copy of this function that still has the cast. because it's the wrong cast. Presumably that cast was put in to silence a compiler warning, but it's the wrong way around. We need to cast the smaller type to the larger type. OK florian tb, could you also fix smtpd? > > > > > > Index: util.c > > === > > RCS file: /cvs/src/usr.sbin/ldapd/util.c,v > > retrieving revision 1.12 > > diff -u -p -r1.12 util.c > > --- util.c 24 Oct 2019 12:39:26 - 1.12 > > +++ util.c 4 Aug 2020 07:14:33 - > > @@ -43,7 +43,7 @@ bsnprintf(char *str, size_t size, const > > va_start(ap, format); > > ret = vsnprintf(str, size, format, ap); > > va_end(ap); > > - if (ret < 0 || ret >= size) > > + if (ret < 0 || (size_t)ret >= size) > > return 0; > > > > return 1; > > > -- I'm not entirely sure you are real.
Re: ldapd warning
On Sat, Nov 28, 2020 at 11:20:30PM +0100, Theo Buehler wrote: > /usr/src/usr.sbin/ldapd/util.c:46:21: warning: comparison of integers of > different signs: > 'int' and 'size_t' (aka 'unsigned long') [-Wsign-compare] > if (ret < 0 || ret >= size) >~~~ ^ > > This has been around for a while. I forgot that I had this patch in my > tree. 'size' was cast to int before r1.11 of util.c, I'm not sure why the cast was removed. smtpd also has a copy of this function that still has the cast. > > Index: util.c > === > RCS file: /cvs/src/usr.sbin/ldapd/util.c,v > retrieving revision 1.12 > diff -u -p -r1.12 util.c > --- util.c24 Oct 2019 12:39:26 - 1.12 > +++ util.c4 Aug 2020 07:14:33 - > @@ -43,7 +43,7 @@ bsnprintf(char *str, size_t size, const > va_start(ap, format); > ret = vsnprintf(str, size, format, ap); > va_end(ap); > - if (ret < 0 || ret >= size) > + if (ret < 0 || (size_t)ret >= size) > return 0; > > return 1; >
Re: ldapd warning
On Sun, Nov 29, 2020 at 08:02:45AM +0100, Emil Engler wrote: > It can overflow! Please check for the positivity and width of size_t before! What can overflow? ret is guaranteed to be non-negative before the cast. As for the width (which would be about truncation, not overflow): while the standard allows for size_t to be an unsigned integer type as small as 16 bits, we generally assume that sizeof(size_t) >= sizeof(int). I don't think I've ever seen a width check ensuring this in our sources. > > Cheers, > Emil > > On 11/28/20 11:20 PM, Theo Buehler wrote: > > /usr/src/usr.sbin/ldapd/util.c:46:21: warning: comparison of integers of > > different signs: > >'int' and 'size_t' (aka 'unsigned long') [-Wsign-compare] > > if (ret < 0 || ret >= size) > > ~~~ ^ > > > > This has been around for a while. I forgot that I had this patch in my > > tree. > > > > Index: util.c > > === > > RCS file: /cvs/src/usr.sbin/ldapd/util.c,v > > retrieving revision 1.12 > > diff -u -p -r1.12 util.c > > --- util.c 24 Oct 2019 12:39:26 - 1.12 > > +++ util.c 4 Aug 2020 07:14:33 - > > @@ -43,7 +43,7 @@ bsnprintf(char *str, size_t size, const > > va_start(ap, format); > > ret = vsnprintf(str, size, format, ap); > > va_end(ap); > > - if (ret < 0 || ret >= size) > > + if (ret < 0 || (size_t)ret >= size) > > return 0; > > return 1; > > >
Re: ldapd warning
It can overflow! Please check for the positivity and width of size_t before! Cheers, Emil On 11/28/20 11:20 PM, Theo Buehler wrote: /usr/src/usr.sbin/ldapd/util.c:46:21: warning: comparison of integers of different signs: 'int' and 'size_t' (aka 'unsigned long') [-Wsign-compare] if (ret < 0 || ret >= size) ~~~ ^ This has been around for a while. I forgot that I had this patch in my tree. Index: util.c === RCS file: /cvs/src/usr.sbin/ldapd/util.c,v retrieving revision 1.12 diff -u -p -r1.12 util.c --- util.c 24 Oct 2019 12:39:26 - 1.12 +++ util.c 4 Aug 2020 07:14:33 - @@ -43,7 +43,7 @@ bsnprintf(char *str, size_t size, const va_start(ap, format); ret = vsnprintf(str, size, format, ap); va_end(ap); - if (ret < 0 || ret >= size) + if (ret < 0 || (size_t)ret >= size) return 0; return 1;
ldapd warning
/usr/src/usr.sbin/ldapd/util.c:46:21: warning: comparison of integers of different signs: 'int' and 'size_t' (aka 'unsigned long') [-Wsign-compare] if (ret < 0 || ret >= size) ~~~ ^ This has been around for a while. I forgot that I had this patch in my tree. Index: util.c === RCS file: /cvs/src/usr.sbin/ldapd/util.c,v retrieving revision 1.12 diff -u -p -r1.12 util.c --- util.c 24 Oct 2019 12:39:26 - 1.12 +++ util.c 4 Aug 2020 07:14:33 - @@ -43,7 +43,7 @@ bsnprintf(char *str, size_t size, const va_start(ap, format); ret = vsnprintf(str, size, format, ap); va_end(ap); - if (ret < 0 || ret >= size) + if (ret < 0 || (size_t)ret >= size) return 0; return 1;
