[PATCH net-next-2.6] Phonet: remove dangling pipe if an endpoint is closed early

[PATCH net-next-2.6] Phonet: remove dangling pipe if an endpoint is closed early

[Rémi Denis-Courmont]

From: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>

Closing a pipe endpoint is not normally allowed by the Phonet pipe,
other than as a side after-effect of removing the pipe between two
endpoints. But there is no way to prevent Linux userspace processes
from being killed or suffering from bugs, so this can still happen.
We might as well forcefully close Phonet pipe endpoints then.

The cellular modem supports only a few existing pipes at a time. So we
really should not leak them. This change instructs the modem to destroy
the pipe if either of the pipe's endpoint (Linux socket) is closed too
early.

Signed-off-by: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
---
 include/net/phonet/pep.h |    5 +++++
 net/phonet/pep.c         |   27 ++++++++++++++++++++++++++-
 2 files changed, 31 insertions(+), 1 deletions(-)

diff --git a/include/net/phonet/pep.h b/include/net/phonet/pep.h
index 35672b1..37f23dc 100644
--- a/include/net/phonet/pep.h
+++ b/include/net/phonet/pep.h
@@ -77,6 +77,11 @@ static inline struct pnpipehdr *pnp_hdr(struct sk_buff *skb)
 #define MAX_PNPIPE_HEADER (MAX_PHONET_HEADER + 4)
 
 enum {
+	PNS_PIPE_CREATE_REQ = 0x00,
+	PNS_PIPE_CREATE_RESP,
+	PNS_PIPE_REMOVE_REQ,
+	PNS_PIPE_REMOVE_RESP,
+
 	PNS_PIPE_DATA = 0x20,
 	PNS_PIPE_ALIGNED_DATA,
 
diff --git a/net/phonet/pep.c b/net/phonet/pep.c
index 04e3419..d0e7eb2 100644
--- a/net/phonet/pep.c
+++ b/net/phonet/pep.c
@@ -620,6 +620,28 @@ drop:
 	return err;
 }
 
+static int pipe_do_remove(struct sock *sk)
+{
+	struct pep_sock *pn = pep_sk(sk);
+	struct pnpipehdr *ph;
+	struct sk_buff *skb;
+
+	skb = alloc_skb(MAX_PNPIPE_HEADER, GFP_KERNEL);
+	if (!skb)
+		return -ENOMEM;
+
+	skb_reserve(skb, MAX_PNPIPE_HEADER);
+	__skb_push(skb, sizeof(*ph));
+	skb_reset_transport_header(skb);
+	ph = pnp_hdr(skb);
+	ph->utid = 0;
+	ph->message_id = PNS_PIPE_REMOVE_REQ;
+	ph->pipe_handle = pn->pipe_handle;
+	ph->data[0] = PAD;
+
+	return pn_skb_send(sk, skb, &pipe_srv);
+}
+
 /* associated socket ceases to exist */
 static void pep_sock_close(struct sock *sk, long timeout)
 {
@@ -638,7 +660,10 @@ static void pep_sock_close(struct sock *sk, long timeout)
 		sk_for_each_safe(sknode, p, n, &pn->ackq)
 			sk_del_node_init(sknode);
 		sk->sk_state = TCP_CLOSE;
-	}
+	} else if ((1 << sk->sk_state) & (TCPF_SYN_RECV|TCPF_ESTABLISHED))
+		/* Forcefully remove dangling Phonet pipe */
+		pipe_do_remove(sk);
+
 	ifindex = pn->ifindex;
 	pn->ifindex = 0;
 	release_sock(sk);
-- 
1.7.0.4

Re: [PATCH net-next-2.6] Phonet: remove dangling pipe if an endpoint is closed early

[David Miller]

From: Rémi Denis-Courmont <remi@remlab.net>
Date: Thu, 16 Sep 2010 01:19:53 +0300

> From: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
> 
> Closing a pipe endpoint is not normally allowed by the Phonet pipe,
> other than as a side after-effect of removing the pipe between two
> endpoints. But there is no way to prevent Linux userspace processes
> from being killed or suffering from bugs, so this can still happen.
> We might as well forcefully close Phonet pipe endpoints then.
> 
> The cellular modem supports only a few existing pipes at a time. So we
> really should not leak them. This change instructs the modem to destroy
> the pipe if either of the pipe's endpoint (Linux socket) is closed too
> early.
> 
> Signed-off-by: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>

Applied.

Re: [PATCH net-next-2.6] Phonet: remove dangling pipe if an endpoint is closed early

[David Miller]

From: David Miller <davem@davemloft.net>
Date: Wed, 15 Sep 2010 21:32:58 -0700 (PDT)

> Applied.

Btw, I had to apply the following warning fix against your other
set of changes.

--------------------
phonet: Fix build warning.

net/phonet/socket.c: In function ‘pn_res_seq_show’:
net/phonet/socket.c:726: warning: format ‘%02X’ expects type ‘unsigned int’, but argument 3 has type ‘long int’

Signed-off-by: David S. Miller <davem@davemloft.net>
---
 net/phonet/socket.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/net/phonet/socket.c b/net/phonet/socket.c
index 6bf6e3c..aca8fba 100644
--- a/net/phonet/socket.c
+++ b/net/phonet/socket.c
@@ -723,7 +723,8 @@ static int pn_res_seq_show(struct seq_file *seq, void *v)
 		struct sock *sk = *psk;
 
 		seq_printf(seq, "%02X %5d %lu%n",
-			psk - pnres.sk, sock_i_uid(sk), sock_i_ino(sk), &len);
+			   (int) (psk - pnres.sk), sock_i_uid(sk),
+			   sock_i_ino(sk), &len);
 	}
 	seq_printf(seq, "%*s\n", 63 - len, "");
 	return 0;
-- 
1.7.2.2

Re: [PATCH net-next-2.6] Phonet: remove dangling pipe if an endpoint is closed early

[Rémi Denis-Courmont]

   Hello,

On Thursday 16 September 2010, David Miller wrote:
> From: David Miller <davem@davemloft.net>
> Date: Wed, 15 Sep 2010 21:32:58 -0700 (PDT)
> 
> > Applied.
> 
> Btw, I had to apply the following warning fix against your other
> set of changes.

Oh yeah, I only have 32-bits systems at hand so ptrdiff_t == int for me :-(
Sorry.

-- 
Rémi Denis-Courmont
http://www.remlab.net/