Re: DB backend support for lmdb?

Hallo Matthias!

Matthias Andree wrote in <[email protected]>:
 |Am 17.01.19 um 19:06 schrieb Steffen Nurpmeso:
 |> Matthias Andree wrote in <[email protected]>:
 ...
 |> Due to some thread on a NetBSD ML i had a glance on the LMDB code
 |> and saw that one overflow check condition was not right.  It
 |> cannot happen in practice because LMDB key length restriction is
 |> far below 31-bit (511 bytes by default), but still one overflow
 |> could happen and would not be detected because of wraparound.
 |> This is fixed by the attached patch.  (Tests all fine but the
 |> skipped ones.)
 |>
 |> Do you have any plans on doing a new bogofilter release?  It is of
 |> course nothing but purely selfish ... but KyotoCabinet is also
 |> a new backend, and is lingering for some time, too.  If i diff
 |> from my imported master (v1.2.4), there are quite some changes.
 |> (I definetely have seen releases with fewer changes.)
 |>
 |> No git yet it seems, but v1.2.4 seems to be more than five years
 |> old, hm hm.  v1.2.5 would be cool, anyway!
 ...
 |indeed no Git yet, real-life work went crazy last autumn as in really
 |crazy, so I pushed everything back quite a bit.

Yes, of course.  You have been talking about it, that is why.
I normally do not have all the auto* series installed, so my
personal interest tends to look in direction pre-prepared ball...

 |Thanks for proactively maintaining and catching up your LMDB support so
 |we can refine it!

I was only fixing a bad overflow check, which should have been
avoided from the start.  If it would be that easy.

 |However, can I ask you to resend the patch without the many
 |reformattings? There's a lot of
 |
 |foo *=A0=A0=A0=A0 ->=A0=A0=A0 foo*
 |
 |reformatting going on that obscures the actual change.

Mist!  You recognized it!!!  I have now compiled with C++ compiler
by myself, to proactively avoid that you have to add more of that
terrible "foo *" syntax instead of my beautiful "foo*" notation.
Fixed in the below.

 |And yes, a new release is planned, but not all changes that I feel are
 |needed are in yet.

Ok.  But the silence on this list does not mean anything but that
people in Germany, and likely in Taiwan and such, are nonetheless
waiting and hoping for a new release.  ^_^

 |Thanks again!

Thank you Matthias.  I say Ciao! already here, i had forgotten
MIME attachments get scrubbed...

diff --git a/bogofilter/src/datastore_lmdb.c b/bogofilter/src/datastore_lmd=
b.c
index b8c4015..681db24 100644
--- a/bogofilter/src/datastore_lmdb.c
+++ b/bogofilter/src/datastore_lmdb.c
@@ -5,7 +5,7 @@
  * datastore_lmdb.c -- implements the datastore, using LMDB.
  *
  * AUTHORS:
- * Steffen Nurpmeso <[email protected]>    2018
+ * Steffen Nurpmeso <[email protected]>    2018, 2019
  * (copied from datastore_kc.c:
  * Gyepi Sam <[email protected]>          2003
  * Matthias Andree <[email protected]> 2003, 2018
@@ -24,11 +24,11 @@
  *    reaches the size limit, the transaction must be aborted, then the
  *    environment must be resized, then a new transaction has to be create=
d.
  *    Resizing will not shrink, effectively.
- * 3. We assume xmalloc() aborts if out of memory.
- * 4. We assume no token->leng actually exceeds int32_t.
- * 5. mdb_env_get_maxkeysize():
+ * 3. mdb_env_get_maxkeysize():
  *      Depends on the compile-time constant #MDB_MAXKEYSIZE. Default 511.
  *    We reject any keys which excess this.
+ * 4. We assume xmalloc() aborts if out of memory.
+ * 5. We assume no token->leng actually exceeds int32_t.
  *
  * In order to be able to deal with 2. we need to track all changes that a=
re
  * performed in a txn, so that in case we are running against the wall we =
are
@@ -114,8 +114,8 @@ struct a_bflm{
 struct a_bflm_txn_cache{
     struct a_bflm_txn_cache *bflmtc_last;   /* Up-to-date (stack usage) */
     struct a_bflm_txn_cache *bflmtc_next;   /* Needs to be build before us=
e! */
-    char *bflmtc_caster;    /* Current caster */
-    char *bflmtc_max;       /* Maximum usable byte, exclusive */
+    char *bflmtc_caster;                    /* Current caster */
+    char *bflmtc_max;                       /* ..imum usable byte, exclusi=
ve */
     /* Actually points to &self[1] TODO [0] or [8], dep. __STDC_VERSION__!=
 */
     char *bflmtc_data;
 };
@@ -615,19 +615,25 @@ a_bflm_txn_cache_put(struct a_bflm *bflmp, MDB_val *k=
ey, MDB_val *val_or_null){
     char const *emsg;
     size_t kl, vl, i;
 =

-    kl =3D key->mv_size;
+    if((kl =3D key->mv_size) >=3D 0x7FFFFFFFu)
+        goto jeoverflow;
     if(val_or_null !=3D NULL){
-        vl =3D val_or_null->mv_size;
-        i =3D (2 * sizeof(uint32_t)) + kl + vl;
+        if((vl =3D val_or_null->mv_size) >=3D 0x7FFFFFFFu)
+            goto jeoverflow;
+        if((i =3D kl + vl) >=3D 0x7FFFFFFFu - 2 * sizeof(uint32_t))
+            goto jeoverflow;
+        i +=3D 2 * sizeof(uint32_t);
     }else{
         vl =3D 0;
-        i =3D sizeof(uint32_t) + kl;
+        if((i =3D kl) >=3D 0x7FFFFFFFu - sizeof(uint32_t))
+            goto jeoverflow;
+        i +=3D sizeof(uint32_t);
     }
     i =3D a_BFLM_TXN_CACHE_ALIGN(i);
 =

     /* XXX We actually should abort() the program instead: cannot be handl=
ed */
-    if(kl >=3D 0x7FFFFFFFu || vl >=3D 0x7FFFFFFFu ||
-            i >=3D 0x7FFFFFFFu - sizeof(*bflmtcp)){
+    if(i >=3D 0x7FFFFFFFu - sizeof(*bflmtcp)){
+jeoverflow:
         emsg =3D "LMDB: entry too large to be stored";
         goto jleave;
     }

--steffen
|
|Der Kragenbaer,                The moon bear,
|der holt sich munter           he cheerfully and one by one
|einen nach dem anderen runter  wa.ks himself off
|(By Robert Gernhardt)
_______________________________________________
bogofilter-dev mailing list
[email protected]
https://www.bogofilter.org/mailman/listinfo/bogofilter-dev