From patchwork Sun Mar 11 20:28:31 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: sjur.brandeland@stericsson.com X-Patchwork-Id: 146014 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id E50E8B6FA3 for ; Mon, 12 Mar 2012 07:28:50 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751406Ab2CKU2t (ORCPT ); Sun, 11 Mar 2012 16:28:49 -0400 Received: from mail-lpp01m010-f46.google.com ([209.85.215.46]:34588 "EHLO mail-lpp01m010-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751582Ab2CKU2r (ORCPT ); Sun, 11 Mar 2012 16:28:47 -0400 Received: by lahj13 with SMTP id j13so3064310lah.19 for ; Sun, 11 Mar 2012 13:28:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:from:to:cc:subject:date:message-id:x-mailer:mime-version :content-type:content-transfer-encoding; bh=x7vkpjD1MHN4IZydxR/UWJIzY2SLLPDePN8jZ3t8ifo=; b=v9c5jRg6I0p+G7pkIZ95O4O87KYtkyVTuaqMLuEiFpj3Z6cohi4uASdkpb7gBPR43i XKSXC56ZEA30Ncf82B6PkEYVH/hDk7N3KAfDaaCoz6XLdRHytZBHrGtoS4dT1NSkWKI2 20y2TRA3ng5fvhvazGjVAAo7EHjCNxFjs/oCEpiYEHaNd/I8wwoNMIFCTPyAr4q6zqoX L4YN4yxv7fs+LiCpGdTSeqkA3V2RbFlvwFuUDj5JWJOMhSPPUZ93XUM0BgZ/bX9XfiNC rKg1LDgpNrf6O9BAUl4aCvPY3OAe1MKGTY+NEqbWYzZM+gMaBJ5XAcWGveELfp+ETlfA KUlQ== Received: by 10.112.8.129 with SMTP id r1mr3728572lba.38.1331497725874; Sun, 11 Mar 2012 13:28:45 -0700 (PDT) Received: from localhost.localdomain ([80.203.142.207]) by mx.google.com with ESMTPS id tt8sm15530187lbb.16.2012.03.11.13.28.43 (version=TLSv1/SSLv3 cipher=OTHER); Sun, 11 Mar 2012 13:28:44 -0700 (PDT) From: =?UTF-8?q?Sjur=20Br=C3=A6ndeland?= To: netdev@vger.kernel.org, davem@davemloft.net Cc: sjurbren@gmail.com, Dmitry Tarnyagin , =?UTF-8?q?Sjur=20Br=C3=A6ndeland?= Subject: [PATCH net-next 1/2] caif: Fix for a race in socket transmit with flow control. Date: Sun, 11 Mar 2012 21:28:31 +0100 Message-Id: <1331497712-13534-1-git-send-email-sjur.brandeland@stericsson.com> X-Mailer: git-send-email 1.7.0.4 MIME-Version: 1.0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org From: Dmitry Tarnyagin 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 --- 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);