Package: release.debian.org
Severity: normal
Tags: stretch
User: release.debian....@packages.debian.org
Usertags: pu

I'd like to fix https://bugs.debian.org/912883 in stretch.

If changes to a new database which don't modify the termlist table
are committed, then a block which has been allocated to be the root
block in the termlist table gets leaked.  This case is triggered by
notmuch - the result is a database which is slightly larger than
it would otherwise be, but works fine *except* that consistency
checking with xapian-check/Database::check() detects there's an
unused block not on the freelist and reports this as
"DatabaseCorruptError" - this tends to alarm users.

While tracking down the above problem, I found a second case where
blocks can be leaked when cancel_transaction() is called.  I haven't
seen reports of this affecting anyone but the fix is trivial enough
that it seems worth fixing this as well.

* This bug is already fixed in unstable (also in testing and
  stretch-backports)
* The bug is of severity "important"
* Bug meta-data is up to date
* The fix is minimal and includes a detailed changelog entry
* A source debdiff of the proposed change is attached
* The proposed package is versioned correctly (+deb9u3)
* The patch was in upstream release 1.4.6 and has been in Debian
  unstable and testing for around 4 months.  It has been confirmed as
  fixing the problem with notmuch.  The patch includes regression
  tests.  The proposed update passes the extensive upstream testsuite
  which runs during the package build.
* The update was built in a stretch chroot
* This isn't a security issue so does not need coordination with the
  security team

Cheers,
    Olly
diff -Nru xapian-core-1.4.3/debian/changelog xapian-core-1.4.3/debian/changelog
--- xapian-core-1.4.3/debian/changelog  2018-08-13 18:19:13.000000000 +1200
+++ xapian-core-1.4.3/debian/changelog  2018-11-05 07:47:57.000000000 +1300
@@ -1,3 +1,11 @@
+xapian-core (1.4.3-2+deb9u3) stretch; urgency=medium
+
+  * fix-freelist-leaks.patch: Fix leaks of freelist blocks in corner cases
+    which then get reported as "DatabaseCorruptError" by Database::check().
+    (Closes: #912883)
+
+ -- Olly Betts <o...@survex.com>  Mon, 05 Nov 2018 07:47:57 +1300
+
 xapian-core (1.4.3-2+deb9u2) stretch; urgency=medium
 
   * fix-glass-cursor-bug.patch: Fix glass backend bug with long-lived cursors
diff -Nru xapian-core-1.4.3/debian/patches/fix-freelist-leaks.patch 
xapian-core-1.4.3/debian/patches/fix-freelist-leaks.patch
--- xapian-core-1.4.3/debian/patches/fix-freelist-leaks.patch   1970-01-01 
12:00:00.000000000 +1200
+++ xapian-core-1.4.3/debian/patches/fix-freelist-leaks.patch   2018-11-05 
07:47:51.000000000 +1300
@@ -0,0 +1,97 @@
+--- a/backends/glass/glass_table.cc
++++ b/backends/glass/glass_table.cc
+@@ -1640,6 +1640,7 @@
+           /* writing - */
+           SET_REVISION(p, revision_number + 1);
+           C[0].set_n(free_list.get_block(this, block_size));
++          C[0].rewrite = true;
+       }
+     } else {
+       /* using a root block stored on disk */
+@@ -1855,9 +1856,7 @@
+       }
+     }
+ 
+-    if (Btree_modified) {
+-      faked_root_block = false;
+-    }
++    faked_root_block = false;
+ }
+ 
+ void
+@@ -1946,6 +1945,13 @@
+     item_count =       root_info.get_num_entries();
+     faked_root_block = root_info.get_root_is_fake();
+     sequential =       root_info.get_sequential();
++    const string & fl_serialised = root_info.get_free_list();
++    if (!fl_serialised.empty()) {
++      if (!free_list.unpack(fl_serialised))
++          throw Xapian::DatabaseCorruptError("Bad freelist metadata");
++    } else {
++      free_list.reset();
++    }
+ 
+     Btree_modified = false;
+ 
+--- a/tests/api_backend.cc
++++ b/tests/api_backend.cc
+@@ -1555,3 +1555,23 @@
+     TEST_EQUAL(mset.get_matches_estimated() % 10, 0);
+     return true;
+ }
++
++/// Regression test for glass bug fixed in 1.4.6 and 1.5.0.
++DEFINE_TESTCASE(nodocs1, transactions && !remote) {
++    {
++      Xapian::WritableDatabase db = get_named_writable_database("nodocs1");
++      db.set_metadata("foo", "bar");
++      db.commit();
++      Xapian::Document doc;
++      doc.add_term("baz");
++      db.add_document(doc);
++      db.commit();
++    }
++
++    size_t check_errors =
++      Xapian::Database::check(get_named_writable_database_path("nodocs1"),
++                              Xapian::DBCHECK_SHOW_STATS, &tout);
++    TEST_EQUAL(check_errors, 0);
++
++    return true;
++}
+--- a/tests/api_transdb.cc
++++ b/tests/api_transdb.cc
+@@ -1,7 +1,7 @@
+ /** @file api_transdb.cc
+  * @brief tests requiring a database backend supporting transactions
+  */
+-/* Copyright (C) 2006,2009 Olly Betts
++/* Copyright (C) 2006,2009,2018 Olly Betts
+  *
+  * This program is free software; you can redistribute it and/or modify
+  * it under the terms of the GNU General Public License as published by
+@@ -126,3 +126,24 @@
+ 
+     return true;
+ }
++
++/// Regression test for glass bug fixed in 1.4.6 and 1.5.0.
++DEFINE_TESTCASE(canceltransaction3, transactions && !remote) {
++    {
++      Xapian::WritableDatabase db = 
get_named_writable_database("canceltransaction3");
++      db.begin_transaction();
++      Xapian::Document doc;
++      doc.add_term("baz");
++      db.add_document(doc);
++      db.cancel_transaction();
++      db.add_document(doc);
++      db.commit();
++    }
++
++    size_t check_errors =
++      
Xapian::Database::check(get_named_writable_database_path("canceltransaction3"),
++                              Xapian::DBCHECK_SHOW_STATS, &tout);
++    TEST_EQUAL(check_errors, 0);
++
++    return true;
++}
diff -Nru xapian-core-1.4.3/debian/patches/series 
xapian-core-1.4.3/debian/patches/series
--- xapian-core-1.4.3/debian/patches/series     2018-08-13 18:14:06.000000000 
+1200
+++ xapian-core-1.4.3/debian/patches/series     2018-11-05 07:46:50.000000000 
+1300
@@ -1,3 +1,4 @@
 fix-unweighted-and.patch
 cve-2018-0499-mset-snippet-escaping.patch
 fix-glass-cursor-bug.patch
+fix-freelist-leaks.patch

Attachment: signature.asc
Description: PGP signature

Reply via email to