Source: whitedb
Version: 0.7.3-2
Severity: important
Tags: patch upstream

When a database field containing a long (over 32 bytes) string is overwritten with the same string, the reference count of the string is checked after decrementing (due to overwriting) and before incrementing (due to re-insert). This can cause prematurely freeing storage allocated to the string and may corrupt internal structures of the database.

Originally reported here: https://github.com/priitj/whitedb/issues/29

Patch attached,
thank you.
From 668a39cb85e1a9c3c63382a009c0706b67f99763 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Priit=20J=C3=A4rv?= <pr...@whitedb.org>
Date: Sat, 19 Aug 2017 22:07:24 +0300
Subject: [PATCH] skip field update if the new value equals the old value. This
 also works around a nasty longstr refcount bug (Issue #29)

---
 Db/dbdata.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/Db/dbdata.c b/Db/dbdata.c
index 2e84a8b..aff064f 100644
--- a/Db/dbdata.c
+++ b/Db/dbdata.c
@@ -3,7 +3,7 @@
 * $Version: $
 *
 * Copyright (c) Tanel Tammet 2004,2005,2006,2007,2008,2009
-* Copyright (c) Priit Järv 2009,2010,2011,2013,2014
+* Copyright (c) Priit Järv 2009,2010,2011,2013,2014,2017
 *
 * Contact: tanel.tam...@gmail.com
 *
@@ -636,6 +636,11 @@ wg_int wg_set_field(void* db, void* record, wg_int fieldnr, wg_int data) {
   fieldadr=((gint*)record)+RECORD_HEADER_GINTS+fieldnr;
   fielddata=*fieldadr;
 
+  /* Skip changes if the value does not change */
+  if(fielddata == data) {
+    return 0;
+  }
+
   /* Update index(es) while the old value is still in the db */
 #ifdef USE_INDEX_TEMPLATE
   if(!is_special_record(record) && fieldnr<=MAX_INDEXED_FIELDNR &&\
@@ -719,12 +724,6 @@ wg_int wg_set_field(void* db, void* record, wg_int fieldnr, wg_int data) {
 setfld_backlink_removed:
 #endif
 
-  //printf("wg_set_field adr %d offset %d\n",fieldadr,ptrtooffset(db,fieldadr));
-  if (isptr(fielddata)) {
-    //printf("wg_set_field freeing old data\n");
-    free_field_encoffset(db,fielddata);
-  }
-  (*fieldadr)=data; // store data to field
 #ifdef USE_CHILD_DB
   if (islongstr(data) && offset_owner == dbmemseg(db)) {
 #else
@@ -734,6 +733,12 @@ setfld_backlink_removed:
     strptr = (gint *) offsettoptr(db,decode_longstr_offset(data));
     ++(*(strptr+LONGSTR_REFCOUNT_POS));
   }
+  //printf("wg_set_field adr %d offset %d\n",fieldadr,ptrtooffset(db,fieldadr));
+  if (isptr(fielddata)) {
+    //printf("wg_set_field freeing old data\n");
+    free_field_encoffset(db,fielddata);
+  }
+  (*fieldadr)=data; // store data to field
 
   /* Update index after new value is written */
 #ifdef USE_INDEX_TEMPLATE
-- 
2.14.0

Reply via email to