From f71f0f4f8d07b09fab205bf4d1cbd9dbc59074fc Mon Sep 17 00:00:00 2001
From: Noriko Hosoi <nhosoi@redhat.com>
Date: Sun, 12 Feb 2017 17:26:46 -0800
Subject: [PATCH] Ticket #49121 - ns-slapd crashes in ldif_sput due to the
output buf size is less than the real size.
Description: There were missing pieces in the entry size calculation
when an attribute had no a_present_values nor a_deleted_values.
1) There was no chance to add the size of the attribute type name since
preceding entry2str_internal_size_valueset did not add any size if
the value was empty. The type name size is now explicitly added.
2) a_deletioncsn is added in entry2str_internal_put_attrlist by calling
valueset_add_string with empty value. The size was not included in
the allocated memory to store the entire entry as a string. Now the
size is added.
Adding CI test ticket49121_test.py.
https://pagure.io/389-ds-base/issue/49121
Reviewed by wibrown@redhat.com (Thank you, William!!)
(cherry picked from commit 543fe89edb0a6410a740a4fff738cace7bc57078)
---
dirsrvtests/tests/data/ticket49121/utf8str.txt | 1 +
dirsrvtests/tests/tickets/ticket49121_test.py | 211 +++++++++++++++++++++++++
ldap/servers/slapd/entry.c | 55 ++++---
3 files changed, 244 insertions(+), 23 deletions(-)
create mode 100644 dirsrvtests/tests/data/ticket49121/utf8str.txt
create mode 100644 dirsrvtests/tests/tickets/ticket49121_test.py
diff --git a/dirsrvtests/tests/data/ticket49121/utf8str.txt b/dirsrvtests/tests/data/ticket49121/utf8str.txt
new file mode 100644
index 0000000..0005c4e
--- /dev/null
+++ b/dirsrvtests/tests/data/ticket49121/utf8str.txt
@@ -0,0 +1 @@
+あいうえお
diff --git a/dirsrvtests/tests/tickets/ticket49121_test.py b/dirsrvtests/tests/tickets/ticket49121_test.py
new file mode 100644
index 0000000..6450297
--- /dev/null
+++ b/dirsrvtests/tests/tickets/ticket49121_test.py
@@ -0,0 +1,211 @@
+# --- BEGIN COPYRIGHT BLOCK ---
+# Copyright (C) 2017 Red Hat, Inc.
+# All rights reserved.
+#
+# License: GPL (version 3 or any later version).
+# See LICENSE for details.
+# --- END COPYRIGHT BLOCK ---
+#
+import pytest
+import sys
+import codecs
+from lib389.tasks import *
+from lib389.utils import *
+from lib389.topologies import topology_m2
+
+DEBUGGING = os.getenv('DEBUGGING', False)
+
+if DEBUGGING:
+ logging.getLogger(__name__).setLevel(logging.DEBUG)
+else:
+ logging.getLogger(__name__).setLevel(logging.INFO)
+log = logging.getLogger(__name__)
+
+
+def test_ticket49121(topology_m2):
+ """
+ Creating some users.
+ Deleting quite a number of attributes which may or may not be in the entry.
+ The attribute type names are to be long.
+ Under the conditions, it did not estimate the size of string format entry
+ shorter than the real size and caused the Invalid write / server crash.
+ """
+ reload(sys)
+ sys.setdefaultencoding('utf-8')
+ log.info('DefaultEncoding: %s' % sys.getdefaultencoding())
+
+ utf8file = os.path.join(topology_m2.ms["master1"].getDir(__file__, DATA_DIR), "ticket49121/utf8str.txt")
+ utf8obj = codecs.open(utf8file, 'r', 'utf-8')
+ utf8strorig = utf8obj.readline()
+ utf8str = utf8strorig.encode('utf-8').rstrip('\n')
+ utf8obj.close()
+ assert(utf8str)
+
+ # Get the sbin directory so we know where to replace 'ns-slapd'
+ sbin_dir = topology_m2.ms["master1"].get_sbin_dir()
+ log.info('sbin_dir: %s' % sbin_dir)
+
+ # stop M1 to do the next updates
+ topology_m2.ms["master1"].stop(30)
+ topology_m2.ms["master2"].stop(30)
+
+ # wait for the servers shutdown
+ time.sleep(5)
+
+ # Enable valgrind
+ if not topology_m2.ms["master1"].has_asan():
+ valgrind_enable(sbin_dir)
+
+ # start M1 to do the next updates
+ topology_m2.ms["master1"].start()
+ topology_m2.ms["master2"].start()
+
+ for idx in range(1, 10):
+ try:
+ USER_DN = 'CN=user%d,ou=People,%s' % (idx, DEFAULT_SUFFIX)
+ log.info('adding user %s...' % (USER_DN))
+ topology_m2.ms["master1"].add_s(Entry((USER_DN,
+ {'objectclass': 'top person extensibleObject'.split(' '),
+ 'cn': 'user%d' % idx,
+ 'sn': 'SN%d-%s' % (idx, utf8str)})))
+ except ldap.LDAPError as e:
+ log.fatal('Failed to add user (%s): error %s' % (USER_DN, e.message['desc']))
+ assert False
+
+ for i in range(1, 3):
+ time.sleep(3)
+ for idx in range(1, 10):
+ try:
+ USER_DN = 'CN=user%d,ou=People,%s' % (idx, DEFAULT_SUFFIX)
+ log.info('[%d] modify user %s - replacing attrs...' % (i, USER_DN))
+ topology_m2.ms["master1"].modify_s(
+ USER_DN, [(ldap.MOD_REPLACE, 'cn', 'user%d' % idx),
+ (ldap.MOD_REPLACE, 'ABCDEFGH_ID', ['239001ad-06dd-e011-80fa-c00000ad5174',
+ '240f0878-c552-e411-b0f3-000006040037']),
+ (ldap.MOD_REPLACE, 'attr1', 'NEW_ATTR'),
+ (ldap.MOD_REPLACE, 'attr20000000000000000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr30000000000000000000000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr40000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr50000000000000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr600000000000000000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr7000000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr8000000000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr900000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr1000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr110000000000000', None),
+ (ldap.MOD_REPLACE, 'attr120000000000000', None),
+ (ldap.MOD_REPLACE, 'attr130000000000000000000000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr140000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr150000000000000000000000000000000000000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr1600000000000000000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr17000000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr18000000000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr1900000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr2000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr210000000000000000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr220000000000000000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr230000000000000000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr240000000000000000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr25000000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr260000000000000000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr270000000000000000000000000000000000000000000000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr280000000000000000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr29000000000000000000000000000000000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr3000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr310000000000000000000000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr320000000000000000000000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr330000000000000000000000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr340000000000000000000000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr350000000000000000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr360000000000000000000000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr370000000000000000000000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr380000000000000000000000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr390000000000000000000000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr4000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr410000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr420000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr430000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr440000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr4500000000000000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr460000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr470000000000000000000000000000000000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr480000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr49000000000000000000000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr5000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr510000000000000', None),
+ (ldap.MOD_REPLACE, 'attr520000000000000', None),
+ (ldap.MOD_REPLACE, 'attr530000000000000', None),
+ (ldap.MOD_REPLACE, 'attr540000000000000', None),
+ (ldap.MOD_REPLACE, 'attr550000000000000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr5600000000000000000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr57000000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr58000000000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr5900000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr6000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr6100000000000000000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr6200000000000000000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr6300000000000000000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr6400000000000000000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr65000000000000000000000000000000000000000000000000000000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr6600000000000000000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr6700000000000000000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr6800000000000000000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr690000000000000000000000000000000000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr7000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr71000000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr72000000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr73000000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr74000000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr750000000000000000000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr7600000000000000000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr77000000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr78000000000000000000000000000000000000000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr79000000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr800000000000000000000000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr81000000000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr82000000000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr83000000000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr84000000000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr85000000000000000000000000000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr8600000000000000000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr87000000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr88000000000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr89000000000000000000000000000000000', None),
+ (ldap.MOD_REPLACE, 'attr9000000000000000000000000000000000000000000000000000', None)])
+ except ldap.LDAPError as e:
+ log.fatal('Failed to modify user - deleting attrs (%s): error %s' % (USER_DN, e.message['desc']))
+
+ if not topology_m2.ms["master1"].has_asan():
+ results_file = valgrind_get_results_file(topology_m2.ms["master1"])
+
+ # Stop master2
+ topology_m2.ms["master1"].stop(30)
+ topology_m2.ms["master2"].stop(30)
+
+ # Check for leak
+ if not topology_m2.ms["master1"].has_asan():
+ # Check for invalid read/write
+ if valgrind_check_file(results_file, VALGRIND_INVALID_STR):
+ log.info('Valgrind reported invalid!')
+ assert False
+ else:
+ log.info('Valgrind is happy!')
+
+ # Disable valgrind
+ if not topology_m2.ms["master1"].has_asan():
+ valgrind_disable(sbin_dir)
+
+ # start M1 to do the next updates
+ topology_m2.ms["master1"].start()
+ topology_m2.ms["master2"].start()
+
+ log.info('Testcase PASSED')
+ if DEBUGGING:
+ # Add debugging steps(if any)...
+ pass
+
+if __name__ == '__main__':
+ # Run isolated
+ # -s for DEBUG mode
+ CURRENT_FILE = os.path.realpath(__file__)
+ pytest.main("-s %s" % CURRENT_FILE)
diff --git a/ldap/servers/slapd/entry.c b/ldap/servers/slapd/entry.c
index 09671a4..a2fd44a 100644
--- a/ldap/servers/slapd/entry.c
+++ b/ldap/servers/slapd/entry.c
@@ -1436,7 +1436,8 @@ bail:
}
static size_t
-entry2str_internal_size_valueset( const char *attrtype, const Slapi_ValueSet *vs, int entry2str_ctrl, int attribute_state, int value_state )
+entry2str_internal_size_valueset( const Slapi_Attr *a, const char *attrtype, const Slapi_ValueSet *vs,
+ int entry2str_ctrl, int attribute_state, int value_state )
{
size_t elen= 0;
if(!valueset_isempty(vs))
@@ -1449,6 +1450,12 @@ entry2str_internal_size_valueset( const char *attrtype, const Slapi_ValueSet *vs
attribute_state, value_state );
}
}
+ if(entry2str_ctrl & SLAPI_DUMP_STATEINFO) {
+ /* ";adcsn-" + a->a_deletioncsn */
+ if ( a && a->a_deletioncsn ) {
+ elen += 1 + LDIF_CSNPREFIX_MAXLENGTH + CSN_STRSIZE;
+ }
+ }
return elen;
}
@@ -1465,30 +1472,34 @@ entry2str_internal_size_attrlist( const Slapi_Attr *attrlist, int entry2str_ctrl
continue;
/* Count the space required for the present and deleted values */
- elen+= entry2str_internal_size_valueset(a->a_type, &a->a_present_values,
- entry2str_ctrl, attribute_state,
- VALUE_PRESENT);
- if(entry2str_ctrl & SLAPI_DUMP_STATEINFO)
- {
- elen+= entry2str_internal_size_valueset(a->a_type, &a->a_deleted_values,
- entry2str_ctrl, attribute_state,
- VALUE_DELETED);
- /* ";adcsn-" + a->a_deletioncsn */
- if ( a->a_deletioncsn )
- {
- elen += 1 + LDIF_CSNPREFIX_MAXLENGTH + CSN_STRSIZE;
- }
- if ( valueset_isempty(&a->a_deleted_values)) {
+ elen += entry2str_internal_size_valueset(a, a->a_type, &a->a_present_values,
+ entry2str_ctrl, attribute_state, VALUE_PRESENT);
+ if (entry2str_ctrl & SLAPI_DUMP_STATEINFO) {
+ elen += entry2str_internal_size_valueset(a, a->a_type, &a->a_deleted_values,
+ entry2str_ctrl, attribute_state, VALUE_DELETED);
+ if (valueset_isempty(&a->a_deleted_values) && valueset_isempty(&a->a_present_values)) {
/* this means the entry is deleted and has no more attributes,
* when writing the attr to disk we would loose the AD-csn.
- * Add an empty value to the set of deleted values. This will
- * never be seen by any client. It will never be moved to the
+ * Add an empty value to the set of deleted values. This will
+ * never be seen by any client. It will never be moved to the
* present values and is only used to preserve the AD-csn
* We need to add the size for that.
*/
elen += 1 + LDIF_CSNPREFIX_MAXLENGTH + CSN_STRSIZE;
- /* need also space for ";deletedattribute;deleted" */
- elen += DELETED_ATTR_STRSIZE + DELETED_VALUE_STRSIZE;
+ /* need also space for ";deletedattribute;deleted" */
+ elen += DELETED_ATTR_STRSIZE + DELETED_VALUE_STRSIZE;
+ /*
+ * If a_deleted_values is empty && if a_deletioncsn is NULL,
+ * a_deletioncsn is initialized via valueset_add_string.
+ * The size needs to be added.
+ */
+ /* ";adcsn-" + a->a_deletioncsn */
+ elen += 1 + LDIF_CSNPREFIX_MAXLENGTH + CSN_STRSIZE;
+ /*
+ * When both a_present_values & a_deleted_values are empty,
+ * the type size is not added.
+ */
+ elen += PL_strlen(a->a_type);
}
}
}
@@ -1766,10 +1777,8 @@ entry2str_internal_ext( Slapi_Entry *e, int *len, int entry2str_ctrl)
if (NULL != slapi_entry_get_rdn_const(e))
{
slapi_value_set_string(&rdnvalue, slapi_entry_get_rdn_const(e));
- elen += entry2str_internal_size_value("rdn", &rdnvalue,
- entry2str_ctrl,
- ATTRIBUTE_PRESENT,
- VALUE_PRESENT);
+ elen += entry2str_internal_size_value("rdn", &rdnvalue, entry2str_ctrl,
+ ATTRIBUTE_PRESENT, VALUE_PRESENT);
}
/* Count the space required for the present attributes */
--
2.7.4