[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: Serious regression in systemd 215-17+deb8u10



Hi,

If I would have to guess, the difference is that strappenda use alloca, which then use the stack to allocate the string. And the stack is freed when the function exists, in contrast with allocating memory in heap. maybe a free(x) is missing from after the IOVEC_SET_STRING

On Wed, Mar 6, 2019 at 8:57 PM Chris Lamb <lamby@debian.org> wrote:
Hi Dan,

> We have discovered that the latest version of systemd uploaded to
> jessie is causing systemd-journald to use an ever increasing amount of
> memory, eventually leading to all available memory being consumed. This
> has been observed on multiple different systems we use which are
> logging quite heavily and have upgraded to the latest systemd package.
> We have some systems which haven't had the new security patches applied
> and are not observing this behaviour - there is a clear correlation
> with the latest version of the systemd package.

So, this is the patch that was applied in systemd 215-17+deb8u10:

    Description: journald: do not store the iovec entry for process commandline on stack
     This fixes a crash (CVE-2018-16864) where we
     would read the commandline, whose length is under control of the
     sending program, and then crash when trying to create a stack
     allocation for it.
     .
     This is a backport of https://github.com/systemd/systemd/commit/084eeb865ca63887098e0945fb4e93c852b91b0f
    Author: Antoine Beaupré <anarcat@debian.org>
    Bug-Debian: https://bugs.debian.org/918841
    Origin: Debian
    Bug: https://bugzilla.redhat.com/show_bug.cgi?id=1653855
    Forwarded: not-needed
    Last-Update: 2019-01-22

    --- systemd-215.orig/src/journal/journald-server.c
    +++ systemd-215/src/journal/journald-server.c
    @@ -602,7 +602,10 @@ static void dispatch_message_real(

                     r = get_process_cmdline(ucred->pid, 0, false, &t);
                     if (r >= 0) {
    -                        x = strappenda("_CMDLINE=", t);
    +                        /* At most _SC_ARG_MAX (2MB usually), which is
    +                         * too much to put on stack. Let's use a heap
    +                         * allocation for this one. */
    +                        x = strappend("_CMDLINE=", t);
                             free(t);
                             IOVEC_SET_STRING(iovec[n++], x);
                     }
    @@ -716,7 +719,9 @@ static void dispatch_message_real(

                     r = get_process_comm(object_pid, &t);
                     if (r >= 0) {
    -                        x = strappenda("OBJECT_COMM=", t);
    +                        /* See above for size limits, only ->cmdline
    +                         * may be large, so use a heap allocation for it. */
    +                        x = strappend("OBJECT_COMM=", t);
                             free(t);
                             IOVEC_SET_STRING(iovec[n++], x);
                     }

I can't immediately see what's up, nor the direct relevance to the
upstream changes listed on #920018, however.


Regards,

--
      ,''`.
     : :'  :     Chris Lamb
     `. `'`      lamby@debian.org 🍥 chris-lamb.co.uk
       `-


Reply to: