[systemd-devel] [Fwd: [PATCH] journal: fix dereferenced pointer in journal_file_rotate()]

[Sjoerd Simons]

On Mon, 2012-05-21 at 21:35 -0700, shawn wrote:
> > If journal_file_open() failed, due to (e.g.) -ENOSPC on open()
> > new_file might still be NULL.
> > 
> > On error, leave pointer to the old JournalFile (now closed),
> > and require caller to check for error approiately.
> > 
> > 	Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=43020
> > 	Reported-by: Sjoerd Simons <sjoerd at luon.net>

The bugzilla link seems wrong ? 

This actually remind me though, i did submit a patch for this issue to
bugzilla (slightly different then your solution) more then a month ago.
And a companion patch to not make the issue occur so easily, bugs filed
here:

https://bugs.freedesktop.org/show_bug.cgi?id=48688
https://bugs.freedesktop.org/show_bug.cgi?id=48685

If the systemd bugzilla is just somewhat of a decoy i'm happy to repost
the patches to the list ofcourse :)


> >  src/journal/journal-file.c |    9 ++++++++-
> >  1 file changed, 8 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/journal/journal-file.c b/src/journal/journal-file.c
> > index 5dd6e57..9f5f26e 100644
> > --- a/src/journal/journal-file.c
> > +++ b/src/journal/journal-file.c
> > @@ -1871,9 +1871,16 @@ int journal_file_rotate(JournalFile **f) {
> >          old_file->header->state = STATE_ARCHIVED;
> >  
> >          r = journal_file_open(old_file->path, old_file->flags, old_file->mode, old_file, &new_file);
> > -        journal_file_close(old_file);
> > +
> > +        if (r < 0) {
> > +                r = -errno;
> > +                goto finish;
> > +        }
> >  
> >          *f = new_file;
> > +
> > +finish:
> > +        journal_file_close(old_file);
> >          return r;
> >  }
> >