[Patches] [ python-Patches-1062014 ] fix for 764437 AF_UNIX socket special linux socket names

Fri Apr 14 10:33:03 CEST 2006

Patches item #1062014, was opened at 2004-11-07 18:18
Message generated for change (Comment added) made by arigo
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1062014&group_id=5470

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: Modules
Group: Python 2.4
Status: Open
Resolution: None
Priority: 5
Submitted By: Irmen de Jong (irmen)
Assigned to: Armin Rigo (arigo)
Summary: fix for 764437 AF_UNIX socket special linux socket names

Initial Comment:
The patch addresses two things:
1. the socket name (obtained using getsockname() or
getpeername() ) will now be correct in case of the
special linux socket names (starting with 0 byte)
2. the socket module now has a new constant:
UNIX_PATH_MAX , that can be used to build
correctly-sized path names.

----------------------------------------------------------------------

>Comment By: Armin Rigo (arigo)
Date: 2006-04-14 08:33

Message:
Logged In: YES 
user_id=4771

You're missing the point of my patch.  It is to preserve
the originally specified length of the name.  The names
'\x00abc', '\x00abc\x00' and '\x00abc\x00\x00' are all
different for the Linux IP stack, as far as I can tell.
In operations like bind() we specify the length of the
name by setting addrlen to the length of the header
plus the number of bytes actually used in the sun_path
array.  The kernel code remembers this length, and
passes it back in &addrlen to functions like
getsockaddr().  That's why I compute the number of
bytes that are really meaningful as the addrlen minus
the headers, instead of just UNIX_PATH_MAX.

About the second #ifdef, I really meant the > to be >
and not >=, because it is acceptable to pass a name
containing exactly UNIX_PATH_MAX bytes -- the abstract
namespace names don't have to be followed by a
terminating '\x00'.  By contrast, the regular names
must not be longer than UNIX_PATH_MAX-1 characters to
allow for the extra '\x00'.

In some sense, the Linux abstract namespace is rather
strange from the point of view of C, but not if you
think about it as you would think about Python strings.
Indeed, apart from the requirement that they start
with '\x00', these names have an intrinsic length
which is not related to any embedded '\x00'.  It is
stored together with the name in some kernel
structures, but outside the sun_path array of bytes.

But enough theory: the patch you propose don't pass
its own new test on my machine because of this :-)
Does it pass on yours?

I'm not too concerned about addrlen ending up shorter
than the size of the headers.  That would be a Linux
kernel bug.

----------------------------------------------------------------------

Comment By: Neal Norwitz (nnorwitz)
Date: 2006-04-14 05:01

Message:
Logged In: YES 
user_id=33168

Wow!  Now I see why you wanted review.

Modules/socketmodule.c:

In the first #ifdef linux section (968+), why the calc for
addrlen?  Isn't the len just sizeof(a->sun_path) and that
can be passed to FromStringAndSize()?

In the second #ifdef linux section (1098+), should if (len >
sizeof ...) be >= like the other check?

It's probably easier to show what I mean in code.  Attached
is another patch.  I didn't test it.  There's a big comment
that we should address in either patch about a negative
length issue.

----------------------------------------------------------------------

Comment By: Irmen de Jong (irmen)
Date: 2006-04-13 14:31

Message:
Logged In: YES 
user_id=129426

Armin, I don't have access to a Linux machine anymore so I
can't really comment on this issue anymore, sorry.
Also this patch was not made for a specific use case I had,
it was a patch for an open bug filed by someone else (bug
764437).
Previous discussion can be found there.

----------------------------------------------------------------------

Comment By: Armin Rigo (arigo)
Date: 2006-04-13 12:10

Message:
Logged In: YES 
user_id=4771

Hi Neal.  Although the patch is small I'd appreciate a
quick review, yes.  Also, Irmen, it would be nice to know
if my solution -- trying to preserve the length of the
addresses -- suits your original use case as well.

----------------------------------------------------------------------

Comment By: Neal Norwitz (nnorwitz)
Date: 2006-04-13 04:09

Message:
Logged In: YES 
user_id=33168

Armin, do you want someone to review this?  Is it
small/straightforward enough that it could be checked in? 
Just trying to figure out if there's anything I should do.

----------------------------------------------------------------------

Comment By: Armin Rigo (arigo)
Date: 2006-04-12 13:19

Message:
Logged In: YES 
user_id=4771

Added "#ifdef linux" in my patch, following the idea from
the original patch.

----------------------------------------------------------------------

Comment By: Armin Rigo (arigo)
Date: 2006-04-12 12:57

Message:
Logged In: YES 
user_id=4771

I have made an independent patch for this, based on another
interpretation of "man 7 unix": all the names in the Linux
abstract namespace don't have to be UNIX_PATH_MAX characters
long; instead, their length is remembered by the system and
available as 'addrlen', which is correctly passed around in
socketmodule.c.

The attached patch allows these special names of any length
to be set and read, and the length is preserved.  It does
not expose UNIX_PATH_MAX because it's not an essential
piece of information.  I've written a small test too.

I don't think the docs need updating for this patch, beyond
a mention in the NEWS file.

----------------------------------------------------------------------

Comment By: Irmen de Jong (irmen)
Date: 2004-11-08 11:05

Message:
Logged In: YES 
user_id=129426

Not only does it need a new regression test, but if it is
accepted as-is, the socket module docs need to be updated
too (mention the new constant for use with the special
naming case)

----------------------------------------------------------------------

Comment By: Johannes Gijsbers (jlgijsbers)
Date: 2004-11-07 20:11

Message:
Logged In: YES 
user_id=469548

As you noted on IRC: this needs a new regression test.

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1062014&group_id=5470