Re: [PATCH] Add reallocarray function.

[Dennis Wölfing <denniswoelfing at gmx dot de>]

On 10.04.2017 20:26, Dennis Wölfing wrote:
> On 10.04.2017 17:18, Zack Weinberg wrote:
>> On Mon, Apr 10, 2017 at 11:00 AM, Dennis Wölfing <denniswoelfing@gmx.de> wrote:
>>> The reallocarray function is an extension from OpenBSD.  It is an
>>> integer-overflow-safe replacement for realloc(p, X*Y) and
>>> malloc(X*Y) (realloc(NULL, X*Y)).  It can therefore help in preventing
>>> certain security issues in code.
>>>
>>> This is an updated version of a patch originally submitted by Rüdiger
>>> Sonderfeld in May 2014.
>>> See <https://sourceware.org/ml/libc-alpha/2014-05/msg00481.html>.
>>
>> I agree in principle with adding this function.  I skimmed the patch
>> and it seems to be mostly the Right Thing.  I do have two concerns:
>>
>> * There do not appear to be any uses of the internal aliases
>> __libc_reallocarray and __reallocarray.  Have you audited glibc itself
>> for places that should use reallocarray?  If you haven't, would you be
>> willing to do that?  This will determine whether we actually need
>> those aliases.
> 
> I have not yet checked where glibc itself should use reallocarray but I
> will do so.

At least the following files contain calls to realloc where the size
argument is the result of a multiplication. Note that I have not checked
all of these files in detail so it is possible that many these
multiplications can never overflow.

catgets/gencat.c
dirent/scandir-tail.c
grp/compat-initgroups.c
hesiod/nss_hesiod/hesiod-grp.c
iconv/iconvconfig.c
io/fts.c
libidn/idna.c
libio/iogetdelim.c
locale/programs/3level.h
locale/programs/charmap-dir.c
locale/programs/ld-collate.c
locale/programs/ld-ctypes.c
locale/programs/ld-monetary.c
locale/programs/ld-numeric.c
locale/programs/ld-time.c
locale/programs/locfile.c
misc/err.c
misc/error.c
nis/nis_addmember.c
nis/nis_call.c
nis/nis_findserv.c
nis/nis_subr.c
nis/nis_table.c
nis/nss_compat/compat-initgroups.c
nis/nss_nis/nis_initgroups.c
nis/nss_nisplus/nisplus-initgroups.c
nscd/grpcache.c
nscd/hstcache.c
nscd/nscd_initgroups.c
nscd/pwdcache.c
nscd/servicescache.c
nss/getXXbyYY.c
nss/getnssent.c
posix/glob.c
resolv/gai_misc.c
resolv/res_hconf.c

Perhaps some of them should use reallocarray to detect overflows. So I
guess these internal aliases should be fine.

But another question occurred to me: Should __libc_reallocarray be
listed in malloc/Versions and the abilist files? I would guess no
because this name should only be used internally, but most other memory
allocation functions do have their __libc_ prefixed name listed as part
of the public abi.