Re: if calloc() needs nmemb and size, why doesn't freezero()?
I used the verbiage: “malloc(3)” as a general all-encompassing manpage which includes malloc(), calloc(), freezero(), etc. Sorry for the confusion. > In malloc(3): >> > “If you use smaller integer types than size_t for ‘nmemb’ and ‘size’, >> then >> > multiplication in freezero() may need to be cast to size_t to avoid >> integer >> > overflow: >> > freezero(ptr, (size_t)nmemb * (size_t)size);” >> > Or maybe even: freezero(ptr, (size_t)nmemb * size); >> >> This is bad advice. The product of two size_t values can exceed >> SIZE_MAX, at which point you would get integer overflow. This is >> why the malloc(3) man page warns against it. Note that on 64-bit >> platforms like amd64, size_t is already 64-bit so casting to unsigned >> long long or uint64_t is not effective. >> >> On OpenBSD, calloc(3) and reallocarray(3) check for integer overflow >> for you, which is why they are preferred over malloc(nmemb * size). >> You can examing the code yourself: >> http://cvsweb.openbsd.org/src/lib/libc/stdlib/reallocarray.c?rev=1.3 >> >> - todd >> > -- > -Luke > -- -Luke
Re: if calloc() needs nmemb and size, why doesn't freezero()?
I agree it can overflow. But if you use the same variables with the same values plugged into ptr = calloc(nmemb, size); as you use in freezero(ptr, (size_t)nmemb * size); If it can overflow, it will have done it already in calloc(). On Fri, Feb 19, 2021 at 12:23 PM Todd C. Miller wrote: > On Fri, 19 Feb 2021 10:38:13 -0600, Luke Small wrote: > > > In malloc(3): > > “If you use smaller integer types than size_t for ‘nmemb’ and ‘size’, > then > > multiplication in freezero() may need to be cast to size_t to avoid > integer > > overflow: > > freezero(ptr, (size_t)nmemb * (size_t)size);” > > Or maybe even: freezero(ptr, (size_t)nmemb * size); > > This is bad advice. The product of two size_t values can exceed > SIZE_MAX, at which point you would get integer overflow. This is > why the malloc(3) man page warns against it. Note that on 64-bit > platforms like amd64, size_t is already 64-bit so casting to unsigned > long long or uint64_t is not effective. > > On OpenBSD, calloc(3) and reallocarray(3) check for integer overflow > for you, which is why they are preferred over malloc(nmemb * size). > You can examing the code yourself: > http://cvsweb.openbsd.org/src/lib/libc/stdlib/reallocarray.c?rev=1.3 > > - todd > -- -Luke
Re: if calloc() needs nmemb and size, why doesn't freezero()?
On Fri, 19 Feb 2021 10:38:13 -0600, Luke Small wrote: > In malloc(3): > “If you use smaller integer types than size_t for ‘nmemb’ and ‘size’, then > multiplication in freezero() may need to be cast to size_t to avoid integer > overflow: > freezero(ptr, (size_t)nmemb * (size_t)size);” > Or maybe even: freezero(ptr, (size_t)nmemb * size); This is bad advice. The product of two size_t values can exceed SIZE_MAX, at which point you would get integer overflow. This is why the malloc(3) man page warns against it. Note that on 64-bit platforms like amd64, size_t is already 64-bit so casting to unsigned long long or uint64_t is not effective. On OpenBSD, calloc(3) and reallocarray(3) check for integer overflow for you, which is why they are preferred over malloc(nmemb * size). You can examing the code yourself: http://cvsweb.openbsd.org/src/lib/libc/stdlib/reallocarray.c?rev=1.3 - todd
Re: if calloc() needs nmemb and size, why doesn't freezero()?
>
> > In the manpage you could succinctly state:
> >
> > In malloc(3):
> > “If you use smaller integer types than size_t for ‘nmemb’ and ‘size’,
> then
> multiplication in freezero() may need to be cast to size_t to avoid
> integer overflow:
> > freezero(ptr, (size_t)nmemb * (size_t)size);”
> > Or maybe even: freezero(ptr, (size_t)nmemb * size);
>
> That is incorrect.
If it’s functionally incorrect, then tell me how the following cast acts
equivalently to intermediate storage or at least calls operations which act
upon uint64_t and makes the function return what is obviously intended on
your OpenBSD:
uint64_t bufferToTime(const u_char buf[])
{
return (( (( (( (( (( (( ((
(uint64_t) buf[7]) << 8)
| buf[6]) << 8)
| buf[5]) << 8)
| buf[4]) << 8)
| buf[3]) << 8)
| buf[2]) << 8)
| buf[1]) << 8)
| buf[0];
}
Because it works.
--
-Luke
Re: if calloc() needs nmemb and size, why doesn't freezero()?
Luke Small wrote:
> malloc(3) already speaks to programmers who might use int multiplication and
> telling
> them to test for int multiplication overflow in malloc(), so you presume that
> they are
> already prepared to use something smaller than size_t, when you could have
> just said:
> “only use size_t variables for integer types.” and cut out the int
> multiplication
> overflow test example.
It seems you don't understand C, and don't want to be taught.
> In the manpage you could succinctly state:
>
> In malloc(3):
> “If you use smaller integer types than size_t for ‘nmemb’ and ‘size’, then
> multiplication in freezero() may need to be cast to size_t to avoid integer
> overflow:
> freezero(ptr, (size_t)nmemb * (size_t)size);”
> Or maybe even: freezero(ptr, (size_t)nmemb * size);
That is incorrect.
> Or:
>
> void freeczero( size_t nmemb, size_t size)
> {
> freezero(nmemb * size);
> }
Not going to happen.
> I suspect that freezero() is already little more than:
>
> void freezero(void *ptr, size_t size)
> {
> explicit_bzero(ptr, size);
> free(ptr);
> }
Wrong.
Re: if calloc() needs nmemb and size, why doesn't freezero()?
malloc(3) already speaks to programmers who might use int multiplication
and telling them to test for int multiplication overflow in malloc(), so
you presume that they are already prepared to use something smaller than
size_t, when you could have just said:
“only use size_t variables for integer types.” and cut out the int
multiplication overflow test example.
In the manpage you could succinctly state:
In malloc(3):
“If you use smaller integer types than size_t for ‘nmemb’ and ‘size’, then
multiplication in freezero() may need to be cast to size_t to avoid integer
overflow:
freezero(ptr, (size_t)nmemb * (size_t)size);”
Or maybe even: freezero(ptr, (size_t)nmemb * size);
Or:
void freeczero( size_t nmemb, size_t size)
{
freezero(nmemb * size);
}
I suspect that freezero() is already little more than:
void freezero(void *ptr, size_t size)
{
explicit_bzero(ptr, size);
free(ptr);
}
On Fri, Feb 19, 2021 at 12:51 AM Otto Moerbeek wrote:
> On Thu, Feb 18, 2021 at 03:24:36PM -0600, Luke Small wrote:
>
> > However, calloc(ptr, nmemb, size) may have been called using smaller int
> > variable types which would overflow when multiplied. Where if the
> variables
> > storing the values passed to nmemb and size are less than or especially
> > equal to their original values, I think it’d be good to state that:
> >
> > freezero(ptr, (size_t)nmemb * (size_t)size);
> > is guaranteed to work, but
> > freezero(ptr, nmemb * size);
> > does not have that guarantee.
>
> Lets try to make things explicit.
>
> The function c() does the overflowe check like calloc does.
> The function f() takes a size_t.
>
> #include
> #include
>
> #define MUL_NO_OVERFLOW (1UL << (sizeof(size_t) * 4))
>
> void c(size_t nmemb, size_t size)
> {
> if ((nmemb >= MUL_NO_OVERFLOW || size >= MUL_NO_OVERFLOW) &&
> nmemb > 0 && SIZE_T_MAX / nmemb < size)
> printf("Overflow\n");
> else
> printf("%zu\n", nmemb * size);
> }
>
> void f(size_t m)
> {
> printf("%zu\n", m);
> }
>
> int
> main()
> {
> int a = INT_MAX;
> int b = INT_MAX;
> c(a, b);
> f(a * b);
> }
>
> Now the issues is that the multiplication in the last line of main()
> overflows:
>
> $ ./a.out
> 4611686014132420609
> 1
>
> because this is an int multiplication only after that the promotion to
> size_t is done.
>
> So you are right that this can happen, *if you are using the wrong
> types*. But I would argue that feeding anything other than either
> size_t or constants to calloc() is already wrong. You *have* to
> consider the argument conversion rules when feeding values to calloc()
> (or any function). To avoid having to think about those, start with
> size_t already for everything that is a size or count of a memory
> object.
>
> -Otto
>
--
-Luke
Re: if calloc() needs nmemb and size, why doesn't freezero()?
On Thu, Feb 18, 2021 at 03:24:36PM -0600, Luke Small wrote:
> However, calloc(ptr, nmemb, size) may have been called using smaller int
> variable types which would overflow when multiplied. Where if the variables
> storing the values passed to nmemb and size are less than or especially
> equal to their original values, I think it’d be good to state that:
>
> freezero(ptr, (size_t)nmemb * (size_t)size);
> is guaranteed to work, but
> freezero(ptr, nmemb * size);
> does not have that guarantee.
Lets try to make things explicit.
The function c() does the overflowe check like calloc does.
The function f() takes a size_t.
#include
#include
#define MUL_NO_OVERFLOW (1UL << (sizeof(size_t) * 4))
void c(size_t nmemb, size_t size)
{
if ((nmemb >= MUL_NO_OVERFLOW || size >= MUL_NO_OVERFLOW) &&
nmemb > 0 && SIZE_T_MAX / nmemb < size)
printf("Overflow\n");
else
printf("%zu\n", nmemb * size);
}
void f(size_t m)
{
printf("%zu\n", m);
}
int
main()
{
int a = INT_MAX;
int b = INT_MAX;
c(a, b);
f(a * b);
}
Now the issues is that the multiplication in the last line of main()
overflows:
$ ./a.out
4611686014132420609
1
because this is an int multiplication only after that the promotion to
size_t is done.
So you are right that this can happen, *if you are using the wrong
types*. But I would argue that feeding anything other than either
size_t or constants to calloc() is already wrong. You *have* to
consider the argument conversion rules when feeding values to calloc()
(or any function). To avoid having to think about those, start with
size_t already for everything that is a size or count of a memory
object.
-Otto
Re: if calloc() needs nmemb and size, why doesn't freezero()?
I had a drawn out email email describing passing by value and the function’s need to only perform size_t multiplication overload checking but not only do you not care I don’t think it’s worth my time to merely succeed in angering you. I love your work! On Thu, Feb 18, 2021 at 7:10 PM Theo de Raadt wrote: > Luke Small wrote: > > > However, calloc(ptr, nmemb, size) may have been called using smaller int > > variable types which would overflow when multiplied. > > In which case the allocation would not have succeeded. > > Where if the variables > > storing the values passed to nmemb and size are less than or especially > > equal to their original values, I think it’d be good to state that: > > Huh? > > > freezero(ptr, (size_t)nmemb * (size_t)size); > > is guaranteed to work, but > > freezero(ptr, nmemb * size); > > does not have that guarantee. > > I hope I never run any software by you. > -- -Luke
Re: if calloc() needs nmemb and size, why doesn't freezero()?
Luke Small wrote: > However, calloc(ptr, nmemb, size) may have been called using smaller int > variable types which would overflow when multiplied. In which case the allocation would not have succeeded. > Where if the variables > storing the values passed to nmemb and size are less than or especially > equal to their original values, I think it’d be good to state that: Huh? > freezero(ptr, (size_t)nmemb * (size_t)size); > is guaranteed to work, but > freezero(ptr, nmemb * size); > does not have that guarantee. I hope I never run any software by you.
Re: if calloc() needs nmemb and size, why doesn't freezero()?
However, calloc(ptr, nmemb, size) may have been called using smaller int variable types which would overflow when multiplied. Where if the variables storing the values passed to nmemb and size are less than or especially equal to their original values, I think it’d be good to state that: freezero(ptr, (size_t)nmemb * (size_t)size); is guaranteed to work, but freezero(ptr, nmemb * size); does not have that guarantee. On Thu, Feb 18, 2021 at 3:42 AM Otto Moerbeek wrote: > On Wed, Feb 17, 2021 at 11:05:49AM -0700, Theo de Raadt wrote: > > > Luke Small wrote: > > > > > I guess I always thought there'd be some more substantial overflow > mitigation. > > > > You have to free with the exact same size as allocation. > > Small correction: the size may be smaller than the original. In that > case, only a partial clear is guaranteed, the deallocation will still > be for the full allocation. Originally we were more strict, but iirc > that was causing to much headaches for some. See > https://cvsweb.openbsd.org/src/lib/libc/stdlib/malloc.c?rev=1.221 > > But the point stands: nmemb * size does not overflow, since the > original allocation would have overflowed and thus failed. > > -Otto > > > > > nmemb and size did not change. > > > > The math has already been checked, and regular codeflows will store the > > multiple in a single variable after successful checking&allocation, for > > reuse. > > > > > Would it be too much hand-holding to put in the manpage that to avoid > potential > > > freeezero() integer overflow, > > > it may be useful to run freezero() as freezero((size_t)nmemb * > (size_t)size); > > > > Wow, Those casts make it very clear you don't understand C, if you do > > that kind of stuff elsewhere you are introducing problems. > > > > Sorry no you are wrong. > > > -- -Luke
Re: if calloc() needs nmemb and size, why doesn't freezero()?
On Wed, Feb 17, 2021 at 11:05:49AM -0700, Theo de Raadt wrote: > Luke Small wrote: > > > I guess I always thought there'd be some more substantial overflow > > mitigation. > > You have to free with the exact same size as allocation. Small correction: the size may be smaller than the original. In that case, only a partial clear is guaranteed, the deallocation will still be for the full allocation. Originally we were more strict, but iirc that was causing to much headaches for some. See https://cvsweb.openbsd.org/src/lib/libc/stdlib/malloc.c?rev=1.221 But the point stands: nmemb * size does not overflow, since the original allocation would have overflowed and thus failed. -Otto > > nmemb and size did not change. > > The math has already been checked, and regular codeflows will store the > multiple in a single variable after successful checking&allocation, for > reuse. > > > Would it be too much hand-holding to put in the manpage that to avoid > > potential > > freeezero() integer overflow, > > it may be useful to run freezero() as freezero((size_t)nmemb * > > (size_t)size); > > Wow, Those casts make it very clear you don't understand C, if you do > that kind of stuff elsewhere you are introducing problems. > > Sorry no you are wrong. >
Re: if calloc() needs nmemb and size, why doesn't freezero()?
Am I incorrect to presume that if the values are successfully used in calloc(), that (size_t)nmemb * (size_t)size will not overflow? Isn't the storage capacity of size_t greater than the amount of addressable space? If it is, calloc() will throw an "out of memory" or other error before you'll ever reach putting freezero((size_t)nmemb * (size_t)size); -Luke On Wed, Feb 17, 2021 at 2:36 PM Luke Small wrote: > if the nmemb and size values being passed to calloc() are of a larger > integer datatype, they will have been truncated when passed to the function > there as well. > > Perhaps you need something larger than size_t in the entire malloc manpage > series? > > > > -Luke > > > On Wed, Feb 17, 2021 at 2:25 PM Theo de Raadt wrote: > >> > > Would it be too much hand-holding to put in the manpage that to >> avoid potential >> > > freeezero() integer overflow, >> > > it may be useful to run freezero() as freezero((size_t)nmemb * >> (size_t)size); >> > >> > Wow, Those casts make it very clear you don't understand C, if you do >> > that kind of stuff elsewhere you are introducing problems. >> >> If nmemb or size are of a type greater than size_t, those casts serve >> only one >> purpose -- truncating the high bits before performing multiply, which >> results in >> an incorrect size. >> >> >> >>
Re: if calloc() needs nmemb and size, why doesn't freezero()?
if the nmemb and size values being passed to calloc() are of a larger integer datatype, they will have been truncated when passed to the function there as well. Perhaps you need something larger than size_t in the entire malloc manpage series? -Luke On Wed, Feb 17, 2021 at 2:25 PM Theo de Raadt wrote: > > > Would it be too much hand-holding to put in the manpage that to avoid > potential > > > freeezero() integer overflow, > > > it may be useful to run freezero() as freezero((size_t)nmemb * > (size_t)size); > > > > Wow, Those casts make it very clear you don't understand C, if you do > > that kind of stuff elsewhere you are introducing problems. > > If nmemb or size are of a type greater than size_t, those casts serve only > one > purpose -- truncating the high bits before performing multiply, which > results in > an incorrect size. > > > >
Re: if calloc() needs nmemb and size, why doesn't freezero()?
I guess I’ve been doing it wrong all this time.
Perhaps you can tell me how the following doesn't return a 0-255 value.
uint64_t bufferToTime(const u_char buf[])
{
return (( (( (( (( (( (( ((
(uint64_t) buf[7]) << 8)
| buf[6]) << 8)
| buf[5]) << 8)
| buf[4]) << 8)
| buf[3]) << 8)
| buf[2]) << 8)
| buf[1]) << 8)
| buf[0];
}
}
On Wed, Feb 17, 2021 at 12:05 PM Theo de Raadt wrote:
> Luke Small wrote:
>
> > I guess I always thought there'd be some more substantial overflow
> mitigation.
>
> You have to free with the exact same size as allocation.
>
> nmemb and size did not change.
>
> The math has already been checked, and regular codeflows will store the
> multiple in a single variable after successful checking&allocation, for
> reuse.
>
> > Would it be too much hand-holding to put in the manpage that to avoid
> potential
> > freeezero() integer overflow,
> > it may be useful to run freezero() as freezero((size_t)nmemb *
> (size_t)size);
>
> Wow, Those casts make it very clear you don't understand C, if you do
> that kind of stuff elsewhere you are introducing problems.
>
> Sorry no you are wrong.
>
Re: if calloc() needs nmemb and size, why doesn't freezero()?
> > Would it be too much hand-holding to put in the manpage that to avoid > potential > > freeezero() integer overflow, > > it may be useful to run freezero() as freezero((size_t)nmemb * > (size_t)size); > > Wow, Those casts make it very clear you don't understand C, if you do > that kind of stuff elsewhere you are introducing problems. If nmemb or size are of a type greater than size_t, those casts serve only one purpose -- truncating the high bits before performing multiply, which results in an incorrect size.
Re: if calloc() needs nmemb and size, why doesn't freezero()?
Luke Small wrote: > I guess I always thought there'd be some more substantial overflow mitigation. You have to free with the exact same size as allocation. nmemb and size did not change. The math has already been checked, and regular codeflows will store the multiple in a single variable after successful checking&allocation, for reuse. > Would it be too much hand-holding to put in the manpage that to avoid > potential > freeezero() integer overflow, > it may be useful to run freezero() as freezero((size_t)nmemb * (size_t)size); Wow, Those casts make it very clear you don't understand C, if you do that kind of stuff elsewhere you are introducing problems. Sorry no you are wrong.
Re: if calloc() needs nmemb and size, why doesn't freezero()?
I guess I always thought there'd be some more substantial overflow mitigation. Would it be too much hand-holding to put in the manpage that to avoid potential freeezero() integer overflow, it may be useful to run freezero() as freezero((size_t)nmemb * (size_t)size); -Luke On Wed, Feb 17, 2021 at 11:04 AM Theo de Raadt wrote: > Luke Small wrote: > > > if calloc() and recallocarray() needs nmemb and size, why doesn't > > freezero()? > > > > Should there be a freeczero(size_t nmemb, size_t size) ? > > Performing the nmemb*size overflow detection a second time provides > no benefit. > > >
Re: if calloc() needs nmemb and size, why doesn't freezero()?
Luke Small wrote: > if calloc() and recallocarray() needs nmemb and size, why doesn't > freezero()? > > Should there be a freeczero(size_t nmemb, size_t size) ? Performing the nmemb*size overflow detection a second time provides no benefit.
if calloc() needs nmemb and size, why doesn't freezero()?
if calloc() and recallocarray() needs nmemb and size, why doesn't freezero()? Should there be a freeczero(size_t nmemb, size_t size) ? -Luke
