Hello. FreeBSD 10.1 Samba as AD DC with BIND with DLZ. Timur updated port to samba 4.2.3 at FreeBSD. After updating samba and other needed components, samba and bind not started with crashes. After some truss and gdb I found, that crash happend when started to work with ldb, tdb files: gdb output is: #0 0x00000008048489d8 in rep_strtoll (str=Cannot access memory at address 0x7fffffbfeff8 ) at ../lib/replace/replace.c:522 522 ../lib/replace/replace.c: No such file or directory. in ../lib/replace/replace.c [New Thread 802006400 (LWP 100093/python2.7)] (gdb) where #0 0x00000008048489d8 in rep_strtoll (str=Cannot access memory at address 0x7fffffbfeff8 ) at ../lib/replace/replace.c:522 #1 0x00000008048489f3 in rep_strtoll (str=0x7fffffffa1e4 "16", endptr=0x7fffffffa1d8, base=10) at ../lib/replace/replace.c:523 So, looking through lib/replace/replace.c in last and previous versions of ldb and tdb I found, that function rep_strtoll was changed from #ifdef HAVE_BSD_STRTOLL #ifdef HAVE_STRTOQ long long int rep_strtoll(const char *str, char **endptr, int base) { long long int nb = strtoq(str, endptr, base); /* In linux EINVAL is only returned if base is not ok */ if (errno == EINVAL) { if (base == 0 || (base >1 && base <37)) { /* Base was ok so it's because we were not * able to make the convertion. * Let's reset errno. */ errno = 0; } } return nb; } #else #error "You need the strtoq function" #endif /* HAVE_STRTOQ */ #endif /* HAVE_BSD_STRTOLL */ to #ifdef HAVE_BSD_STRTOLL long long int rep_strtoll(const char *str, char **endptr, int base) { long long int nb = strtoll(str, endptr, base); /* With glibc EINVAL is only returned if base is not ok */ if (errno == EINVAL) { if (base == 0 || (base >1 && base <37)) { /* Base was ok so it's because we were not * able to make the convertion. * Let's reset errno. */ errno = 0; } } return nb; } #endif /* HAVE_BSD_STRTOLL */ And new code is crashing at FreeBSD. Also I checked replace.c in samba 4.2.3 and ntdb, there old code. Checked ldbedit, classicupgrade procedures, they also crashed. This needs to be fixed ASAP, because current state of samba and needed components in FreeBSD is fully unusable as AD DC.
Created attachment 11350 [details] Patch This patch is for master, but it should similarly apply to 4.2
(In reply to Volker Lendecke from comment #1) lgtm & pushed to master.
Edited replace.c according to new code and patch. Final code is: #ifdef HAVE_BSD_STRTOLL #undef strtoll long long int rep_strtoll(const char *str, char **endptr, int base) { long long int nb = strtoll(str, endptr, base); /* With glibc EINVAL is only returned if base is not ok */ if (errno == EINVAL) { if (base == 0 || (base >1 && base <37)) { /* Base was ok so it's because we were not * able to make the convertion. * Let's reset errno. */ errno = 0; } } return nb; } #endif /* HAVE_BSD_STRTOLL */ replace.c was edited in next FreeBSD ports: net/samba42 databases/ldb databases/ntdb databases/tdb devel/talloc devel/tevent Seems these all ports which include lib/replace but files in ports have differences (defferent sizes). Timur needs to review ports with lib/replace and update to last version. After recompiling, samba-tool dbcheck passed, samba and bind started without problem with data from 4.2.2, ldbedit normally open files. Clean provision and classicupgrade I didn't test yet. Also seems there is mistype in patch description. Function name is rep_strtoll, not rep_strtoull. Thanks!
rep_strtoull() needs the same fix.
Hello. So, rep_srttoull needs to be changed from that: #ifdef HAVE_BSD_STRTOLL #ifdef HAVE_STRTOUQ unsigned long long int rep_strtoull(const char *str, char **endptr, int base) { unsigned long long int nb = strtouq(str, endptr, base); /* In linux EINVAL is only returned if base is not ok */ if (errno == EINVAL) { if (base == 0 || (base >1 && base <37)) { /* Base was ok so it's because we were not * able to make the convertion. * Let's reset errno. */ errno = 0; } } return nb; } #else #error "You need the strtouq function" #endif /* HAVE_STRTOUQ */ #endif /* HAVE_BSD_STRTOLL */ #endif /* HAVE_STRTOULL */ to this #ifdef HAVE_BSD_STRTOLL #ifdef HAVE_STRTOUQ #undef strtoull unsigned long long int rep_strtoull(const char *str, char **endptr, int base) { unsigned long long int nb = strtouq(str, endptr, base); /* In linux EINVAL is only returned if base is not ok */ if (errno == EINVAL) { if (base == 0 || (base >1 && base <37)) { /* Base was ok so it's because we were not * able to make the convertion. * Let's reset errno. */ errno = 0; } } return nb; } #else #error "You need the strtouq function" #endif /* HAVE_STRTOUQ */ #endif /* HAVE_BSD_STRTOLL */ #endif /* HAVE_STRTOULL */ Am I right? Thanks!
(In reply to Dron from comment #5) Looks correct
(In reply to Stefan Metzmacher from comment #6) Really? The endless recursion in rep_strtoll came from the #define strtoll rep_strtoll which thus called itself. In rep_strtoull we call strtouq, thus we don't have that recursion. Whether calling strtouq is the right thing to do here is a different question, but I don't see how the same bug would happen for rep_strtoull.
(In reply to Volker Lendecke from comment #7) Ok, I just realized that the following patches are not yet in master: https://lists.samba.org/archive/samba-technical/2015-July/108587.html https://lists.samba.org/archive/samba-technical/2015-July/108588.html https://lists.samba.org/archive/samba-technical/2015-July/108589.html https://lists.samba.org/archive/samba-technical/2015-July/108587.html would introduce the same problem for rep_strtoull(). Can you push your fix for rep_strtoll() together with a fixed version of https://lists.samba.org/archive/samba-technical/2015-July/108587.html and the two additional changes to master? All reviewed-by: me Thanks!
(In reply to Stefan Metzmacher from comment #8) Ok, now I realized that you fix is already in master... But can you look at the rest? Thanks!
(In reply to Stefan Metzmacher from comment #9) No, sorry, I have already deleted the patches and I don't have the energy right now to manually re-craft them from the list archive.
Closing this bug as fixed. The fact that some patches might be missing is definitely a different issue than the infinite recursion in rep_strtoll
Created attachment 11359 [details] Patch for v4-3-test
We need this for 4.3
Pushed to autobuild-v4-3-test
Hello. What about 4.2, will be patch for this branch? Will it be included in 4.2.4?
(In reply to Dron from comment #15) Once we've sorted everthing out for master and v4-3-test, we can backport all fixes to v4-2-test.
Ok, Thanks!
Pushed to v4-3-test. Keep open for v4-2-test
*** Bug 11497 has been marked as a duplicate of this bug. ***
Samba 4.2 is now in security fixes only mode. Closing again. Fixed in master with 62d08ea715d1664a7600250abbd1dc83f3a33a4c and 4.3 with 02d549ab4da7b9689f22dd719bdd8b9d66f61d60