[libvirt PATCH] logging: allow max_len=0 to disable log rollover

Daniel P. Berrangé posted 1 patch 3 years, 5 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20201013092158.80428-1-berrange@redhat.com
src/logging/virtlogd.conf  |  4 ++++
src/util/virrotatingfile.c | 48 +++++++++++++++++++++-----------------
2 files changed, 30 insertions(+), 22 deletions(-)
[libvirt PATCH] logging: allow max_len=0 to disable log rollover
Posted by Daniel P. Berrangé 3 years, 5 months ago
Currently setting max_len=0 causes virtlogd to spin in a busy loop. It
is natural to allow this to disable log rollover which can be useful for
developers debugging things.

Note disabling rollover exposes the host to denial of service from a
malicious guest, so must be used with care.

Closes https://gitlab.com/libvirt/libvirt/-/issues/85
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 src/logging/virtlogd.conf  |  4 ++++
 src/util/virrotatingfile.c | 48 +++++++++++++++++++++-----------------
 2 files changed, 30 insertions(+), 22 deletions(-)

diff --git a/src/logging/virtlogd.conf b/src/logging/virtlogd.conf
index 8b1ff0156f..c53a1112bd 100644
--- a/src/logging/virtlogd.conf
+++ b/src/logging/virtlogd.conf
@@ -87,6 +87,10 @@
 
 # Maximum file size before rolling over. Defaults to 2 MB
 #
+# Setting max_size to zero will disable rollover entirely.
+# NOTE: disabling rollover exposes the host filesystem to
+# denial of service from a malicious guest.
+#
 # Beware that a logrotate config file might be installed too,
 # to handle cases where virtlogd is disabled. To ensure that
 # the logrotate config is a no-op when virtlogd is running,
diff --git a/src/util/virrotatingfile.c b/src/util/virrotatingfile.c
index a88c332cf4..9f1ef17c3e 100644
--- a/src/util/virrotatingfile.c
+++ b/src/util/virrotatingfile.c
@@ -225,7 +225,8 @@ virRotatingFileWriterDelete(virRotatingFileWriterPtr file)
  *
  * The files will never exceed @maxlen bytes in size,
  * but may be rolled over before they reach this size
- * in order to avoid splitting lines
+ * in order to avoid splitting lines. If @maxlen is
+ * zero then no rollover will be performed.
  */
 virRotatingFileWriterPtr
 virRotatingFileWriterNew(const char *path,
@@ -430,25 +431,27 @@ virRotatingFileWriterAppend(virRotatingFileWriterPtr file,
         size_t towrite = len;
         bool forceRollover = false;
 
-        if (file->entry->pos > file->maxlen) {
-            /* If existing file is for some reason larger then max length we
-             * won't write to this file anymore, but we rollover this file.*/
-            forceRollover = true;
-            towrite = 0;
-        } else if ((file->entry->pos + towrite) > file->maxlen) {
-            towrite = file->maxlen - file->entry->pos;
-
-            /*
-             * If there's a newline in the last 80 chars
-             * we're about to write, then break at that
-             * point to avoid splitting lines across
-             * separate files
-             */
-            for (i = 0; i < towrite && i < 80; i++) {
-                if (buf[towrite - i - 1] == '\n') {
-                    towrite -= i;
-                    forceRollover = true;
-                    break;
+        if (file->maxlen != 0) {
+            if (file->entry->pos > file->maxlen) {
+                /* If existing file is for some reason larger then max length we
+                 * won't write to this file anymore, but we rollover this file.*/
+                forceRollover = true;
+                towrite = 0;
+            } else if ((file->entry->pos + towrite) > file->maxlen) {
+                towrite = file->maxlen - file->entry->pos;
+
+                /*
+                 * If there's a newline in the last 80 chars
+                 * we're about to write, then break at that
+                 * point to avoid splitting lines across
+                 * separate files
+                 */
+                for (i = 0; i < towrite && i < 80; i++) {
+                    if (buf[towrite - i - 1] == '\n') {
+                        towrite -= i;
+                        forceRollover = true;
+                        break;
+                    }
                 }
             }
         }
@@ -468,8 +471,9 @@ virRotatingFileWriterAppend(virRotatingFileWriterPtr file,
             file->entry->len += towrite;
         }
 
-        if ((file->entry->pos == file->maxlen && len) ||
-            forceRollover) {
+        if (file->maxlen != 0 &&
+            ((file->entry->pos == file->maxlen && len) ||
+             forceRollover)) {
             virRotatingFileWriterEntryPtr tmp;
             VIR_DEBUG("Hit max size %zu on %s (force=%d)",
                       file->maxlen, file->basepath, forceRollover);
-- 
2.26.2

Re: [libvirt PATCH] logging: allow max_len=0 to disable log rollover
Posted by Peter Krempa 3 years, 5 months ago
On Tue, Oct 13, 2020 at 10:21:58 +0100, Daniel Berrange wrote:
> Currently setting max_len=0 causes virtlogd to spin in a busy loop. It
> is natural to allow this to disable log rollover which can be useful for
> developers debugging things.
> 
> Note disabling rollover exposes the host to denial of service from a
> malicious guest, so must be used with care.
> 
> Closes https://gitlab.com/libvirt/libvirt/-/issues/85
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  src/logging/virtlogd.conf  |  4 ++++
>  src/util/virrotatingfile.c | 48 +++++++++++++++++++++-----------------
>  2 files changed, 30 insertions(+), 22 deletions(-)

Reviewed-by: Peter Krempa <pkrempa@redhat.com>