Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Socket.xs does not handle correctly UNIX domain sockets on FreeBSD #9788

Closed
p5pRT opened this issue Jul 7, 2009 · 10 comments
Closed

Socket.xs does not handle correctly UNIX domain sockets on FreeBSD #9788

p5pRT opened this issue Jul 7, 2009 · 10 comments

Comments

@p5pRT
Copy link

p5pRT commented Jul 7, 2009

Migrated from rt.perl.org#67298 (status was 'resolved')

Searchable as RT67298$

@p5pRT
Copy link
Author

p5pRT commented Jul 7, 2009

From btik-fbsd@scoubidou.com

This is a bug report for perl from btik-fbsd@​scoubidou.com,
generated with the help of perlbug 1.36 running under perl 5.10.0.

Bug with UNIX domain sockets on FreeBSD (at least 7.0 until 7.2
RELEASE).

The unpack_sockaddr_un of perl-5.10.0/ext/Socket/Socket.xs does not
handle correctly struct sockaddr_un under FreeBSD in some cases.

It returns errors like​:
  Bad arg length for Socket​::unpack_sockaddr_un, length is 16,
  should be 106 at ...

When the system returns a struct sockaddr_un in getpeername(), it
returns the same structure the server gives to bind().

In some cases, servers (X for /tmp/.X11-unix/X0 and devd for
/var/run/devd.pipe, for example) give a shortest structure than
sizeof(struct sockaddr_un) but with a correct sun_len field. In these
cases, sun_path may not be '\0' terminated.

Then getpeername() returns a structure that Socket.xs
unpack_sockaddr_un() function think wrong sized. But it is not.

One can reproduce the problem with the following simple server and
client code.

Server side :


use strict;
use Socket;

my $name = $ARGV[0] or die "usage​: $0 server_socket_file";

#my $sun = sockaddr_un($name);
# Use a shortest sockaddr_un as FreeBSD allows it for its daemons
my $sun = pack('CCA*', length($name)+2, 1, $name);

socket(my $fh, PF_UNIX, SOCK_STREAM, 0) or die "socket​: $!\n";
bind($fh, $sun) or die "bind​: $!\n";
listen($fh, 1) or die "listen​: $!\n";
while (1)
{
  accept(my $new_fh, $fh) or die "accept​: $!\n";
  sleep 1;
}


Client side (note that /var/run/devd.pipe socket is always available
on standard FreeBSD installation)​:


use strict;
use Socket;

my $name = $ARGV[0] // '/var/run/devd.pipe';

warn "*** Connecting to $name\n";
my $sun = sockaddr_un($name);

socket(my $fh, PF_UNIX, SOCK_STREAM, 0) or die "socket​: $!\n";
connect($fh, $sun) or die "connect​: $!\n";

my $sockaddr = getpeername($fh) or die "getpeername​: $!\n";
my $warn;

($warn = $sockaddr) =~ s/([\x00-\x19\x7f-\xff])/sprintf("<%02x>",ord$1)/ge;
warn "hexdump​: $warn--\n";
warn "real length​: " . length($sockaddr) . "\n";
warn "sun_len field​: " . unpack('C', $sockaddr) . "\n";
my $file = unpack_sockaddr_un($sockaddr);
warn "unpack​: $file--\n";

close $fh;


I join a patch that correct the problem on FreeBSD.

Note that the version in the GIT repository have the same problem.

Perhaps the same problem occurs on other BSD (Open, Net, DragonFly),
but I don't have any of them to try...

Many thanks for your work, dont hesitate to contact me to do some
tests if you need...

Best regards,

Maxime Soulé.

Patch to perl-5.10.0/ext/Socket/Socket.xs :

Inline Patch
--- perl-5.10.0/ext/Socket/Socket.xs.orig       2009-07-07 17:29:59.000000000 +0200
+++ perl-5.10.0/ext/Socket/Socket.xs    2009-07-07 18:25:57.000000000 +0200
@@ -351,19 +351,25 @@
 #ifdef I_SYS_UN
        struct sockaddr_un addr;
        STRLEN sockaddrlen;
+       STRLEN sockaddrmaxlen;
        char * sun_ad = SvPVbyte(sun_sv,sockaddrlen);
        char * e;
+#   ifdef __FreeBSD__
+       sockaddrmaxlen = addr.sun_len;
+#   else
+       sockaddrmaxlen = sizeof(addr);
+#   endif
 #   ifndef __linux__
        /* On Linux sockaddrlen on sockets returned by accept, recvfrom,
           getpeername and getsockname is not equal to sizeof(addr). */
-       if (sockaddrlen != sizeof(addr)) {
+       if (sockaddrlen != sockaddrmaxlen) {
            croak("Bad arg length for %s, length is %d, should be %d",
                        "Socket::unpack_sockaddr_un",
-                       sockaddrlen, sizeof(addr));
+                       sockaddrlen, sockaddrmaxlen);
        }
 #   endif
 
-       Copy( sun_ad, &addr, sizeof addr, char );
+       Copy( sun_ad, &addr, sockaddrmaxlen, char );
 
        if ( addr.sun_family != AF_UNIX ) {
            croak("Bad address family for %s, got %d, should be %d",
@@ -372,11 +378,18 @@
                        AF_UNIX);
        }
        e = (char*)addr.sun_path;
+#   ifdef __FreeBSD__
+       /* On FreeBSD sun_path ends not always with a '\0'.
+        * How do other BSDs work? */
+        while (e < (char*)&addr + sockaddrmaxlen && *e)
+           ++e;
+#   else
        /* On Linux, the name of abstract unix domain sockets begins
         * with a '\0', so allow this. */
        while ((*e || (e == addr.sun_path && e[1] && sockaddrlen > 1))
                && e < (char*)addr.sun_path + sizeof addr.sun_path)
            ++e;
+#   endif
        ST(0) = sv_2mortal(newSVpvn(addr.sun_path, e - (char*)addr.sun_path));
 #else
        ST(0) = (SV *) not_here("unpack_sockaddr_un");
---
Flags:   category=library   severity=medium

Site configuration information for perl 5.10.0​:

Configured by max at Wed Jun 10 09​:50​:59 CEST 2009.

Summary of my perl5 (revision 5 version 10 subversion 0) configuration​:
  Platform​:
  osname=freebsd, osvers=7.2-release, archname=amd64-freebsd-thread-multi
  uname='freebsd auber.mobigard.com 7.2-release freebsd 7.2-release #1​: mon may 4 19​:16​:51 cest 2009 max@​auber.mobigard.com​:usrobjusrsrcsysauber amd64 '
  config_args='-sde -Dprefix=/usr/local -Darchlib=/usr/local/lib/perl5/5.10.0/mach -Dprivlib=/usr/local/lib/perl5/5.10.0 -Dman3dir=/usr/local/lib/perl5/5.10.0/perl/man/man3 -Dman1dir=/usr/local/man/man1 -Dsitearch=/usr/local/lib/perl5/site_perl/5.10.0/mach -Dsitelib=/usr/local/lib/perl5/site_perl/5.10.0 -Dscriptdir=/usr/local/bin -Dsiteman3dir=/usr/local/lib/perl5/5.10.0/man/man3 -Dsiteman1dir=/usr/local/man/man1 -Ui_malloc -Ui_iconv -Uinstallusrbinperl -Dcc=cc -Duseshrplib -Dinc_version_list=none -Dccflags=-DAPPLLIB_EXP="/usr/local/lib/perl5/5.10.0/BSDPAN" -Doptimize=-O2 -fno-strict-aliasing -pipe -march=nocona -Ud_dosuid -Ui_gdbm -Dusethreads=y -Dusemymalloc=n -Duse64bitint'
  hint=recommended, useposix=true, d_sigaction=define
  useithreads=define, usemultiplicity=define
  useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef
  use64bitint=define, use64bitall=define, uselongdouble=undef
  usemymalloc=n, bincompat5005=undef
  Compiler​:
  cc='cc', ccflags ='-DAPPLLIB_EXP="/usr/local/lib/perl5/5.10.0/BSDPAN" -DHAS_FPSETMASK -DHAS_FLOATINGPOINT_H -fno-strict-aliasing -pipe -I/usr/local/include',
  optimize='-O2 -fno-strict-aliasing -pipe -march=nocona',
  cppflags='-DAPPLLIB_EXP="/usr/local/lib/perl5/5.10.0/BSDPAN" -DHAS_FPSETMASK -DHAS_FLOATINGPOINT_H -fno-strict-aliasing -pipe -I/usr/local/include'
  ccversion='', gccversion='4.2.1 20070719 [FreeBSD]', gccosandvers=''
  intsize=4, longsize=8, ptrsize=8, doublesize=8, byteorder=12345678
  d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16
  ivtype='long', ivsize=8, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8
  alignbytes=8, prototype=define
  Linker and Libraries​:
  ld='cc', ldflags ='-pthread -Wl,-E -L/usr/local/lib'
  libpth=/usr/lib /usr/local/lib
  libs=-lgdbm -lm -lcrypt -lutil
  perllibs=-lm -lcrypt -lutil
  libc=, so=so, useshrplib=true, libperl=libperl.so
  gnulibc_version=''
  Dynamic Linking​:
  dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags=' -Wl,-R/usr/local/lib/perl5/5.10.0/mach/CORE'
  cccdlflags='-DPIC -fPIC', lddlflags='-shared -L/usr/local/lib'

Locally applied patches​:
 


@​INC for perl 5.10.0​:
  /usr/local/lib/perl5/5.10.0/BSDPAN
  /usr/local/lib/perl5/site_perl/5.10.0/mach
  /usr/local/lib/perl5/site_perl/5.10.0
  /usr/local/lib/perl5/5.10.0/mach
  /usr/local/lib/perl5/5.10.0
  .


Environment for perl 5.10.0​:
  HOME=/home/max
  LANG=fr_FR.ISO8859-1
  LANGUAGE (unset)
  LD_LIBRARY_PATH (unset)
  LOGDIR (unset)
  PATH=/sbin​:/bin​:/usr/sbin​:/usr/bin​:/usr/local/bin​:/usr/X11R6/bin​:/usr/local/sbin
  PERL_BADLANG (unset)
  SHELL=/usr/local/bin/zsh

@p5pRT
Copy link
Author

p5pRT commented Jul 8, 2009

From @smpeters

On Tue, Jul 7, 2009 at 11​:53 AM, btik-fbsd@​scoubidou.com (via RT) <
perlbug-followup@​perl.org> wrote​:

# New Ticket Created by btik-fbsd@​scoubidou.com
# Please include the string​: [perl #67298]
# in the subject line of all future correspondence about this issue.
# <URL​: http​://rt.perl.org/rt3/Ticket/Display.html?id=67298 >

This is a bug report for perl from btik-fbsd@​scoubidou.com,
generated with the help of perlbug 1.36 running under perl 5.10.0.

Bug with UNIX domain sockets on FreeBSD (at least 7.0 until 7.2
RELEASE).

The unpack_sockaddr_un of perl-5.10.0/ext/Socket/Socket.xs does not
handle correctly struct sockaddr_un under FreeBSD in some cases.

It returns errors like​:
Bad arg length for Socket​::unpack_sockaddr_un, length is 16,
should be 106 at ...

When the system returns a struct sockaddr_un in getpeername(), it
returns the same structure the server gives to bind().

In some cases, servers (X for /tmp/.X11-unix/X0 and devd for
/var/run/devd.pipe, for example) give a shortest structure than
sizeof(struct sockaddr_un) but with a correct sun_len field. In these
cases, sun_path may not be '\0' terminated.

Then getpeername() returns a structure that Socket.xs
unpack_sockaddr_un() function think wrong sized. But it is not.

One can reproduce the problem with the following simple server and
client code.

Server side :
---------------------------------------
use strict;
use Socket;

my $name = $ARGV[0] or die "usage​: $0 server_socket_file";

#my $sun = sockaddr_un($name);
# Use a shortest sockaddr_un as FreeBSD allows it for its daemons
my $sun = pack('CCA*', length($name)+2, 1, $name);

socket(my $fh, PF_UNIX, SOCK_STREAM, 0) or die "socket​: $!\n";
bind($fh, $sun) or die "bind​: $!\n";
listen($fh, 1) or die "listen​: $!\n";
while (1)
{
accept(my $new_fh, $fh) or die "accept​: $!\n";
sleep 1;
}
---------------------------------------

Client side (note that /var/run/devd.pipe socket is always available
on standard FreeBSD installation)​:
---------------------------------------
use strict;
use Socket;

my $name = $ARGV[0] // '/var/run/devd.pipe';

warn "*** Connecting to $name\n";
my $sun = sockaddr_un($name);

socket(my $fh, PF_UNIX, SOCK_STREAM, 0) or die "socket​: $!\n";
connect($fh, $sun) or die "connect​: $!\n";

my $sockaddr = getpeername($fh) or die "getpeername​: $!\n";
my $warn;

($warn = $sockaddr) =~ s/([\x00-\x19\x7f-\xff])/sprintf("<%02x>",ord$1)/ge;
warn "hexdump​: $warn--\n";
warn "real length​: " . length($sockaddr) . "\n";
warn "sun_len field​: " . unpack('C', $sockaddr) . "\n";
my $file = unpack_sockaddr_un($sockaddr);
warn "unpack​: $file--\n";

close $fh;
---------------------------------------

I join a patch that correct the problem on FreeBSD.

Note that the version in the GIT repository have the same problem.

Perhaps the same problem occurs on other BSD (Open, Net, DragonFly),
but I don't have any of them to try...

Many thanks for your work, dont hesitate to contact me to do some
tests if you need...

Best regards,

Maxime Soulé.

Patch to perl-5.10.0/ext/Socket/Socket.xs :

--- perl-5.10.0/ext/Socket/Socket.xs.orig 2009-07-07
17​:29​:59.000000000 +0200
+++ perl-5.10.0/ext/Socket/Socket.xs 2009-07-07 18​:25​:57.000000000 +0200
@​@​ -351,19 +351,25 @​@​
#ifdef I_SYS_UN
struct sockaddr_un addr;
STRLEN sockaddrlen;
+ STRLEN sockaddrmaxlen;
char * sun_ad = SvPVbyte(sun_sv,sockaddrlen);
char * e;
+# ifdef __FreeBSD__
+ sockaddrmaxlen = addr.sun_len;
+# else
+ sockaddrmaxlen = sizeof(addr);
+# endif
# ifndef __linux__
/* On Linux sockaddrlen on sockets returned by accept, recvfrom,
getpeername and getsockname is not equal to sizeof(addr). */
- if (sockaddrlen != sizeof(addr)) {
+ if (sockaddrlen != sockaddrmaxlen) {
croak("Bad arg length for %s, length is %d, should be %d",
"Socket​::unpack_sockaddr_un",
- sockaddrlen, sizeof(addr));
+ sockaddrlen, sockaddrmaxlen);
}
# endif

- Copy( sun_ad, &addr, sizeof addr, char );
+ Copy( sun_ad, &addr, sockaddrmaxlen, char );

   if \( addr\.sun\_family \!= AF\_UNIX \) \{
       croak\("Bad address family for %s\, got %d\, should be %d"\,

@​@​ -372,11 +378,18 @​@​
AF_UNIX);
}
e = (char*)addr.sun_path;
+# ifdef __FreeBSD__
+ /* On FreeBSD sun_path ends not always with a '\0'.
+ * How do other BSDs work? */
+ while (e < (char*)&addr + sockaddrmaxlen && *e)
+ ++e;
+# else
/* On Linux, the name of abstract unix domain sockets begins
* with a '\0', so allow this. */
while ((*e || (e == addr.sun_path && e[1] && sockaddrlen > 1))
&& e < (char*)addr.sun_path + sizeof addr.sun_path)
++e;
+# endif
ST(0) = sv_2mortal(newSVpvn(addr.sun_path, e -
(char*)addr.sun_path));
#else
ST(0) = (SV *) not_here("unpack_sockaddr_un");

I'll see if I can get a workable test script for those tomorrow once I have
a FreeBSD 7+ set up locally.

Steve Peters
steve@​fisharerojo.org

@p5pRT
Copy link
Author

p5pRT commented Jul 8, 2009

The RT System itself - Status changed from 'new' to 'open'

@p5pRT
Copy link
Author

p5pRT commented Jun 26, 2013

From @tonycoz

On Tue Jul 07 09​:53​:21 2009, maximum.solo wrote​:

This is a bug report for perl from btik-fbsd@​scoubidou.com,
generated with the help of perlbug 1.36 running under perl 5.10.0.

Bug with UNIX domain sockets on FreeBSD (at least 7.0 until 7.2
RELEASE).

The unpack_sockaddr_un of perl-5.10.0/ext/Socket/Socket.xs does not
handle correctly struct sockaddr_un under FreeBSD in some cases.

It returns errors like​:
Bad arg length for Socket​::unpack_sockaddr_un, length is 16,
should be 106 at ...

Hi,

I can reproduce the failure with your test code on FreeBSD 9.1.

Socket.xs has changed significantly since you submitted this changed,
and is now upstream CPAN.

Could you please produce a new patch and submit it as a ticket to Socket?

Otherwise I'll take a closer look at some point.

Tony

@p5pRT
Copy link
Author

p5pRT commented Jun 26, 2013

@tonycoz - Status changed from 'open' to 'stalled'

@p5pRT
Copy link
Author

p5pRT commented Jun 28, 2013

From btik-fbsd@scoubidou.com

Hi Tony,

I will try to have a look at this tonight.

Stay tuned :)

Best regards,

Max.

Le 26.06.2013 02​:33, Tony Cook via RT a écrit :

On Tue Jul 07 09​:53​:21 2009, maximum.solo wrote​:

This is a bug report for perl from btik-fbsd@​scoubidou.com,
generated with the help of perlbug 1.36 running under perl 5.10.0.

Bug with UNIX domain sockets on FreeBSD (at least 7.0 until 7.2
RELEASE).

The unpack_sockaddr_un of perl-5.10.0/ext/Socket/Socket.xs does not
handle correctly struct sockaddr_un under FreeBSD in some cases.

It returns errors like​:
Bad arg length for Socket​::unpack_sockaddr_un, length is 16,
should be 106 at ...

Hi,

I can reproduce the failure with your test code on FreeBSD 9.1.

Socket.xs has changed significantly since you submitted this changed,
and is now upstream CPAN.

Could you please produce a new patch and submit it as a ticket to Socket?

Otherwise I'll take a closer look at some point.

Tony

@p5pRT
Copy link
Author

p5pRT commented Jun 28, 2013

The RT System itself - Status changed from 'stalled' to 'open'

@p5pRT
Copy link
Author

p5pRT commented Jul 1, 2013

From btik-fbsd@scoubidou.com

Le Mar. Jun. 25 17​:33​:17 2013, tonyc a �crit�​:

On Tue Jul 07 09​:53​:21 2009, maximum.solo wrote​:

This is a bug report for perl from btik-fbsd@​scoubidou.com,
generated with the help of perlbug 1.36 running under perl 5.10.0.

Bug with UNIX domain sockets on FreeBSD (at least 7.0 until 7.2
RELEASE).

The unpack_sockaddr_un of perl-5.10.0/ext/Socket/Socket.xs does not
handle correctly struct sockaddr_un under FreeBSD in some cases.

It returns errors like​:
Bad arg length for Socket​::unpack_sockaddr_un, length is 16,
should be 106 at ...

Hi,

I can reproduce the failure with your test code on FreeBSD 9.1.

Socket.xs has changed significantly since you submitted this changed,
and is now upstream CPAN.

Could you please produce a new patch and submit it as a ticket to Socket?

Otherwise I'll take a closer look at some point.

Tony

Hi Tony,

Done​: https://rt.cpan.org/Ticket/Display.html?id=86613

Best regards,

Max.

@p5pRT
Copy link
Author

p5pRT commented Sep 12, 2013

From @tonycoz

On Mon Jul 01 13​:23​:03 2013, maximum.solo wrote​:

Done​: https://rt.cpan.org/Ticket/Display.html?id=86613

Since this was fixed in Socket 2.011 and no longer crashes when I test
it against blead, I'm marking this closed.

Tony

@p5pRT
Copy link
Author

p5pRT commented Sep 12, 2013

@tonycoz - Status changed from 'open' to 'resolved'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant