Bug 7528 - Solaris with NIS autohome
Solaris with NIS autohome
Status: RESOLVED FIXED
Product: Samba 3.5
Classification: Unclassified
Component: VFS Modules
3.5.3
Sparc Solaris
: P3 major
: ---
Assigned To: Karolin Seeger
Samba QA Contact
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-06-21 10:19 UTC by corti
Modified: 2011-06-01 18:41 UTC (History)
2 users (show)

See Also:


Attachments
Proposed patch. (536 bytes, patch)
2010-06-21 15:23 UTC, Jeremy Allison
obnox: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description corti 2010-06-21 10:19:59 UTC
The routine automount_lookup in the file lib/util.c needs to be patched in order to have Samba work with NIS autohomes in Solaris (I am running Solaris 10 with all patches). The problem is that yp_match returns NL+NUL (and not only NUL) as the last two bytes in nis_result.
This is an excerpt from the yp_match man page:
"For  each  outkey and outval, two extra bytes of memory
 are allocated at the end  that  contain  NEWLINE  and  null,
 respectively,  but  these  two  bytes  are  not reflected in
 outkeylen or outvallen."
My patch consists in overwriting the NL character with NUL:
(line 1315)   nis_result[nis_result_len] = '\0';
I think this patch is also required for other UNIX systems. At least the HP-UX man page says the same about the NL+NUL.

Without the patch the users don't get their home mounted, and this is the entry in the Samba log file:
[...]
smbd/service.c:988(make_connection_snum)
  canonicalize_connect_path failed for service username, path /home/username_

Please note the trailing underscore at the end of the path.

PS:
This bug has been present for several years now; I've just checked with our previous 3.0.24 installation (which I had patched, too).
Comment 1 Jeremy Allison 2010-06-21 15:23:39 UTC
Created attachment 5803 [details]
Proposed patch.

Can you check if this patch works for you please ?
Jeremy.
Comment 2 corti 2010-06-22 04:45:16 UTC
Yes, the patch works. I've tested against 3.5.3, although the line numbers in the patch you've provided are wrong.
I think the test for "nis_result[nis_result_len] == '\n'" can be removed. The code would then overwrite a NUL with a NUL in the case that the last character is not '\n'. In theory even the test for "nis_result_len>0" is superfluous because nis_result_len won't be negative when yp_match returns with success.
Comment 3 Jeremy Allison 2010-06-22 14:15:56 UTC
No, I don't think any of the tests can be removed. All of them are defensive programming, which makes the code safer and more robust.

I'll commit to master, and get a review for 3.5.x.

Jeremy.
Comment 4 Jeremy Allison 2010-06-22 15:00:14 UTC
Comment on attachment 5803 [details]
Proposed patch.

Hi Michael, can you review for 3.5.next. If you're ok, re-assign to Karolin for inclusion.

Thanks,

Jeremy.
Comment 5 Michael Adam 2010-06-23 02:27:13 UTC
(In reply to comment #4)
> (From update of attachment 5803 [details])
> Hi Michael, can you review for 3.5.next. If you're ok, re-assign to Karolin for
> inclusion.
> 
> Thanks,
> 
> Jeremy.

The patch looks good.

But I do not have a test environment to check whether this
can have undesired side effects on non-solaris systems.

So before giving an ACK, I would like to clarify that bit.

Michael
Comment 6 Michael Adam 2011-05-31 21:22:45 UTC
Comment on attachment 5803 [details]
Proposed patch.

oops this was still open for review. sorry for the delay...
Comment 7 Michael Adam 2011-05-31 21:23:26 UTC
==> Karolin
Comment 8 Karolin Seeger 2011-06-01 18:41:07 UTC
Pushed to v3-5-test (and verified that it is already in v3-6-test).
Closing out bug report.

Thanks!