netlink nlmsg_pid supposed to be pid or tid?

When using netlink, should "nlmsg_pid" be set to pid (current->tgid) or 
tid (current->pid)?

Chris

Friesen, Christopher [CAR:VC21:EXCH] wrote:

When using netlink, should "nlmsg_pid" be set to pid (current->tgid) or 
tid (current->pid)?

The reason I ask is that with 2.6.10 we had to set it to tid, which 
really doesn't make much sense given than sockets are shared across all 
threads in a process (and gettid() isn't even a libc function).

Chris

Christopher Friesen [off-list ref] wrote:

When using netlink, should "nlmsg_pid" be set to pid (current->tgid) or 
tid (current->pid)?

I tend to agree with you here that tgid makes more sense.  Dave, what
do you think?

However, you should never assume whatever value the kernel uses since
it may always choose an arbitrary value should the preferred pid/tgid
be taken by someone else.

So always use getsockname(2) to find out the correct address.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} [off-list ref]
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Hello!

I tend to agree with you here that tgid makes more sense.

I agree, apparently netlink_autobind was missed when sed'ing pid->tgid.
Of course, it does not matter, but tgid is nicer choice from user's viewpoint.

Alexey

Alexey Kuznetsov wrote:

Hello!

quoted

I tend to agree with you here that tgid makes more sense.

I agree, apparently netlink_autobind was missed when sed'ing pid->tgid.
Of course, it does not matter, but tgid is nicer choice from user's viewpoint.

I'm glad you agree, but I'm not sure what you mean by "it does not matter".

TIPC wants the user to fill in the pid to use in the nlmsghdr portion of 
a particular message.

When an NPTL child thread uses getpid() to specify the pid, it never 
receives a response to this request.  Running the same code on the 
parent works, and running the same code under Linuxthreads works.

Using gettid() works, but it also means that only the thread that issued 
the request can read the reply.

Chris

Christopher Friesen [off-list ref] wrote:

When an NPTL child thread uses getpid() to specify the pid, it never 
receives a response to this request.  Running the same code on the 
parent works, and running the same code under Linuxthreads works.

As I said before, you should not rely on getpid() to work.  Any other
process in the system can bind to your pid which makes it unavailable
to your process.  In which case the kernel will pick an arbitrary
negative pid as your address.

You should always use getsockaddr(2) to get your local address.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} [off-list ref]
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Alexey Kuznetsov [off-list ref] wrote:

I agree, apparently netlink_autobind was missed when sed'ing pid->tgid.
Of course, it does not matter, but tgid is nicer choice from user's viewpoint.

Great, here is the patch to do just that.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Thanks,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} [off-list ref]
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--

diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c

@@ -476,7 +476,7 @@ static int netlink_autobind(struct socke
 	struct hlist_head *head;
 	struct sock *osk;
 	struct hlist_node *node;
-	s32 pid = current->pid;
+	s32 pid = current->tgid;
 	int err;
 	static s32 rover = -4097;
 

Hello!

TIPC wants the user to fill in the pid to use in the nlmsghdr portion of 
a particular message.

It is wrong. netlink_pid used not to be associated with process pids.
Kernel used pid just as a seed to calculate a random value to bind,
when user did not bind explicitly. It is equal to current->pid occasionally.
F.e. libnetlink from iproute autobinds and gets netlink_pid with
getsockname().

When user binds the socket himself, he was free to bind to any value,
including pid and tgid.

Actually, I remember one discussion. Herbert, wait a minute...
That's it: February 2005, Subject: [PATCH] Add audit uid to netlink credentials
We decided (or not?) that binding to anything but tgid and pid
must be prohibited by security reasons. Apaprently, the finding was lost.

Alexey

Herbert Xu wrote:

As I said before, you should not rely on getpid() to work.

You should always use getsockaddr(2) to get your local address.

Should that be getsockname(2)?  Anyways, I now understand the issues.

As it turns out, we were actually already calling getsockname() a bit 
earlier in the code path, so we can just use the "pid" value from there.

Chris

On Tue, Nov 22, 2005 at 01:49:13AM +0300, Alexey Kuznetsov wrote:

Actually, I remember one discussion. Herbert, wait a minute...
That's it: February 2005, Subject: [PATCH] Add audit uid to netlink credentials
We decided (or not?) that binding to anything but tgid and pid
must be prohibited by security reasons. Apaprently, the finding was lost.

Thanks for reminding me.  We may still need to track that down (we
have now serialised most of the netlink processing so this my not be
as bad as it was).

However, I think explicit binding should still be allowed for root,
so nobody should take the PID for granted.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} [off-list ref]
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

On Mon, Nov 21, 2005 at 04:51:03PM -0600, Christopher Friesen wrote:

Should that be getsockname(2)?  Anyways, I now understand the issues.

Yes that's what I meant.

As it turns out, we were actually already calling getsockname() a bit 
earlier in the code path, so we can just use the "pid" value from there.

Excellent.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} [off-list ref]
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Tue, 22 Nov 2005 09:16:27 +1100

Alexey Kuznetsov [off-list ref] wrote:

quoted

I agree, apparently netlink_autobind was missed when sed'ing pid->tgid.
Of course, it does not matter, but tgid is nicer choice from user's viewpoint.

Great, here is the patch to do just that.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Applied, of course.

I can't for the life of me figure out how we missed this when
we fixed up all the current->pid references under net/.
Ulrich Drepper let us know that the problem existed, and
I was sure we eliminated all such cases.

It is possible we accidently reintroduced current->pid when
we redid all of the netlink hashing. :-)

On Tue, Nov 22, 2005 at 02:43:34PM -0800, David S. Miller wrote:

Applied, of course.

Thanks Dave.
 

It is possible we accidently reintroduced current->pid when
we redid all of the netlink hashing. :-)

I just checked using git-whatchanged and that line goes back to
2002 :)

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} [off-list ref]
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Wed, 23 Nov 2005 11:03:37 +1100

On Tue, Nov 22, 2005 at 02:43:34PM -0800, David S. Miller wrote:

quoted

It is possible we accidently reintroduced current->pid when
we redid all of the netlink hashing. :-)

I just checked using git-whatchanged and that line goes back to
2002 :)

Ho hum, I guess we just missed it on the current->pid
scan then :-)