Serious bug in arenas_extend_ctl

Mike Hommey mh+jemalloc at glandium.org
Mon Nov 26 05:19:36 PST 2012


On Mon, Nov 26, 2012 at 12:15:44PM +0100, Mike Hommey wrote:
> On Mon, Nov 26, 2012 at 12:09:01PM +0100, Mike Hommey wrote:
> > Hi,
> > 
> > Version 3.2 fails to build on windows, which thankfully made me spot
> > this horrible bug in ctl.c:
> > 
> > 1502         READ(ctl_stats.narenas - 1, unsigned);
> > 
> > This expands to:
> > (...) memcpy(oldp, (void *)&ctl_stats.narenas - 1, copylen); (...)
> > 
> > Which obviously doesn't do the right thing on other platforms.
> 
> This small patch would avoid such mistakes to go unnoticed:
> --- a/src/ctl.c
> +++ b/src/ctl.c
> @@ -960,7 +960,7 @@ ctl_postfork_child(void)
>                 if (*oldlenp != sizeof(t)) {                            \
>                         size_t  copylen = (sizeof(t) <= *oldlenp)       \
>                             ? sizeof(t) : *oldlenp;                     \
> -                       memcpy(oldp, (void *)&v, copylen);              \
> +                       memcpy(oldp, (void *)&(v), copylen);            \
>                         ret = EINVAL;                                   \
>                         goto label_return;                              \
>                 } else                                                  \
> 
> 
> And this should fix the issue itself:
> 
> --- a/src/ctl.c
> +++ b/src/ctl.c
> @@ -1499,7 +1499,8 @@ arenas_extend_ctl(const size_t *mib, size_t miblen, void *oldp, size_t *oldlenp,
>                 ret = EAGAIN;
>                 goto label_return;
>         }
> -       READ(ctl_stats.narenas - 1, unsigned);
> +       unsigned n = ctl_stats.narenas - 1;

Note the variable declaration needs to go at the beginning of the
function.



More information about the jemalloc-discuss mailing list