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 <la...@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é <anar...@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 > `. `'` la...@debian.org 🍥 chris-lamb.co.uk > `- > >
