Merge ~paelzer/ubuntu/+source/qemu:fix-1820291-freeze-hook-and-CVE into ~ubuntu-virt/ubuntu/+source/qemu:ubuntu-disco-3.1

[Christian Ehrhardt ]

[Robie Basak (racb)]

AFAICT the implementation of your conffile rename is necessary and correct.

I am somewhat confused by the approach you took though, which seems to have three different "what to minimise" choices in three different cases. I've left comments inline in an effort to explain what I mean. However, I don't think it's worth worrying about, since this upgrade path code will go away soon anyway, you have tested the different cases with your implementation here carefully, and I don't see any functional problems with your approach as dpkg-maintscript-helper and dpkg are written today.

Separately I suggested one minor changelog fixup (inline), which I leave to your discretion in case I'm missing something.

review: Approve

[Andreas Hasenack (ahasenack)]

review: Abstain

[Robie Basak (racb)]

(oh, and I did review the CVE fix, which looks fine, matching upstream, etc)

[Christian Ehrhardt  (paelzer)]

Thanks for the review!
I have made quite some cleanups due to that and I think it is better now.
I'll reset and to the testing again, once good I'll ping for a re-review.

[Christian Ehrhardt  (paelzer)]

BTW - the PPA to test is:
https://launchpad.net/~paelzer/+archive/ubuntu/bug-1820291-fix-fsfreeze

[Christian Ehrhardt  (paelzer)]

tested and pushed the new version

[Christian Ehrhardt  (paelzer)]

@rbasak - would you mind to do a quick re-review?

[Robie Basak (racb)]

review: Approve

[Christian Ehrhardt  (paelzer)]

Thanks, I proposed it to Debian as well now [1] - and give them a chance to reply so that we handle it similarly.
Likely uploading tomorrow to meet the Beta freeze in time.

[1]: https://salsa.debian.org/qemu-team/qemu/merge_requests/5

[Christian Ehrhardt  (paelzer)]

Building at https://launchpad.net/ubuntu/+source/qemu/1:3.1+dfsg-2ubuntu3
Setting merged.

diff --git a/debian/changelog b/debian/changelog
index 4652669..6430822 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,17 @@
+qemu (1:3.1+dfsg-2ubuntu3) disco; urgency=medium
+
+  * qemu-guest-agent: fix path of fsfreeze-hook (LP: #1820291)
+    - d/qemu-guest-agent.install: use correct path for fsfreeze-hook
+    - d/qemu-guest-agent.pre{rm|inst}/.postrm: special handling for
+      mv_conffile since the new path is a directory in the old package
+      version which can not be handled by mv_conffile.
+  * i2c-ddc-fix-oob-read-CVE-2019-3812.patch fixes
+    OOB read in hw/i2c/i2c-ddc.c which allows for memory disclosure.
+    Closes: #922635 (Thanks to Gerd Hoffmann and Michael Tokarev)
+    CVE-2019-3812
+
+ -- Christian Ehrhardt <[e-mail]>  Mon, 18 Mar 2019 09:20:07 +0100
+
 qemu (1:3.1+dfsg-2ubuntu2) disco; urgency=medium
 
   * disable pvrdma - besides several security holes there are many other
diff --git a/debian/patches/i2c-ddc-fix-oob-read-CVE-2019-3812.patch b/debian/patches/i2c-ddc-fix-oob-read-CVE-2019-3812.patch
new file mode 100644
index 0000000..628696b
--- /dev/null
+++ b/debian/patches/i2c-ddc-fix-oob-read-CVE-2019-3812.patch
@@ -0,0 +1,34 @@
+From: Gerd Hoffmann <[e-mail]>
+Date: Tue, 8 Jan 2019 11:23:01 +0100
+Subject: i2c-ddc: fix oob read
+Commit-Id: b05b267840515730dbf6753495d5b7bd8b04ad1c
+Bug-Debian: https://bugs.debian.org/922635
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: 8bit
+
+Suggested-by: Michael Hanselmann <[e-mail]>
+Signed-off-by: Gerd Hoffmann <[e-mail]>
+Reviewed-by: Michael Hanselmann <[e-mail]>
+Reviewed-by: Philippe Mathieu-Daudé <[e-mail]>
+Message-id: [e-mail]
+---
+ hw/i2c/i2c-ddc.c | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/hw/i2c/i2c-ddc.c b/hw/i2c/i2c-ddc.c
+index be34fe072c..0a0367ff38 100644
+--- a/hw/i2c/i2c-ddc.c
++++ b/hw/i2c/i2c-ddc.c
+@@ -56,7 +56,7 @@ static int i2c_ddc_rx(I2CSlave *i2c)
+     I2CDDCState *s = I2CDDC(i2c);
+ 
+     int value;
+-    value = s->edid_blob[s->reg];
++    value = s->edid_blob[s->reg % sizeof(s->edid_blob)];
+     s->reg++;
+     return value;
+ }
+-- 
+2.11.0
+
diff --git a/debian/patches/series b/debian/patches/series
index fbe83e2..54769a7 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -5,6 +5,7 @@ bt-use-size_t-type-for-length-parameters-instead-of-int-CVE-2018-19665.patch
 hw_usb-fix-mistaken-de-initialization-of-CCID-state.patch
 scsi-generic-avoid-possible-oob-access-to-r-buf-CVE-2019-6501.patch
 slirp-check-data-length-while-emulating-ident-function-CVE-2019-6778.patch
+i2c-ddc-fix-oob-read-CVE-2019-3812.patch
 
 # ubuntu patches
 ubuntu/expose-vmx_qemu64cpu.patch
diff --git a/debian/qemu-guest-agent.install b/debian/qemu-guest-agent.install
index 8bf0389..7049413 100644
--- a/debian/qemu-guest-agent.install
+++ b/debian/qemu-guest-agent.install
@@ -3,4 +3,4 @@ debian/tmp/usr/share/man/man8/qemu-ga.8 /usr/share/man/man8
 debian/tmp/usr/share/man/man7/qemu-ga-ref.7 /usr/share/man/man7
 debian/tmp/usr/share/doc/qemu/qemu-ga-ref.* /usr/share/doc/qemu-guest-agent
 qga/qapi-schema.json /usr/share/doc/qemu-guest-agent
-scripts/qemu-guest-agent/fsfreeze-hook /etc/qemu/fsfreeze-hook
+scripts/qemu-guest-agent/fsfreeze-hook /etc/qemu/
diff --git a/debian/qemu-guest-agent.postinst b/debian/qemu-guest-agent.postinst
new file mode 100644
index 0000000..925b748
--- /dev/null
+++ b/debian/qemu-guest-agent.postinst
@@ -0,0 +1,59 @@
+#!/bin/sh
+# postinst script for qemu-guest-agent
+#
+# see: dh_installdeb(1)
+
+set -e
+
+# summary of how this script can be called:
+#        * <postinst> `configure' <most-recently-configured-version>
+#        * <old-postinst> `abort-upgrade' <new version>
+#        * <conflictor's-postinst> `abort-remove' `in-favour' <package>
+#          <new-version>
+#        * <postinst> `abort-remove'
+#        * <deconfigured's-postinst> `abort-deconfigure' `in-favour'
+#          <failed-install-package> <version> `removing'
+#          <conflicting-package> <version>
+# for details, see https://www.debian.org/doc/debian-policy/ or
+# the debian-policy package
+
+
+case "$1" in
+    configure)
+    ;;
+
+    abort-upgrade|abort-remove|abort-deconfigure)
+    ;;
+
+    *)
+        echo "postinst called with unknown argument \`$1'" >&2
+        exit 1
+    ;;
+esac
+
+# dh_installdeb will replace this with shell code automatically
+# generated by other debhelper scripts.
+
+#DEBHELPER#
+
+# Normal mv_conffile alone would fail due to the new path being a DIR in the old package version (LP: 1820291)
+case "$1" in
+    configure)
+        # From /usr/bin/dpkg-maintscript-helper modified to be able to cope with this edge case
+        if [ -n "$2" ] && dpkg --compare-versions -- "$2" le-nl "1:3.1+dfsg-2ubuntu3~"; then
+            TMPCONFFILE="/etc/qemu/fsfreeze-hook.old"
+            NEWCONFFILE="/etc/qemu/fsfreeze-hook"
+            ORIGCONFFILE="/etc/qemu/fsfreeze-hook/fsfreeze-hook"
+            rm -f "$TMPCONFFILE.dpkg-remove"
+            if [ -e "$TMPCONFFILE" ]; then
+                echo "Preserving user changes to $NEWCONFFILE (renamed from $ORIGCONFFILE)..."
+                if [ -e "$NEWCONFFILE" ]; then
+                    mv -f "$NEWCONFFILE" "$NEWCONFFILE.dpkg-new"
+                fi
+                mv -f "$TMPCONFFILE" "$NEWCONFFILE"
+            fi
+        fi
+        ;;
+esac
+
+exit 0
diff --git a/debian/qemu-guest-agent.postrm b/debian/qemu-guest-agent.postrm
new file mode 100644
index 0000000..14abbb8
--- /dev/null
+++ b/debian/qemu-guest-agent.postrm
@@ -0,0 +1,56 @@
+#!/bin/sh
+# postrm script for qemu-guest-agent
+#
+# see: dh_installdeb(1)
+
+set -e
+
+# summary of how this script can be called:
+#        * <postrm> `remove'
+#        * <postrm> `purge'
+#        * <old-postrm> `upgrade' <new-version>
+#        * <new-postrm> `failed-upgrade' <old-version>
+#        * <new-postrm> `abort-install'
+#        * <new-postrm> `abort-install' <old-version>
+#        * <new-postrm> `abort-upgrade' <old-version>
+#        * <disappearer's-postrm> `disappear' <overwriter>
+#          <overwriter-version>
+# for details, see https://www.debian.org/doc/debian-policy/ or
+# the debian-policy package
+
+
+case "$1" in
+    purge|remove|upgrade|failed-upgrade|abort-install|abort-upgrade|disappear)
+    ;;
+
+    *)
+        echo "postrm called with unknown argument \`$1'" >&2
+        exit 1
+    ;;
+esac
+
+# dh_installdeb will replace this with shell code automatically
+# generated by other debhelper scripts.
+
+#DEBHELPER#
+
+# If needed revert the move we have made in preinst to compensate the new path being a DIR in the old package version (LP: 1820291)
+case "$1" in
+    abort-install|abort-upgrade)
+        # From /usr/bin/dpkg-maintscript-helper modified to be able to cope with this edge case
+        if [ -n "$2" ] && dpkg --compare-versions -- "$2" le-nl "1:3.1+dfsg-2ubuntu3~"; then
+            TMPCONFFILE="/etc/qemu/fsfreeze-hook.old"
+            NEWCONFFILE="/etc/qemu/fsfreeze-hook"
+            ORIGCONFFILE="/etc/qemu/fsfreeze-hook/fsfreeze-hook"
+            if [ -e "$TMPCONFFILE.dpkg-remove" ]; then
+                echo "Reinstalling $ORIGCONFFILE that was moved away"
+                if [ -f "$NEWCONFFILE" ]; then
+                    rm -f "$NEWCONFFILE"
+                fi
+                mkdir -p "$NEWCONFFILE"
+                mv "$TMPCONFFILE.dpkg-remove" "$ORIGCONFFILE"
+            fi
+        fi
+esac
+
+exit 0
diff --git a/debian/qemu-guest-agent.preinst b/debian/qemu-guest-agent.preinst
new file mode 100644
index 0000000..4f47675
--- /dev/null
+++ b/debian/qemu-guest-agent.preinst
@@ -0,0 +1,62 @@
+#!/bin/sh
+# preinst script for qemu-guest-agent
+#
+# see: dh_installdeb(1)
+
+set -e
+
+# summary of how this script can be called:
+#        * <new-preinst> `install'
+#        * <new-preinst> `install' <old-version>
+#        * <new-preinst> `upgrade' <old-version>
+#        * <old-preinst> `abort-upgrade' <new-version>
+# for details, see https://www.debian.org/doc/debian-policy/ or
+# the debian-policy package
+
+
+case "$1" in
+    install|upgrade)
+    ;;
+
+    abort-upgrade)
+    ;;
+
+    *)
+        echo "preinst called with unknown argument \`$1'" >&2
+        exit 1
+    ;;
+esac
+
+# dh_installdeb will replace this with shell code automatically
+# generated by other debhelper scripts.
+
+# Normal mv_conffile alone would fail due to the new path being a DIR in the old package version (LP: 1820291)
+case "$1" in
+    install|upgrade)
+        # From /usr/bin/dpkg-maintscript-helper modified to be able to cope with this edge case
+        if [ -n "$2" ] && dpkg --compare-versions -- "$2" le-nl "1:3.1+dfsg-2ubuntu3~"; then
+            TMPCONFFILE="/etc/qemu/fsfreeze-hook.old"
+            NEWCONFFILE="/etc/qemu/fsfreeze-hook"
+            ORIGCONFFILE="/etc/qemu/fsfreeze-hook/fsfreeze-hook"
+            if [ -f "$ORIGCONFFILE" ]; then
+                disk_md5sum="$(md5sum "$ORIGCONFFILE" | sed -e 's/ .*//')"
+                pkg_md5sum="$(dpkg-query -W -f='${Conffiles}' "qemu-guest-agent" | \
+                    sed -n -e "\'^ $ORIGCONFFILE ' { s/ obsolete$//; s/.* //; p }")"
+                if [ "$disk_md5sum" = "$pkg_md5sum" ]; then
+                    # mark as having no custom content
+                    mv -f "$ORIGCONFFILE" "${TMPCONFFILE}.dpkg-remove"
+                else
+                    # keep the "old" name to reflect there is content to be preserved
+                    mv -f "$ORIGCONFFILE" "$TMPCONFFILE"
+                fi
+                # In any case the old directory blocking the new conffile
+                # has to be removed before unpack happens
+                rmdir "$NEWCONFFILE" || echo "failed to remove $NEWCONFFILE"
+            fi
+        fi
+        ;;
+esac
+
+#DEBHELPER#
+
+exit 0