Issue 8158 - cldap broken for aix and solaris
Summary: cldap broken for aix and solaris
Status: VERIFIED FIXED
Alias: None
Product: OpenLDAP
Classification: Unclassified
Component: slapd (show other issues)
Version: 2.4.40
Hardware: All All
: --- normal
Target Milestone: ---
Assignee: OpenLDAP project
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-05-29 12:42 UTC by goran.hammarback@foxt.com
Modified: 2015-08-18 17:41 UTC (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this issue.
Description goran.hammarback@foxt.com 2015-05-29 12:42:27 UTC
Full_Name: G.ran Hammarb.ck
Version: 2.4.40
OS: Solaris 11, aix 7.1
URL: ftp://ftp.openldap.org/incoming/
Submission from: (NULL) (192.71.43.2)


In the routine libraries/liblber/sockbuf.s:sb_dgram_write(), the last argument
to the sendto() call is sizeof( struct sockaddr_storage ). While this works on
Linux, on Solaris 11 and AIX 7.1 (and likely other versions of these OSes) the
call returns EINVAL. This causes attempts to use connectionless ldap to fail.

Here is a suggestion to a patch that has been tested on Solaris 11, AIX 7.1 and
RedHat 6:

*** libraries/liblber/sockbuf.c..org	2015-05-27 09:55:14.001639041 +0200
--- libraries/liblber/sockbuf.c
Comment 1 Hallvard Furuseth 2015-06-02 22:18:29 UTC
On 29/05/15 14:42, goran.hammarback@foxt.com wrote:
> Here is a suggestion to a patch that has been tested on Solaris 11, AIX 7.1 and
> RedHat 6:
>
> *** libraries/liblber/sockbuf.c..org	2015-05-27 09:55:14.001639041 +0200
> --- libraries/liblber/sockbuf.c

That patch is empty.  You can upload it to
ftp://ftp.openldap.org/incoming/your-name-YYMMDD.ext.
See the guidelines for contributing:
http://www.openldap.org/devel/contributing.html

Comment 2 goran.hammarback@foxt.com 2015-06-03 06:26:56 UTC
Not sure why the patch itself was not included. Anyway, I uploaded the
patch (it is trivial) as goran-hammarback-150203.ext.

Regards,

/Göran Hammarbäck

On ons, 2015-06-03 at 00:18 +0200, Hallvard Breien Furuseth wrote:
> On 29/05/15 14:42, goran.hammarback@foxt.com wrote:
> > Here is a suggestion to a patch that has been tested on Solaris 11, AIX 7.1 and
> > RedHat 6:
> >
> > *** libraries/liblber/sockbuf.c..org	2015-05-27 09:55:14.001639041 +0200
> > --- libraries/liblber/sockbuf.c
> 
> That patch is empty.  You can upload it to
> ftp://ftp.openldap.org/incoming/your-name-YYMMDD.ext.
> See the guidelines for contributing:
> http://www.openldap.org/devel/contributing.html



Comment 3 goran.hammarback@foxt.com 2015-06-03 06:46:17 UTC
Hmm, my fingers thought it was still february. Now I have uploaded a
file with the correct name, goran-hammarback-150603.ext.

Regards,
/Göran Hammarbäck

On ons, 2015-06-03 at 00:18 +0200, Hallvard Breien Furuseth wrote:
> On 29/05/15 14:42, goran.hammarback@foxt.com wrote:
> > Here is a suggestion to a patch that has been tested on Solaris 11, AIX 7.1 and
> > RedHat 6:
> >
> > *** libraries/liblber/sockbuf.c..org	2015-05-27 09:55:14.001639041 +0200
> > --- libraries/liblber/sockbuf.c
> 
> That patch is empty.  You can upload it to
> ftp://ftp.openldap.org/incoming/your-name-YYMMDD.ext.
> See the guidelines for contributing:
> http://www.openldap.org/devel/contributing.html



Comment 4 Hallvard Furuseth 2015-06-03 09:34:29 UTC
changed notes
changed state Open to Active
moved from Incoming to Software Bugs
Comment 5 Hallvard Furuseth 2015-06-04 21:06:30 UTC
On 03/06/15 08:47, goran.hammarback@foxt.com wrote:
> Hmm, my fingers thought it was still february. Now I have uploaded a
> file with the correct name, goran-hammarback-150603.ext.

The date is probably just for disambiguation anyway.

Tweaked it a bit, I expect it should look like this:
ftp://ftp.openldap.org/incoming/Hallvard-Furuseth-150604.diff

Is there a reason you switched to sizeof(struct sockaddr) from
sockaddr_storage as the fallback?  Otherwise sockaddr_storage
seems better, since that has presumably worked on most hosts.
Should only matter if people play tricks like using ldap_init_fd()
to create an unsupported connection type, e.g. "UDP ldapi://".

Anyway, please test (or protest if it should be sockaddr).
The change from your patch may be trivial, but that doesn't
mean I couldn't have managed to break it:-)

Note that OpenLDAP should be edited with editor tab width =
indentation = 4.  Though some code has been written with
tab width = 8, so it looks messy either way.

-- 
Hallvard


Comment 6 Hallvard Furuseth 2015-06-04 21:08:23 UTC
changed state Active to Feedback
Comment 7 goran.hammarback@foxt.com 2015-06-05 06:58:40 UTC
On tor, 2015-06-04 at 23:06 +0200, Hallvard Breien Furuseth wrote:
> On 03/06/15 08:47, goran.hammarback@foxt.com wrote:
> > Hmm, my fingers thought it was still february. Now I have uploaded a
> > file with the correct name, goran-hammarback-150603.ext.
> 
> The date is probably just for disambiguation anyway.
> 
> Tweaked it a bit, I expect it should look like this:
> ftp://ftp.openldap.org/incoming/Hallvard-Furuseth-150604.diff
> 
> Is there a reason you switched to sizeof(struct sockaddr) from
> sockaddr_storage as the fallback?  Otherwise sockaddr_storage
> seems better, since that has presumably worked on most hosts.
> Should only matter if people play tricks like using ldap_init_fd()
> to create an unsupported connection type, e.g. "UDP ldapi://".

Yeah, the fallback... I really did not know what to use there. I knew
sizeof(struct sockaddr_storage) would work on Linux but most likely not
on Solaris and AIX, so I used sizeof(struct sockaddr). Unfortunately I
have no way of testing if that really works at all for any platform so I
guess your way is probably better...

> 
> Anyway, please test (or protest if it should be sockaddr).
> The change from your patch may be trivial, but that doesn't
> mean I couldn't have managed to break it:-)
> 
> Note that OpenLDAP should be edited with editor tab width =
> indentation = 4.  Though some code has been written with
> tab width = 8, so it looks messy either way.
> 
Your patch looks fine. I will not have time to test it until next week
earliest. My patch was meant only as an example, so I did not really
think about indentation. 


Regards,

/Göran


Comment 8 Hallvard Furuseth 2015-06-05 07:13:43 UTC
On 05. juni 2015 08:59, goran.hammarback@foxt.com wrote:
> Yeah, the fallback... I really did not know what to use there. I knew
> sizeof(struct sockaddr_storage) would work on Linux but most likely not
> on Solaris and AIX, so I used sizeof(struct sockaddr). Unfortunately I
> have no way of testing if that really works at all for any platform so I
> guess your way is probably better...

Aha!  Yes, then it should be sockaddr_storage.  sockaddr breaks for
any address type which is bigger than sizeof(struct sockaddr), which
can be small. sockaddr_storage only breaks on an OS which complains
when it is too big for the specified address type.

-- 
Hallvard

Comment 9 Hallvard Furuseth 2015-06-15 19:27:12 UTC
changed notes
changed state Feedback to Test
Comment 10 Quanah Gibson-Mount 2015-07-28 15:46:39 UTC
changed notes
changed state Test to Release
Comment 11 Quanah Gibson-Mount 2015-08-18 17:41:25 UTC
changed notes
changed state Release to Partial
Comment 12 OpenLDAP project 2015-08-18 17:41:30 UTC
Fixed in master
Fixed in RE25
Fixed in RE24 (2.4.42)
Comment 13 Quanah Gibson-Mount 2015-08-18 17:41:30 UTC
changed notes
changed state Partial to Closed