Bug 7115 - Portability issues
Portability issues
Status: RESOLVED FIXED
Product: Samba 4.0
Classification: Unclassified
Component: Other
unspecified
x86 FreeBSD
: P3 normal
: ---
Assigned To: Karolin Seeger
Samba QA Contact
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-02-08 04:12 UTC by Christoph Theis
Modified: 2010-02-18 05:59 UTC (History)
1 user (show)

See Also:


Attachments
Patch for v3-5 (986 bytes, patch)
2010-02-17 08:40 UTC, Stefan Metzmacher
no flags Details
Patches for v3-5 (3.34 KB, patch)
2010-02-18 02:42 UTC, Stefan Metzmacher
kai: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Christoph Theis 2010-02-08 04:12:33 UTC
Recently I build Alpha 11 on FreeBSD. This time configure and compile went smooth, that is a big step forward! Trying to join another FreeBSD machine with Samba 3.[345] I came accross several issues in the tsocket library of Samba 4.

tsocket_bsd.c, function tdgram_bsd_recvfrom_handler: This calls tsocket_bsd_pending first which in turn makes an ioctl(fd, FIONREAD, ...) to get the number of pending bytes and then tries to recvfrom exactly that much bytes. For UDP sockets FreeBSDs implementation of FIONREAD does not only return the size of the UDP payload but includes the UDP header and the source and destination address as well, returning 16 bytes more than recvfrom will return. I have seen this in Wireshark as well: An UDP packet with a payload of 100 bytes arrived, the UDP length field was set to 108, FIONREAD returned 116 and recvfrom returned 100, no bytes from the payload missing. 
I don't understand why recvfrom could ever not read what is pending, a sufficient large buffer provided, and I don't know what FIONREAD will return if more than one UDP packet is pending, especially if those are from more than one source. In my opinion one should be happy with whatever recvfrom has read.

In the same file some calls are made to bind or connect with the length argument set to "sizeof(struct sockaddr_storage)". FreeBSD seems not to like it and returns the error EINVAL. There is already a special treatment for the AF_UNIX family, maybe it should be done for AF_INET and AF_INET6 as well.

There is one call to dsdb/sambd/ldb_modules/dsdb_module_search with a format string deliberately set to NULL. This function passes the format string as is to talloc_vasprintf which passes it on to vasprintf. I could not find anything about the expected behaviour of vasprintf with NULL as the format string. At least FreeBSD doesn't like it and dumps core.
Comment 1 Matthias Dieter Wallnöfer 2010-02-08 05:08:23 UTC
Well, I hope that the tsocket bugs will be fixed by metze since he is the author of the tsocket library I think.

Regarding the NULL format string: I think it's indeed a bug - yeah. I will push a fix.
Comment 2 Stefan Metzmacher 2010-02-17 08:40:24 UTC
Created attachment 5367 [details]
Patch for v3-5

Can you please test this? This should fix the tsocket_bsd_pending() problem with
tdgram_bsd_recvfrom*
Comment 3 Christoph Theis 2010-02-17 11:42:16 UTC
(In reply to comment #2)
> Created an attachment (id=5367) [details]
> Patch for v3-5
> 
> Can you please test this? This should fix the tsocket_bsd_pending() problem
> with
> tdgram_bsd_recvfrom*
> 

It's working :)
One remark: In the comment "some systems too much bytes" there is a verb missing.
A second remark: I still had to patch the calculation of sa_socklen. I can send you the patch I made for this.
And a third remark: If "v3-5" means Samba 3.5, then there should also be a patch for Samba 4.

Comment 4 Stefan Metzmacher 2010-02-17 12:09:55 UTC
I'll fix the comment:-)

Are the socklen problems fixed by the patch on bug
https://bugzilla.samba.org/show_bug.cgi?id=7140
Comment 5 Stefan Metzmacher 2010-02-17 12:15:32 UTC
The patch is already in the master branch
Comment 6 Christoph Theis 2010-02-17 12:21:51 UTC
(In reply to comment #4)
> Are the socklen problems fixed by the patch on bug
> https://bugzilla.samba.org/show_bug.cgi?id=7140

Yes, that includes my patch. And addresses also the IPv6 issue I had (I blamed my DNS setup for this).
Comment 7 Matthias Dieter Wallnöfer 2010-02-17 12:54:48 UTC
I ask as QA: Is it working now, Christoph? Then feel free to close with "FIXED".
Comment 8 Christoph Theis 2010-02-17 13:36:30 UTC
(In reply to comment #7)
> I ask as QA: Is it working now, Christoph? Then feel free to close with
> "FIXED".

Yes, it is working. Thanks a lot!
Comment 9 Stefan Metzmacher 2010-02-17 13:56:47 UTC
Kai, please assign the bug bug to Karolin, when you're done with our tests
and the patch can go into 3.5.0

Karolin: this should be picked after the patches from
https://bugzilla.samba.org/show_bug.cgi?id=7140
Comment 10 Kai Blin 2010-02-17 15:43:25 UTC
Pending the patch JRA added on top of this one, this seems to be ok. Metze, can you have a look at 936828de71023d90aaec6c1dba84052246bbad11 ?
Comment 11 Stefan Metzmacher 2010-02-18 02:42:04 UTC
Created attachment 5374 [details]
Patches for v3-5

This includes the fix from Jeremy and a additional patch that fixes the comment.
Comment 12 Kai Blin 2010-02-18 04:50:20 UTC
Comment on attachment 5374 [details]
Patches for v3-5

Looks good,
Comment 13 Kai Blin 2010-02-18 04:50:50 UTC
Karolin, please pick.
Comment 14 Karolin Seeger 2010-02-18 05:59:19 UTC
Pushed to v3-5-test.
Closing out bug report.

Thanks!