[PATCH net-next 1/2] caif: Fix for a race in socket transmit with flow control.

[PATCH net-next 1/2] caif: Fix for a race in socket transmit with flow control.

[Sjur Brændeland]

From: Dmitry Tarnyagin <dmitry.tarnyagin@stericsson.com>

Kill faulty checks on flow-off leading to connection drop at race conditions.
caif_socket checks for flow-on before transmitting and goes to sleep or
return -EAGAIN upon flow stop. Remove faulty subsequent checks on flow-off
leading to connection drop. Also fix memory leaks on some of the errors paths.

Signed-off-by: Sjur Brændeland <sjur.brandeland@stericsson.com>
---
 net/caif/cfdbgl.c  |    4 +++-
 net/caif/cfdgml.c  |    9 +++++++--
 net/caif/cfrfml.c  |   25 +++++++++++--------------
 net/caif/cfsrvl.c  |    6 +-----
 net/caif/cfutill.c |    5 ++++-
 net/caif/cfvidl.c  |    6 +++++-
 6 files changed, 31 insertions(+), 24 deletions(-)

diff --git a/net/caif/cfdbgl.c b/net/caif/cfdbgl.c
index 65d6ef3..2914659 100644
--- a/net/caif/cfdbgl.c
+++ b/net/caif/cfdbgl.c
@@ -41,8 +41,10 @@ static int cfdbgl_transmit(struct cflayer *layr, struct cfpkt *pkt)
 	struct caif_payload_info *info;
 	int ret;
 
-	if (!cfsrvl_ready(service, &ret))
+	if (!cfsrvl_ready(service, &ret)) {
+		cfpkt_destroy(pkt);
 		return ret;
+	}
 
 	/* Add info for MUX-layer to route the packet out */
 	info = cfpkt_info(pkt);
diff --git a/net/caif/cfdgml.c b/net/caif/cfdgml.c
index 0f5ff27..a63f4a5 100644
--- a/net/caif/cfdgml.c
+++ b/net/caif/cfdgml.c
@@ -86,12 +86,17 @@ static int cfdgml_transmit(struct cflayer *layr, struct cfpkt *pkt)
 	struct caif_payload_info *info;
 	struct cfsrvl *service = container_obj(layr);
 	int ret;
-	if (!cfsrvl_ready(service, &ret))
+
+	if (!cfsrvl_ready(service, &ret)) {
+		cfpkt_destroy(pkt);
 		return ret;
+	}
 
 	/* STE Modem cannot handle more than 1500 bytes datagrams */
-	if (cfpkt_getlen(pkt) > DGM_MTU)
+	if (cfpkt_getlen(pkt) > DGM_MTU) {
+		cfpkt_destroy(pkt);
 		return -EMSGSIZE;
+	}
 
 	cfpkt_add_head(pkt, &zero, 3);
 	packet_type = 0x08; /* B9 set - UNCLASSIFIED */
diff --git a/net/caif/cfrfml.c b/net/caif/cfrfml.c
index 6dc75d4..2b563ad 100644
--- a/net/caif/cfrfml.c
+++ b/net/caif/cfrfml.c
@@ -184,6 +184,11 @@ out:
 					rfml->serv.dev_info.id);
 	}
 	spin_unlock(&rfml->sync);
+
+	if (unlikely(err == -EAGAIN))
+		/* It is not possible to recover after drop of a fragment */
+		err = -EIO;
+
 	return err;
 }
 
@@ -218,7 +223,7 @@ static int cfrfml_transmit(struct cflayer *layr, struct cfpkt *pkt)
 	caif_assert(layr->dn->transmit != NULL);
 
 	if (!cfsrvl_ready(&rfml->serv, &err))
-		return err;
+		goto out;
 
 	err = -EPROTO;
 	if (cfpkt_getlen(pkt) <= RFM_HEAD_SIZE-1)
@@ -251,8 +256,11 @@ static int cfrfml_transmit(struct cflayer *layr, struct cfpkt *pkt)
 
 		err = cfrfml_transmit_segment(rfml, frontpkt);
 
-		if (err != 0)
+		if (err != 0) {
+			frontpkt = NULL;
 			goto out;
+		}
+
 		frontpkt = rearpkt;
 		rearpkt = NULL;
 
@@ -286,19 +294,8 @@ out:
 		if (rearpkt)
 			cfpkt_destroy(rearpkt);
 
-		if (frontpkt && frontpkt != pkt) {
-
+		if (frontpkt)
 			cfpkt_destroy(frontpkt);
-			/*
-			 * Socket layer will free the original packet,
-			 * but this packet may already be sent and
-			 * freed. So we have to return 0 in this case
-			 * to avoid socket layer to re-free this packet.
-			 * The return of shutdown indication will
-			 * cause connection to be invalidated anyhow.
-			 */
-			err = 0;
-		}
 	}
 
 	return err;
diff --git a/net/caif/cfsrvl.c b/net/caif/cfsrvl.c
index b99f5b2..4aa33d4 100644
--- a/net/caif/cfsrvl.c
+++ b/net/caif/cfsrvl.c
@@ -174,15 +174,11 @@ void cfsrvl_init(struct cfsrvl *service,
 
 bool cfsrvl_ready(struct cfsrvl *service, int *err)
 {
-	if (service->open && service->modem_flow_on && service->phy_flow_on)
-		return true;
 	if (!service->open) {
 		*err = -ENOTCONN;
 		return false;
 	}
-	caif_assert(!(service->modem_flow_on && service->phy_flow_on));
-	*err = -EAGAIN;
-	return false;
+	return true;
 }
 
 u8 cfsrvl_getphyid(struct cflayer *layer)
diff --git a/net/caif/cfutill.c b/net/caif/cfutill.c
index 53e49f3..86d2dad 100644
--- a/net/caif/cfutill.c
+++ b/net/caif/cfutill.c
@@ -84,8 +84,11 @@ static int cfutill_transmit(struct cflayer *layr, struct cfpkt *pkt)
 	caif_assert(layr != NULL);
 	caif_assert(layr->dn != NULL);
 	caif_assert(layr->dn->transmit != NULL);
-	if (!cfsrvl_ready(service, &ret))
+
+	if (!cfsrvl_ready(service, &ret)) {
+		cfpkt_destroy(pkt);
 		return ret;
+	}
 
 	cfpkt_add_head(pkt, &zero, 1);
 	/* Add info for MUX-layer to route the packet out. */
diff --git a/net/caif/cfvidl.c b/net/caif/cfvidl.c
index e3f37db..a8e2a2d 100644
--- a/net/caif/cfvidl.c
+++ b/net/caif/cfvidl.c
@@ -50,8 +50,12 @@ static int cfvidl_transmit(struct cflayer *layr, struct cfpkt *pkt)
 	struct caif_payload_info *info;
 	u32 videoheader = 0;
 	int ret;
-	if (!cfsrvl_ready(service, &ret))
+
+	if (!cfsrvl_ready(service, &ret)) {
+		cfpkt_destroy(pkt);
 		return ret;
+	}
+
 	cfpkt_add_head(pkt, &videoheader, 4);
 	/* Add info for MUX-layer to route the packet out */
 	info = cfpkt_info(pkt);
-- 
1.7.0.4

[PATCH net-next 2/2] caif: make zero a legal caif connetion id.

[Sjur Brændeland]

Connection ID configured through RTNL must allow zero as
connection id. If connection-id is not given when creating the
interface, configure a loopback interface using ifindex as
connection-id.

Signed-off-by: Sjur Brændeland <sjur.brandeland@stericsson.com>
---
 net/caif/chnl_net.c |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/net/caif/chnl_net.c b/net/caif/chnl_net.c
index e866234..20618dd 100644
--- a/net/caif/chnl_net.c
+++ b/net/caif/chnl_net.c
@@ -28,6 +28,7 @@
 /* 5 sec. connect timeout */
 #define CONNECT_TIMEOUT (5 * HZ)
 #define CAIF_NET_DEFAULT_QUEUE_LEN 500
+#define UNDEF_CONNID 0xffffffff
 
 /*This list is protected by the rtnl lock. */
 static LIST_HEAD(chnl_net_list);
@@ -408,7 +409,7 @@ static void ipcaif_net_setup(struct net_device *dev)
 	priv->conn_req.link_selector = CAIF_LINK_HIGH_BANDW;
 	priv->conn_req.priority = CAIF_PRIO_LOW;
 	/* Insert illegal value */
-	priv->conn_req.sockaddr.u.dgm.connection_id = 0;
+	priv->conn_req.sockaddr.u.dgm.connection_id = UNDEF_CONNID;
 	priv->flowenabled = false;
 
 	init_waitqueue_head(&priv->netmgmt_wq);
@@ -471,9 +472,11 @@ static int ipcaif_newlink(struct net *src_net, struct net_device *dev,
 	else
 		list_add(&caifdev->list_field, &chnl_net_list);
 
-	/* Take ifindex as connection-id if null */
-	if (caifdev->conn_req.sockaddr.u.dgm.connection_id == 0)
+	/* Use ifindex as connection id, and use loopback channel default. */
+	if (caifdev->conn_req.sockaddr.u.dgm.connection_id == UNDEF_CONNID) {
 		caifdev->conn_req.sockaddr.u.dgm.connection_id = dev->ifindex;
+		caifdev->conn_req.protocol = CAIFPROTO_DATAGRAM_LOOP;
+	}
 	return ret;
 }
 
-- 
1.7.0.4

Re: [PATCH net-next 1/2] caif: Fix for a race in socket transmit with flow control.

[David Miller]

From: Sjur Brændeland <sjur.brandeland@stericsson.com>
Date: Sun, 11 Mar 2012 21:28:31 +0100

> From: Dmitry Tarnyagin <dmitry.tarnyagin@stericsson.com>
> 
> Kill faulty checks on flow-off leading to connection drop at race conditions.
> caif_socket checks for flow-on before transmitting and goes to sleep or
> return -EAGAIN upon flow stop. Remove faulty subsequent checks on flow-off
> leading to connection drop. Also fix memory leaks on some of the errors paths.
> 
> Signed-off-by: Sjur Brændeland <sjur.brandeland@stericsson.com>

Applied.

Re: [PATCH net-next 2/2] caif: make zero a legal caif connetion id.

[David Miller]

From: Sjur Brændeland <sjur.brandeland@stericsson.com>
Date: Sun, 11 Mar 2012 21:28:32 +0100

> Connection ID configured through RTNL must allow zero as
> connection id. If connection-id is not given when creating the
> interface, configure a loopback interface using ifindex as
> connection-id.
> 
> Signed-off-by: Sjur Brændeland <sjur.brandeland@stericsson.com>

Applied.