[systemd-devel] [PATCH] journalctl: verify object size in enumerate_unique

[Lennart Poettering]

On Sat, 23.08.14 22:39, Zbigniew Jędrzejewski-Szmek (zbyszek at in.waw.pl) wrote:

> We assumed that objects in a unique chain are good enough,
> and only checked object type. But mmap code crashes when some object
> has zero size. This most likely is caused by a corrupted journal
> file, but we should fail gracefully.

Shouldn't we check the full field name too with memcmp()? I mean, if we
look at the length we really can also compare the string for good, no?

But looks good otherwise, except that I dont like "log_error()" being
invoked from a library. This should be downgraded to
"log_debug()". Libraries should never write anything to stdout/stderr by
default...
> 
> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=758392
> https://bugs.freedesktop.org/show_bug.cgi?id=82894
> ---
> I cannot reproduce the crash, but anyway, the check seems to be in order.
> 
>  src/journal/sd-journal.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/src/journal/sd-journal.c b/src/journal/sd-journal.c
> index 3840ee486f..a30db439f0 100644
> --- a/src/journal/sd-journal.c
> +++ b/src/journal/sd-journal.c
> @@ -2590,6 +2590,13 @@ _public_ int sd_journal_enumerate_unique(sd_journal *j, const void **data, size_
>                          return -EBADMSG;
>                  }
>  
> +                if (o->object.size < k) {
> +                        log_error("%s:offset " OFSfmt ": object has size %"PRIu64", expected at least %zu",
> +                                  j->unique_file->path, j->unique_offset,
> +                                  o->object.size, k);
> +                        return -EBADMSG;
> +                }
> +
>                  r = journal_file_object_keep(j->unique_file, o, j->unique_offset);
>                  if (r < 0)
>                          return r;


Lennart

-- 
Lennart Poettering, Red Hat