From f71f0f4f8d07b09fab205bf4d1cbd9dbc59074fc Mon Sep 17 00:00:00 2001 From: Noriko Hosoi 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