openldap-bugs

openldap-bugs@openldap.org

1 participants
20961 discussions

Re: (ITS#8719) slapd_crypt() become slow when many ldap cliant connections occur.
by yos-nishino＠ys.jp.nec.com 03 Sep '17

03 Sep '17

Dear Howard-san, >That comment only applies when using LDAP_MEMORY_DEBUG and is irrelevant. I saw memory.c again.=20 As you told, I confirmed the comment was available only when LDAP_MEMORY_DE= BUG was used(from line 27 to line 119). I am sorry to bother you, and I am grateful for your help. I am going to use "case2" patch, which I expect slapd_crypt() becomes much = faster. Best Reagrds, ************************************************ Yoshinori Nishino NEC Solution Innovators, Ltd. 1-18-7 Shinkiba, Koto-ku, Tokyo, 136-8627 Japan E-MAIL: yos-nishino(a)ys.jp.nec.com ************************************************

1 0

Re: (ITS#8719) slapd_crypt() become slow when many ldap cliant connections occur.
by hyc＠symas.com 03 Sep '17

03 Sep '17

Yoshinori Nishino wrote: > Dear Howard-san, > >> There is no issue with thread-safety for ber_strdup. What exact line of memory.c are you talking about? > > I think ber_strdup(), which uses ber_memalloc_x(), is non thread-safe because of the following comment at around line 69 of memory.c: > > /* Note sequence and ber_int_meminuse are counters, but are not > * thread safe. If you want to use these values for multithreaded applications, > * you must put mutexes around them, otherwise they will have incorrect values. > * When debugging, if you sort the debug output, the sequence number will > * put allocations/frees together. It is then a simple matter to write a script > * to find any allocations that don't have a buffer free function. > */ That comment only applies when using LDAP_MEMORY_DEBUG and is irrelevant. -- -- Howard Chu CTO, Symas Corp. http://www.symas.com Director, Highland Sun http://highlandsun.com/hyc/ Chief Architect, OpenLDAP http://www.openldap.org/project/

1 0

Re: (ITS#8719) slapd_crypt() become slow when many ldap cliant connections occur.
by yos-nishino＠ys.jp.nec.com 03 Sep '17

03 Sep '17

Dear Howard-san, >There is no issue with thread-safety for ber_strdup. What exact line of me= mory.c are you talking about? I think ber_strdup(), which uses ber_memalloc_x(), is non thread-safe becau= se of the following comment at around line 69 of memory.c: /* Note sequence and ber_int_meminuse are counters, but are not * thread safe. If you want to use these values for multithreaded applicat= ions, * you must put mutexes around them, otherwise they will have incorrect val= ues. * When debugging, if you sort the debug output, the sequence number will=20 * put allocations/frees together. It is then a simple matter to write a s= cript * to find any allocations that don't have a buffer free function. */ Best Regards, ************************************************ Yoshinori Nishino NEC Solution Innovators, Ltd. 1-18-7 Shinkiba, Koto-ku, Tokyo, 136-8627 Japan E-MAIL: yos-nishino(a)ys.jp.nec.com ************************************************

1 0

Re: (ITS#8719) slapd_crypt() become slow when many ldap cliant connections occur.
by hyc＠symas.com 03 Sep '17

03 Sep '17

Yoshinori Nishino wrote: > I decide to use passwd_mutex only for ber_strdup(), which seems to be non thread-safe according to ./libraries/liblber/memory.c. There is no issue with thread-safety for ber_strdup. What exact line of memory.c are you talking about? -- -- Howard Chu CTO, Symas Corp. http://www.symas.com Director, Highland Sun http://highlandsun.com/hyc/ Chief Architect, OpenLDAP http://www.openldap.org/project/

1 0

Re: (ITS#8719) slapd_crypt() become slow when many ldap cliant connections occur.
by yos-nishino＠ys.jp.nec.com 03 Sep '17

03 Sep '17

--_003_282E583D7ED52C4FA3C4CC199465868B02804CD4BPXM04GPgispnec_ Content-Type: text/plain; charset="iso-2022-jp" Content-Transfer-Encoding: quoted-printable Dear Howard-san, >I created two patches for passwd.c. I am sorry. The patches I sent before were wrong because free(data) is left= . I send the fixed patches again. Best Regards, ************************************************ Yoshinori Nishino NEC Solution Innovators, Ltd. 1-18-7 Shinkiba, Koto-ku, Tokyo, 136-8627 Japan E-MAIL: yos-nishino(a)ys.jp.nec.com ************************************************ --_003_282E583D7ED52C4FA3C4CC199465868B02804CD4BPXM04GPgispnec_ Content-Type: application/octet-stream; name="openldap-slapd_crypt_case2.patch" Content-Description: openldap-slapd_crypt_case2.patch Content-Disposition: attachment; filename="openldap-slapd_crypt_case2.patch"; size=1382; creation-date="Sun, 03 Sep 2017 01:05:35 GMT"; modification-date="Sun, 03 Sep 2017 02:11:42 GMT" Content-Transfer-Encoding: base64 bW9kaWZ5IHBhc3N3ZC5jIHNvIHRoYXQgc2xhcGRfY3J5cHQoKSB1c2VzIGNyeXB0X3IoKS4KCmRp ZmYgLS1naXQgYS9zZXJ2ZXJzL3NsYXBkL3Bhc3N3ZC5jIGIvc2VydmVycy9zbGFwZC9wYXNzd2Qu YwppbmRleCBkZmEzNzBjLi5lY2UwNWZiIDEwMDY0NAotLS0gYS9zZXJ2ZXJzL3NsYXBkL3Bhc3N3 ZC5jCisrKyBiL3NlcnZlcnMvc2xhcGQvcGFzc3dkLmMKQEAgLTIzLDggKzIzLDExIEBACiAjaW5j bHVkZSA8YWMvdW5pc3RkLmg+CiAKICNpZmRlZiBTTEFQRF9DUllQVAorI2lmZGVmIEhBVkVfQ1JZ UFRfUgorI2RlZmluZSBfX1VTRV9HTlUKKyNlbmRpZiAvKiBIQVZFX0NSWVBUX1IgKi8KICNpbmNs dWRlIDxhYy9jcnlwdC5oPgotI2VuZGlmCisjZW5kaWYgLyogU0xBUERfQ1JZUFQgKi8KIAogI2lu Y2x1ZGUgInNsYXAuaCIKIApAQCAtNTkwLDYgKzU5MywzMCBAQCBzbGFwX3Bhc3N3ZF9oYXNoKAog c3RhdGljIGxkYXBfcHZ0X3RocmVhZF9tdXRleF90IHBhc3N3ZF9tdXRleDsKIHN0YXRpYyBsdXRp bF9jcnlwdGZ1bmMgc2xhcGRfY3J5cHQ7CiAKKyNpZmRlZiBIQVZFX0NSWVBUX1IKK3N0YXRpYyBp bnQgc2xhcGRfY3J5cHQoIGNvbnN0IGNoYXIgKmtleSwgY29uc3QgY2hhciAqc2FsdCwgY2hhciAq Kmhhc2ggKQoreworCWNoYXIgKmNyOworCWludCByYzsKKwlzdHJ1Y3QgY3J5cHRfZGF0YSBkYXRh OworICAgIAorCWRhdGEuaW5pdGlhbGl6ZWQgPSAwOworCWNyID0gY3J5cHRfcigga2V5LCBzYWx0 LCAmZGF0YSApOworCWlmICggY3IgPT0gTlVMTCB8fCBjclswXSA9PSAnXDAnICkgeworCQkvKiBz YWx0IG11c3QgaGF2ZSBiZWVuIGludmFsaWQgKi8KKwkJcmMgPSBMVVRJTF9QQVNTV0RfRVJSOwor CX0gZWxzZSB7CisJCWlmICggaGFzaCApIHsKKwkJCSpoYXNoID0gYmVyX3N0cmR1cCggY3IgKTsK KwkJCXJjID0gTFVUSUxfUEFTU1dEX09LOworCQl9IGVsc2UgeworCQkJcmMgPSBzdHJjbXAoIHNh bHQsIGNyICkgPyBMVVRJTF9QQVNTV0RfRVJSIDogTFVUSUxfUEFTU1dEX09LOworCQl9CisJfQor CisgICAgcmV0dXJuIHJjOworfQorI2Vsc2UKIHN0YXRpYyBpbnQgc2xhcGRfY3J5cHQoIGNvbnN0 IGNoYXIgKmtleSwgY29uc3QgY2hhciAqc2FsdCwgY2hhciAqKmhhc2ggKQogewogCWNoYXIgKmNy OwpAQCAtNjE0LDYgKzY0MSw4IEBAIHN0YXRpYyBpbnQgc2xhcGRfY3J5cHQoIGNvbnN0IGNoYXIg KmtleSwgY29uc3QgY2hhciAqc2FsdCwgY2hhciAqKmhhc2ggKQogCWxkYXBfcHZ0X3RocmVhZF9t dXRleF91bmxvY2soICZwYXNzd2RfbXV0ZXggKTsKIAlyZXR1cm4gcmM7CiB9CisjZW5kaWYgLyog SEFWRV9DUllQVF9SICovCisKICNlbmRpZiAvKiBTTEFQRF9DUllQVCAqLwogCiB2b2lkIHNsYXBf cGFzc3dkX2luaXQoKQo= --_003_282E583D7ED52C4FA3C4CC199465868B02804CD4BPXM04GPgispnec_ Content-Type: application/octet-stream; name="openldap-slapd_crypt_case1.patch" Content-Description: openldap-slapd_crypt_case1.patch Content-Disposition: attachment; filename="openldap-slapd_crypt_case1.patch"; size=1482; creation-date="Sun, 03 Sep 2017 01:05:32 GMT"; modification-date="Sun, 03 Sep 2017 02:10:39 GMT" Content-Transfer-Encoding: base64 bW9kaWZ5IHBhc3N3ZC5jIHNvIHRoYXQgc2xhcGRfY3J5cHQoKSB1c2VzIGNyeXB0X3IoKS4KCmRp ZmYgLS1naXQgYS9zZXJ2ZXJzL3NsYXBkL3Bhc3N3ZC5jIGIvc2VydmVycy9zbGFwZC9wYXNzd2Qu YwppbmRleCBkZmEzNzBjLi40OTcwNWY4IDEwMDY0NAotLS0gYS9zZXJ2ZXJzL3NsYXBkL3Bhc3N3 ZC5jCisrKyBiL3NlcnZlcnMvc2xhcGQvcGFzc3dkLmMKQEAgLTIzLDggKzIzLDExIEBACiAjaW5j bHVkZSA8YWMvdW5pc3RkLmg+CiAKICNpZmRlZiBTTEFQRF9DUllQVAorI2lmZGVmIEhBVkVfQ1JZ UFRfUgorI2RlZmluZSBfX1VTRV9HTlUKKyNlbmRpZiAvKiBIQVZFX0NSWVBUX1IgKi8KICNpbmNs dWRlIDxhYy9jcnlwdC5oPgotI2VuZGlmCisjZW5kaWYgLyogU0xBUERfQ1JZUFQgKi8KIAogI2lu Y2x1ZGUgInNsYXAuaCIKIApAQCAtNTkwLDYgKzU5MywzMiBAQCBzbGFwX3Bhc3N3ZF9oYXNoKAog c3RhdGljIGxkYXBfcHZ0X3RocmVhZF9tdXRleF90IHBhc3N3ZF9tdXRleDsKIHN0YXRpYyBsdXRp bF9jcnlwdGZ1bmMgc2xhcGRfY3J5cHQ7CiAKKyNpZmRlZiBIQVZFX0NSWVBUX1IKK3N0YXRpYyBp bnQgc2xhcGRfY3J5cHQoIGNvbnN0IGNoYXIgKmtleSwgY29uc3QgY2hhciAqc2FsdCwgY2hhciAq Kmhhc2ggKQoreworCWNoYXIgKmNyOworCWludCByYzsKKwlzdHJ1Y3QgY3J5cHRfZGF0YSBkYXRh OworICAgIAorCWRhdGEuaW5pdGlhbGl6ZWQgPSAwOworCWNyID0gY3J5cHRfcigga2V5LCBzYWx0 LCAmZGF0YSApOworCWlmICggY3IgPT0gTlVMTCB8fCBjclswXSA9PSAnXDAnICkgeworCQkvKiBz YWx0IG11c3QgaGF2ZSBiZWVuIGludmFsaWQgKi8KKwkJcmMgPSBMVVRJTF9QQVNTV0RfRVJSOwor CX0gZWxzZSB7CisJCWlmICggaGFzaCApIHsKKwkJCWxkYXBfcHZ0X3RocmVhZF9tdXRleF9sb2Nr KCAmcGFzc3dkX211dGV4ICk7CisJCQkqaGFzaCA9IGJlcl9zdHJkdXAoIGNyICk7CisJCQlsZGFw X3B2dF90aHJlYWRfbXV0ZXhfdW5sb2NrKCAmcGFzc3dkX211dGV4ICk7CisJCQlyYyA9IExVVElM X1BBU1NXRF9PSzsKKwkJfSBlbHNlIHsKKwkJCXJjID0gc3RyY21wKCBzYWx0LCBjciApID8gTFVU SUxfUEFTU1dEX0VSUiA6IExVVElMX1BBU1NXRF9PSzsKKwkJfQorCX0KKworICAgIHJldHVybiBy YzsKK30KKyNlbHNlCiBzdGF0aWMgaW50IHNsYXBkX2NyeXB0KCBjb25zdCBjaGFyICprZXksIGNv bnN0IGNoYXIgKnNhbHQsIGNoYXIgKipoYXNoICkKIHsKIAljaGFyICpjcjsKQEAgLTYxNCw2ICs2 NDMsOCBAQCBzdGF0aWMgaW50IHNsYXBkX2NyeXB0KCBjb25zdCBjaGFyICprZXksIGNvbnN0IGNo YXIgKnNhbHQsIGNoYXIgKipoYXNoICkKIAlsZGFwX3B2dF90aHJlYWRfbXV0ZXhfdW5sb2NrKCAm cGFzc3dkX211dGV4ICk7CiAJcmV0dXJuIHJjOwogfQorI2VuZGlmIC8qIEhBVkVfQ1JZUFRfUiAq LworCiAjZW5kaWYgLyogU0xBUERfQ1JZUFQgKi8KIAogdm9pZCBzbGFwX3Bhc3N3ZF9pbml0KCkK --_003_282E583D7ED52C4FA3C4CC199465868B02804CD4BPXM04GPgispnec_--

1 0

Re: (ITS#8719) slapd_crypt() become slow when many ldap cliant connections occur.
by yos-nishino＠ys.jp.nec.com 03 Sep '17

03 Sep '17

--_003_282E583D7ED52C4FA3C4CC199465868B02801BECBPXM04GPgispnec_ Content-Type: text/plain; charset="iso-2022-jp" Content-Transfer-Encoding: quoted-printable Dear Howard-san, I appreciate your many valuable advices. >Never use calloc/malloc/free directly in slapd code. Always use ch_calloc/= ch_malloc/ch_free. > >In this particular case, there's no reason to allocate at all. Just define= "struct crypt_data data" as a local variable. You are right. There is no need to allocate memory for "data" because using= a local variable is also thread-safe in this case. >Also there's no reason to lock the passwd_mutex at all. That would complet= ely defeat the purpose of using crypt_r() in the first place. I decide to use passwd_mutex only for ber_strdup(), which seems to be non = thread-safe according to ./libraries/liblber/memory.c. I created two patches for passwd.c. Both patches uses "data" as a local variable. =20 "case1" uses passwd_mutex for ber_strdup(), "case2" dose not so. For the time being , I am going to use "case1" due to the aforementioned th= read-safe concern about ber_strdup(). Best Regards, ************************************************ Yoshinori Nishino NEC Solution Innovators, Ltd. 1-18-7 Shinkiba, Koto-ku, Tokyo, 136-8627 Japan E-MAIL: yos-nishino(a)ys.jp.nec.com ************************************************ --_003_282E583D7ED52C4FA3C4CC199465868B02801BECBPXM04GPgispnec_ Content-Type: application/octet-stream; name="openldap-slapd_crypt_case1.patch" Content-Description: openldap-slapd_crypt_case1.patch Content-Disposition: attachment; filename="openldap-slapd_crypt_case1.patch"; size=1345; creation-date="Sun, 03 Sep 2017 01:05:32 GMT"; modification-date="Sun, 03 Sep 2017 01:08:20 GMT" Content-Transfer-Encoding: base64 bW9kaWZ5IHBhc3N3ZC5jIHNvIHRoYXQgc2xhcGRfY3J5cHQoKSB1c2VzIGNyeXB0X3IoKS4KCmRp ZmYgLS1naXQgYS9wYXNzd2QuYy4gYi9wYXNzd2QuYwppbmRleCBkZmEzNzBjLi5hYzA3ZGQyIDEw MDY0NAotLS0gYS9wYXNzd2QuYworKysgYi9wYXNzd2QuYwpAQCAtMjMsOCArMjMsMTEgQEAKICNp bmNsdWRlIDxhYy91bmlzdGQuaD4KIAogI2lmZGVmIFNMQVBEX0NSWVBUCisjaWZkZWYgSEFWRV9D UllQVF9SCisjZGVmaW5lIF9fVVNFX0dOVQorI2VuZGlmIC8qIEhBVkVfQ1JZUFRfUiAqLwogI2lu Y2x1ZGUgPGFjL2NyeXB0Lmg+Ci0jZW5kaWYKKyNlbmRpZiAvKiBTTEFQRF9DUllQVCAqLwogCiAj aW5jbHVkZSAic2xhcC5oIgogCkBAIC01OTAsNiArNTkzLDMxIEBAIHNsYXBfcGFzc3dkX2hhc2go CiBzdGF0aWMgbGRhcF9wdnRfdGhyZWFkX211dGV4X3QgcGFzc3dkX211dGV4Owogc3RhdGljIGx1 dGlsX2NyeXB0ZnVuYyBzbGFwZF9jcnlwdDsKIAorI2lmZGVmIEhBVkVfQ1JZUFRfUgorc3RhdGlj IGludCBzbGFwZF9jcnlwdCggY29uc3QgY2hhciAqa2V5LCBjb25zdCBjaGFyICpzYWx0LCBjaGFy ICoqaGFzaCApCit7CisJY2hhciAqY3I7CisJaW50IHJjOworCXN0cnVjdCBjcnlwdF9kYXRhIGRh dGE7CisgICAgCisJZGF0YS5pbml0aWFsaXplZCA9IDA7CisJY3IgPSBjcnlwdF9yKCBrZXksIHNh bHQsICZkYXRhICk7CisJaWYgKCBjciA9PSBOVUxMIHx8IGNyWzBdID09ICdcMCcgKSB7CisJCS8q IHNhbHQgbXVzdCBoYXZlIGJlZW4gaW52YWxpZCAqLworCQlyYyA9IExVVElMX1BBU1NXRF9FUlI7 CisJfSBlbHNlIHsKKwkJaWYgKCBoYXNoICkgeworCQkJKmhhc2ggPSBiZXJfc3RyZHVwKCBjciAp OworCQkJcmMgPSBMVVRJTF9QQVNTV0RfT0s7CisJCX0gZWxzZSB7CisJCQlyYyA9IHN0cmNtcCgg c2FsdCwgY3IgKSA/IExVVElMX1BBU1NXRF9FUlIgOiBMVVRJTF9QQVNTV0RfT0s7CisJCX0KKwl9 CisgCisgICAgZnJlZShkYXRhKTsKKyAgICByZXR1cm4gcmM7Cit9CisjZWxzZQogc3RhdGljIGlu dCBzbGFwZF9jcnlwdCggY29uc3QgY2hhciAqa2V5LCBjb25zdCBjaGFyICpzYWx0LCBjaGFyICoq aGFzaCApCiB7CiAJY2hhciAqY3I7CkBAIC02MTQsNiArNjQyLDggQEAgc3RhdGljIGludCBzbGFw ZF9jcnlwdCggY29uc3QgY2hhciAqa2V5LCBjb25zdCBjaGFyICpzYWx0LCBjaGFyICoqaGFzaCAp CiAJbGRhcF9wdnRfdGhyZWFkX211dGV4X3VubG9jayggJnBhc3N3ZF9tdXRleCApOwogCXJldHVy biByYzsKIH0KKyNlbmRpZiAvKiBIQVZFX0NSWVBUX1IgKi8KKwogI2VuZGlmIC8qIFNMQVBEX0NS WVBUICovCiAKIHZvaWQgc2xhcF9wYXNzd2RfaW5pdCgpCg== --_003_282E583D7ED52C4FA3C4CC199465868B02801BECBPXM04GPgispnec_ Content-Type: application/octet-stream; name="openldap-slapd_crypt_case2.patch" Content-Description: openldap-slapd_crypt_case2.patch Content-Disposition: attachment; filename="openldap-slapd_crypt_case2.patch"; size=1444; creation-date="Sun, 03 Sep 2017 01:05:35 GMT"; modification-date="Sun, 03 Sep 2017 01:08:20 GMT" Content-Transfer-Encoding: base64 bW9kaWZ5IHBhc3N3ZC5jIHNvIHRoYXQgc2xhcGRfY3J5cHQoKSB1c2VzIGNyeXB0X3IoKS4KCmRp ZmYgLS1naXQgYS9wYXNzd2QuYyBiL3Bhc3N3ZC5jCmluZGV4IGRmYTM3MGMuLjM1ZDg3ZTUgMTAw NjQ0Ci0tLSBhL3Bhc3N3ZC5jCisrKyBiL3Bhc3N3ZC5jCkBAIC0yMyw4ICsyMywxMSBAQAogI2lu Y2x1ZGUgPGFjL3VuaXN0ZC5oPgogCiAjaWZkZWYgU0xBUERfQ1JZUFQKKyNpZmRlZiBIQVZFX0NS WVBUX1IKKyNkZWZpbmUgX19VU0VfR05VCisjZW5kaWYgLyogSEFWRV9DUllQVF9SICovCiAjaW5j bHVkZSA8YWMvY3J5cHQuaD4KLSNlbmRpZgorI2VuZGlmIC8qIFNMQVBEX0NSWVBUICovCiAKICNp bmNsdWRlICJzbGFwLmgiCiAKQEAgLTU5MCw2ICs1OTMsMzMgQEAgc2xhcF9wYXNzd2RfaGFzaCgK IHN0YXRpYyBsZGFwX3B2dF90aHJlYWRfbXV0ZXhfdCBwYXNzd2RfbXV0ZXg7CiBzdGF0aWMgbHV0 aWxfY3J5cHRmdW5jIHNsYXBkX2NyeXB0OwogCisjaWZkZWYgSEFWRV9DUllQVF9SCitzdGF0aWMg aW50IHNsYXBkX2NyeXB0KCBjb25zdCBjaGFyICprZXksIGNvbnN0IGNoYXIgKnNhbHQsIGNoYXIg KipoYXNoICkKK3sKKwljaGFyICpjcjsKKwlpbnQgcmM7CisJc3RydWN0IGNyeXB0X2RhdGEgZGF0 YTsKKyAgICAKKwlkYXRhLmluaXRpYWxpemVkID0gMDsKKwljciA9IGNyeXB0X3IoIGtleSwgc2Fs dCwgJmRhdGEgKTsKKwlpZiAoIGNyID09IE5VTEwgfHwgY3JbMF0gPT0gJ1wwJyApIHsKKwkJLyog c2FsdCBtdXN0IGhhdmUgYmVlbiBpbnZhbGlkICovCisJCXJjID0gTFVUSUxfUEFTU1dEX0VSUjsK Kwl9IGVsc2UgeworCQlpZiAoIGhhc2ggKSB7CisJCQlsZGFwX3B2dF90aHJlYWRfbXV0ZXhfbG9j ayggJnBhc3N3ZF9tdXRleCApOworCQkJKmhhc2ggPSBiZXJfc3RyZHVwKCBjciApOworCQkJbGRh cF9wdnRfdGhyZWFkX211dGV4X3VubG9jayggJnBhc3N3ZF9tdXRleCApOworCQkJcmMgPSBMVVRJ TF9QQVNTV0RfT0s7CisJCX0gZWxzZSB7CisJCQlyYyA9IHN0cmNtcCggc2FsdCwgY3IgKSA/IExV VElMX1BBU1NXRF9FUlIgOiBMVVRJTF9QQVNTV0RfT0s7CisJCX0KKwl9CisgCisgICAgZnJlZShk YXRhKTsKKyAgICByZXR1cm4gcmM7Cit9CisjZWxzZQogc3RhdGljIGludCBzbGFwZF9jcnlwdCgg Y29uc3QgY2hhciAqa2V5LCBjb25zdCBjaGFyICpzYWx0LCBjaGFyICoqaGFzaCApCiB7CiAJY2hh ciAqY3I7CkBAIC02MTQsNiArNjQ0LDggQEAgc3RhdGljIGludCBzbGFwZF9jcnlwdCggY29uc3Qg Y2hhciAqa2V5LCBjb25zdCBjaGFyICpzYWx0LCBjaGFyICoqaGFzaCApCiAJbGRhcF9wdnRfdGhy ZWFkX211dGV4X3VubG9jayggJnBhc3N3ZF9tdXRleCApOwogCXJldHVybiByYzsKIH0KKyNlbmRp ZiAvKiBIQVZFX0NSWVBUX1IgKi8KKwogI2VuZGlmIC8qIFNMQVBEX0NSWVBUICovCiAKIHZvaWQg c2xhcF9wYXNzd2RfaW5pdCgpCg== --_003_282E583D7ED52C4FA3C4CC199465868B02801BECBPXM04GPgispnec_--

1 0

Re: (ITS#8350) lmdb SIGBUS error on full partition and possible double free issue
by vladimir.cunat＠nic.cz 01 Sep '17

01 Sep '17

On 09/01/2017 02:38 PM, Howard Chu wrote: > vladimir.cunat(a)nic.cz wrote: >> The documentation of txn_commit did confuse me (as well). >> https://github.com/LMDB/lmdb/blob/LMDB_0.9.21/libraries/liblmdb/lmdb.h#L994 >> >> Would you consider adding a line stressing that the txn gets released >> even in case any error is returned? (or something similar) I'm >> personally used to failing operations not "having externally observable >> effects" unless explicitly stated. > > The very first words on the line you highlight are "The transaction > handle is freed." I don't know how to state that any more explicitly. Well, okay. (I knew the line stated it and still it somehow didn't make me sure, but it's perhaps just me.)

1 0

Re: (ITS#8350) lmdb SIGBUS error on full partition and possible double free issue
by hyc＠symas.com 01 Sep '17

01 Sep '17

vladimir.cunat(a)nic.cz wrote: > Hello! > >> The docs say quite clearly, after txn_commit the txn must not be used > again. > > The documentation of txn_commit did confuse me (as well). > https://github.com/LMDB/lmdb/blob/LMDB_0.9.21/libraries/liblmdb/lmdb.h#L994 > Would you consider adding a line stressing that the txn gets released > even in case any error is returned? (or something similar) I'm > personally used to failing operations not "having externally observable > effects" unless explicitly stated. The very first words on the line you highlight are "The transaction handle is freed." I don't know how to state that any more explicitly. -- -- Howard Chu CTO, Symas Corp. http://www.symas.com Director, Highland Sun http://highlandsun.com/hyc/ Chief Architect, OpenLDAP http://www.openldap.org/project/

1 0

Re: (ITS#8719) slapd_crypt() become slow when many ldap cliant connections occur.
by hyc＠symas.com 01 Sep '17

01 Sep '17

Howard Chu wrote: > Yoshinori Nishino wrote: >> Dear Howard-san, >> >> >> Thanks to your advice, I could create the attached tentative patches. >=20 > Thanks, this looks much better. >> >> They modifies servers/slapd/passwd.c and configure.in >> so that running "./configure" can check whether or not crypt_r() exist= s >> and use the function in slapd_crypt() if possible. >> >> I am going to check whether or not "calloc()/free()" causes >> unacceptable=E3=80=80memory fragmentation. >=20 > Never use calloc/malloc/free directly in slapd code. Always use=20 > ch_calloc/ch_malloc/ch_free. >=20 > In this particular case, there's no reason to allocate at all. Just def= ine=20 > "struct crypt_data data" as a local variable. Also there's no reason to lock the passwd_mutex at all. That would comple= tely=20 defeat the purpose of using crypt_r() in the first place. >> >> >> Best Regards, >> ------------------------------------------------ >> On 2017/09/01 20:23, Yoshinori Nishino wrote: >>> >>> >>> On 2017/08/30 20:46, Yoshinori Nishino wrote: >>>> Dear Howard-san, >>>> >>>> >>>> Thank you so much for your prompt advice. >>>> >>>> As you told, I need to check whether or not crypt_r() exists. >>>> I am going to consider including the check. >>>> >>>> Would it be possible to let me know whether there are any other >>>> concerns about the fix if we can use crypt_r()? >>>> >>>> The implementation that "both calloc() and free() occur every time >>>> slapd_crypt() is called" does not seem to look good. >>>> The callers of slapd_crypt() might be able to get the memory for >>>> "struct crypt_data" before they call it. >>>> However, I think it causes the changes of other source codes. >>>> So, I think the aforementioned implementation is a workaround for th= e >>>> slowdown if the risk of memory fragmentation can be acceptable. >>>> >>>> On the other hands, I think we do not need to care about strcmp() >>>> because it is thread-safe. >>>> >>>> >>>> Best Regards, >>>> ---------------------------------------- >>>> On 2017/08/30 19:26, Howard Chu wrote: >>>>> yos-nishino(a)ys.jp.nec.com wrote: >>>>>> Full_Name: Yoshinori Nishino >>>>>> Version: 2.4.45 >>>>>> OS: CentOS 7 >>>>>> URL: ftp://ftp.openldap.org/incoming/ >>>>>> Submission from: (NULL) (210.143.35.20) >>>>>> >>>>>> >>>>>> Dear sir, >>>>>> >>>>>> The function slapd_crypt() in servers/slapd/passwd.c seems to beco= me >>>>>> slow when >>>>>> many ldap client connections occur. >>>>>> It seems it is because the function uses crypt()(non thread-safe >>>>>> function) and >>>>>> pthread_mutex_lock(), which results in the slowdown. >>>>>> #Besides, we need to use {CRYPT} hash as users' password hash. >>>>>> >>>>>> So, I modified servers/slapd/passwd.c like the following. >>>>>> As a result, slapd_crypt() becomes much faster under the same >>>>>> condition. >>>>>> Would you let me know whether or not the fix is appropriate for sl= apd? >>>>> >>>>> No it is not an appropriate fix. >>>>> >>>>> You should add an autoconf test to check for the existence of the >>>>> crypt_r function, and use an #ifdef here based on the result of tha= t >>>>> test, since crypt_r is a non-standard function. >>>>>> >> >> >=20 >=20 --=20 -- Howard Chu CTO, Symas Corp. http://www.symas.com Director, Highland Sun http://highlandsun.com/hyc/ Chief Architect, OpenLDAP http://www.openldap.org/project/

1 0

Re: (ITS#8719) slapd_crypt() become slow when many ldap cliant connections occur.
by hyc＠symas.com 01 Sep '17

01 Sep '17

Yoshinori Nishino wrote: > Dear Howard-san, >=20 >=20 > Thanks to your advice, I could create the attached tentative patches. Thanks, this looks much better. >=20 > They modifies servers/slapd/passwd.c and configure.in > so that running "./configure" can check whether or not crypt_r() exists > and use the function in slapd_crypt() if possible. >=20 > I am going to check whether or not "calloc()/free()" causes > unacceptable=E3=80=80memory fragmentation. Never use calloc/malloc/free directly in slapd code. Always use=20 ch_calloc/ch_malloc/ch_free. In this particular case, there's no reason to allocate at all. Just defin= e=20 "struct crypt_data data" as a local variable. >=20 >=20 > Best Regards, > ------------------------------------------------ > On 2017/09/01 20:23, Yoshinori Nishino wrote: >> >> >> On 2017/08/30 20:46, Yoshinori Nishino wrote: >>> Dear Howard-san, >>> >>> >>> Thank you so much for your prompt advice. >>> >>> As you told, I need to check whether or not crypt_r() exists. >>> I am going to consider including the check. >>> >>> Would it be possible to let me know whether there are any other >>> concerns about the fix if we can use crypt_r()? >>> >>> The implementation that "both calloc() and free() occur every time >>> slapd_crypt() is called" does not seem to look good. >>> The callers of slapd_crypt() might be able to get the memory for >>> "struct crypt_data" before they call it. >>> However, I think it causes the changes of other source codes. >>> So, I think the aforementioned implementation is a workaround for the >>> slowdown if the risk of memory fragmentation can be acceptable. >>> >>> On the other hands, I think we do not need to care about strcmp() >>> because it is thread-safe. >>> >>> >>> Best Regards, >>> ---------------------------------------- >>> On 2017/08/30 19:26, Howard Chu wrote: >>>> yos-nishino(a)ys.jp.nec.com wrote: >>>>> Full_Name: Yoshinori Nishino >>>>> Version: 2.4.45 >>>>> OS: CentOS 7 >>>>> URL: ftp://ftp.openldap.org/incoming/ >>>>> Submission from: (NULL) (210.143.35.20) >>>>> >>>>> >>>>> Dear sir, >>>>> >>>>> The function slapd_crypt() in servers/slapd/passwd.c seems to becom= e >>>>> slow when >>>>> many ldap client connections occur. >>>>> It seems it is because the function uses crypt()(non thread-safe >>>>> function) and >>>>> pthread_mutex_lock(), which results in the slowdown. >>>>> #Besides, we need to use {CRYPT} hash as users' password hash. >>>>> >>>>> So, I modified servers/slapd/passwd.c like the following. >>>>> As a result, slapd_crypt() becomes much faster under the same >>>>> condition. >>>>> Would you let me know whether or not the fix is appropriate for sla= pd? >>>> >>>> No it is not an appropriate fix. >>>> >>>> You should add an autoconf test to check for the existence of the >>>> crypt_r function, and use an #ifdef here based on the result of that >>>> test, since crypt_r is a non-standard function. >>>>> >=20 >=20 --=20 -- Howard Chu CTO, Symas Corp. http://www.symas.com Director, Highland Sun http://highlandsun.com/hyc/ Chief Architect, OpenLDAP http://www.openldap.org/project/

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

openldap-bugs