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

[Lennart Poettering]

On Sat, 23.08.14 22:39, Zbigniew Jędrzejewski-Szmek (zbys...@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
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

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

[Zbigniew Jędrzejewski-Szmek]

On Tue, Aug 26, 2014 at 09:47:54PM +0200, Lennart Poettering wrote:
 On Sat, 23.08.14 22:39, Zbigniew Jędrzejewski-Szmek (zbys...@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?
True, not necessary to fix the crash, but good for correctness.

 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...
Yeah, I noticed your commit for sd-journal.c.

I'll fix up the patch and push.

Zbyszek
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

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

[Zbigniew Jędrzejewski-Szmek]

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.

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;
-- 
1.9.3

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel