On 01/24/18 12:37, Conrad Meyer wrote:
On Tue, Jan 23, 2018 at 11:40 AM, Pedro Giffuni <p...@freebsd.org> wrote:
On 23/01/2018 14:08, Conrad Meyer wrote:
On Sun, Jan 21, 2018 at 7:42 AM, Pedro F. Giffuni <p...@freebsd.org> wrote:
Author: pfg
Date: Sun Jan 21 15:42:36 2018
New Revision: 328218
I'm confused about this change. Wouldn't it be better to remove the
annotation/attributes from mallocarray() than to remove the protection
against overflow?
Not in my opinion: it would be better to detect such overflows at compile
time (or through a static analyzer) than to have late notification though
panics.
Sure, it would be better, but some situations are only detected at
runtime -- hence mallocarray. And occasional use of the annotations
on systems with plenty of RAM would keep the source tree free of
compiler-detectable overflows, which I suspect are incredibly
uncommon.
I think the runtime error cases are way more uncommon, specially if the
checks are unnecessary.
In any case I collected the patch with malloc--> mallocarray changes in
PR 225197.
Maybe their are useful with fuzzing.
The blind use of mallocarray(9) is probably a mistake also: we
shouldn't use it unless there is some real risk of overflow.
I'm not sure I follow that.
You normally don't get "tainted" values in the kernel. Most of the
allocations in question were variables with very small number multiplied
by the size of an int. As Bruce mentioned, we don't do big allocations
with malloc(9), at least not the size required to get an unsigned number
overflow. In such cases checking for an overflow is a complete waste of
time. And then there is also the bug of mallocarray(9) size types not
matching malloc(9).
(If the compiler is fixed in the future to not use
excessive memory with these attributes, they can be conditionalized on
compiler version, of course.)
All in all, the compiler is not provably wrong: it's just using more swap
space, which is rather inconvenient for small platforms but not necessarily
wrong.
Seems wrong if it's a noticeable amount. Maybe we could flip the
annotations on or off with a low-ram build knob or something like
that.
There is actually no proof that this was related to the attributes. I
absolutely dislike the idea of disabling the attributes and even more
the idea of adding complex machinery to the system headers to account
for such case.
Pedro.
_______________________________________________
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
