[PATCH] create_hashtable together with version_record (was: trustdb locking)

[NIIBE Yutaka]

On 06/13/2016 11:34 AM, NIIBE Yutaka wrote:
> We have this bug in 1.4, 2.0, and 2.1.  My intention is to fix
> in 2.1 and backporting fix to 1.4 and 2.0.

Well, fixing all serialization problems of trustdb access requires
major change.

I think that:

    If we rely on the fact that write(2) is atomic, the size of
    records should be 64 (or some number which is power of 2).
    Currently, the size is 40, so, I think that in some specific cases
    around a boundary of a block, it won't be atomic.

    Fine grained lock requires many change of the code.

    Easier way would be introducing something like a giant lock, which
    allows the access to trustdb by a single process.

Many use-cases of GPG are interactive.  Especially, trustdb write
access is rare, and it is happened in an interactive use mainly.
Because of that, fixing all serialization problems wouldn't be high
priority, perhaps.

Even so, the situation of the issue 1675 is important.  It occurs when
a user only does --verify (it's not the intentional modification of
trustdb, trust or validity).  It is about the first time access to
trustdb, and it might result not only a failure by another process
(demonstrated by my email yesterday), but also a corruption of
trustdb.


Since I believe that it is so important to be fixed soon, I propose
following patch (for 2.1).

This patch moves create_hashtable into create_version_record.  Thus,
initial creation of trustdb will be always 1200-byte with version
record and hash table.  Since initial creation of trustdb is already
serialized properly, it fixes a problem of the serialization of
trustdb access in this particular case.

diff --git a/g10/tdbio.c b/g10/tdbio.c
index a414709..9e35fcc 100644
--- a/g10/tdbio.c
+++ b/g10/tdbio.c
@@ -119,6 +119,7 @@ static int in_transaction;

 

 static void open_db (void);
+static void create_hashtable (TRUSTREC *vr, int type);


 

@@ -582,8 +583,13 @@ create_version_record (void)
   rec.rectype = RECTYPE_VER;
   rec.recnum = 0;
   rc = tdbio_write_record (&rec);
+
   if (!rc)
     tdbio_sync ();
+
+  if (!rc)
+    create_hashtable (&rec, 0);
+
   return rc;
 }

@@ -957,8 +963,6 @@ get_trusthashrec(void)
       if (rc)
         log_fatal (_("%s: error reading version record: %s\n"),
                    db_name, gpg_strerror (rc) );
-      if (!vr.r.ver.trusthashtbl)
-        create_hashtable (&vr, 0);

       trusthashtbl = vr.r.ver.trusthashtbl;
     }
--