[email protected] wrote: > On Apr 13, 2013, at 12:24 AM, Hallvard Breien Furuseth wrote: > >> Looking again at the st_mode compare: I'm pretty sure the old >> code worked somewhere, at some time. Maybe somewhere it still >> does, and expecting a different fstat():st_mode will break that. >> Unless something changed in OpenLDAP itself? >> >> Anyway, after re-reading ITS#4893 I'd be very wary about tweaking >> this code. Looks like at least I just gave up at the end, but >> followup#9 suggests using Solaris "doors". IIRC I did look at at >> the supposedly more general STREAMS pipes and found myself wading >> into portability issues or kernel tuning params or whateveer. >> >> -- >> Hallvard > > > There are several issues on Solaris 8: > > 1. getsockname(s, (struct sockaddr *)&lname, &llen) > > The llen parameter does not return the actual size of the socket address, > but the original buffer size. As a result, the comparison > !memcmp(&lname, &rname, llen) compares the whole buffer. > Since rname is not initialized to 0 for the entire buffer, the garbage > characters following the socket path name always fail the comparison. > > + if( err == 0 && st.st_mode == mode && llen == rlen > + && !memcmp(&lname, &rname, llen)) > > 2. The checking "llen == rlen" always fails as well. > > 3. The fchmod( fds[0], S_ISUID|S_IRWXU) on the client side does not change > the mode to include S_ISUID|S_IRWXU. The mode stays as only > S_IFIFO: > > (gdb) next > 81 if (rc = fchmod( fds[0], S_ISUID|S_IRWXU ) < 0) { > (gdb) print st > $2 = {...st_mode = 4096, ...} > (gdb) next > 85 (void) fstat(fds[0], &st); > (gdb) next > 86 write( fds[1], sa, salen ); > (gdb) print st > $3 = {...st_mode = 4096, ...} > > Trying to compare (st.st_mode == S_IFIFO|S_ISUID|S_IRWXU) > always fails.
As Hallvard points out, all of this code was known to work at one time. Apparently a subsequent Solaris patchlevel has broken it. I don't see a portable way to conditionalize this behavior now. > I have revised the patch to (1) initialize rname buffer, (2) ignore > the comparison "llen == rlen", (3) compare st.st_mode only against > S_IFIFO, but not S_ISUID|S_IRWXU. > > The socket path comparison demands full-path match, e.g., > "/var/suum/run/socket" > won't match against "/var/suum/run//socket". Nor should it. In this case whoever is using "//" in the path is misconfigured, as I already said before in private email. > > The revised patch is at: > https://dl.dropboxusercontent.com/u/94235048/ITS7575.patch > > Ted C. Cheng > Symas Corporation -- -- Howard Chu CTO, Symas Corp. http://www.symas.com Director, Highland Sun http://highlandsun.com/hyc/ Chief Architect, OpenLDAP http://www.openldap.org/project/
