This is not a winbindd bug, but a winbind-related one. In two places the following approach for temporary disabling winbind communication is used (pseudo code): char* env = getenv(...); winbind_off(); //{ putenv(WINBINDD_DONT_ENV=1) } . . . if (!(env && strequal(env, "1")) { winbind_on(); //{ putenv(INBINDD_DONT_ENV=0) } } This approach is not safe and won't work on some systems, because variable env is a *pointer* into environment space of a program, not a value or a copy. Thus, it is possible that setenv(3)/putenv(3) would simply overwrite a value at the same location, e.g.: env = getenv(ENV_VAR); printf("%s\n", env); //prints old val putenv(ENV_VAR=newval); printf("%s\n", env); //same pointer, but prints new val If this is the case, after winbind_off() env would point to string "1" and thus the quoted pseudo-code would fail to restore original state/value of that variable. On FreeBSD 6.1 I see the follwing behavior that only after the very first winbind_off() the code calls winbind_on() (this happens because env was originally NULL, i.e. env.var. was undet), after the second winbind_off() the code never calls winbind_on(), because of teh reasons explained above. I think that the easiest way in this situtation is to store *value* of environment variable somewhere, e.g.: char* env = getenv(WINBINDD_DONT_ENV); char winbind_disabled_flag = env ? env[0] : '0'; winbind_off(); ... if (winbind_disabled_flag == '0') winbind_on();
I submitted this bug report in a haste, so I would like add some description of the symptoms and system: FreeBSD 6.1-RELEASE-p2 i386 samba-3.0.23c,1 built from sources via the FreeBSD port Samba is configured as a work-group member (not a domain controller or anything) with security=domain and uses winbind to get user/group information from Windows domain controller (and also has some shares with restricted access via "valid users" directive of form valid users = @DOMAIN.group). I can verify that configuration of winbindd, nsswitch and samba is OK: all wbinfo commands work well system command 'id' works well for (non-local) groups and users in my windows workgroup On the other hand, any access attempt to the samba server in question resulted in NT_STATUS_NO_LOGON_SERVERS errors. My debugging showed that this happend because winbindd_request_response() was actually a NOP, so response was not touched at all and remain zeroed-out structure and thus both response.data.auth.nt_status and response.extra_data.data were both zeros/nulls. My first post explained why winbindd_request_response() was a NOP -- environment variable defined in WINBINDD_DONT_ENV macro was set to 1 and never reset back to 0 due to unsafe usage of getenv/putenv and peculiarities of the particular FreeBSD libc with respect to those functions.
(In reply to comment #1) Hi Andriy! Thanks a lot for your perfect analysis of the problem! I got few reports about non-working winbindd in FreeBSD, but couldn't imagine, that the problem was in inproper usage of getenv/putenv. I'll try to make patch for the port meanwhile and, lets hope, Samba team will come with more permanent solution for the problem. With best regards, Timur.
Created attachment 2151 [details] Work around getenv/putenv async
(In reply to comment #1) Hi, Andryi! Can you please try attached patch? Hope, it fixes the problem, you encountered. If so, I'll add it to net/samba3 ASAP. With best regards, Tmur
(In reply to comment #4) Have tried the patch, without success. Still getting NT_STATUS_NO_LOGON_SERVERS error wbinfo all commands work shares not available (NT_STATUS_NO_LOGON_SERVERS error) users available via pam_winbind unless I'm doing something wrong
(In reply to comment #4) Timur, I haven't tried your patch but I see that it won't work - you need to *save* state of environment *before* calling winbind_off(), because winbind_off() modifies the environment and thus winbind_env() will always return True when called after winbind_off().
(In reply to comment #6) > (In reply to comment #4) > > Timur, I haven't tried your patch but I see that it won't work - you need to > *save* state of environment *before* calling winbind_off(), because > winbind_off() modifies the environment and thus winbind_env() will always > return True when called after winbind_off(). Yeh, shouldn't do such things at 5a.m. in the morning :) Wait a bit, I'll send a new version :) Cheers, Timur.
Created attachment 2158 [details] Updated patch Hopefuly, this one is ok :)
(In reply to comment #6) > (In reply to comment #4) Hi, Andryi! Can you try a new patch? Hope, it's ok.
(In reply to comment #5) > (In reply to comment #4) Hi, Tim! Can you try new patch? I made a mistake in previous one, so it wasn't working. Thanks to Andryi for spotting it. With regards, Timur.
Created attachment 2160 [details] one more attempt Name of the variable and function clushed :> So, did some renaming.
(In reply to comment #10) > Hi, Tim! > > Can you try new patch? I made a mistake in previous one, so it wasn't working. > Thanks to Andryi for spotting it. I couldn't make it work, hopefully Andryi has better luck than I did
(In reply to comment #11) Yes, this patch fixes the problem for me! Thank you! Not sure about the name of the new function :-) but important thing is that it does the job.
Timur, The patch initially looks right. I'll try to finish reviewing it today.
This patch solved the problem. Thanks! FreeBSD 6.2-PRERELEASE i386
(In reply to comment #15) > This patch solved the problem. Thanks! FreeBSD 6.2-PRERELEASE i386 Thanks, Denis, for the feedback! I'd try to commit it into the port before the freeze. With best regards, Timur.
Hi, Jerry! (In reply to comment #14) > Timur, The patch initially looks right. I'll try to finish reviewing > it today. One more problem with this patch showed up today - it affects pam_winbind.so (and, possibly, nss_windbind.so) due usage of strequal() function for comparation with "1". It defined in util_str.c, which is not linked against pam_winbind.so. The solution is either to add all bunch of files, that implement strequal() and dependencies or simply use strcmp(), which comes with libc and safe to use in this case. Sorry for overlooking this problem :( With best regards, Timur.
or to simply use comparisons for char array elements, like (pseudo-code): char[0]=='1' && char[1]=='\0'
Looks good. Applying to SAMBA_3_0 and SAMBA_3_0_23