[{"id":1765601,"web_url":"http://patchwork.ozlabs.org/comment/1765601/","msgid":"<2ddbb6da514108b4e70ccd8362292134@codeaurora.org>","list_archive_url":null,"date":"2017-09-08T19:44:52","subject":"Re: [PATCH net] net: qualcomm: rmnet: Fix a double free","submitter":{"id":65547,"url":"http://patchwork.ozlabs.org/api/people/65547/","name":"Subash Abhinov Kasiviswanathan","email":"subashab@codeaurora.org"},"content":"> diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c\n> b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c\n> index 557c9bf1a469..0335fce54201 100644\n> --- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c\n> +++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c\n> @@ -95,10 +95,8 @@ struct sk_buff *rmnet_map_deaggregate(struct sk_buff \n> *skb)\n> \tskb_pull(skb, packet_len);\n> \n> \t/* Some hardware can send us empty frames. Catch them */\n> -\tif (ntohs(maph->pkt_len) == 0) {\n> -\t\tkfree_skb(skb);\n> +\tif (ntohs(maph->pkt_len) == 0)\n> \t\treturn NULL;\n> -\t}\n> \n> \treturn skbn;\n> }\n\nThanks for the patch. This is fixing the double free, but is leaking the \nnew skb skbn\ncreated. Perhaps we should add the check earlier -\n\ndiff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c \nb/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c\nindex 557c9bf..86b8c75 100644\n--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c\n+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c\n@@ -84,6 +84,10 @@ struct sk_buff *rmnet_map_deaggregate(struct sk_buff \n*skb)\n if (((int)skb->len - (int)packet_len) < 0)\n return NULL;\n\n+ /* Some hardware can send us empty frames. Catch them */\n+ if (ntohs(maph->pkt_len) == 0)\n+ return NULL;\n+\n skbn = alloc_skb(packet_len + RMNET_MAP_DEAGGR_SPACING, \nGFP_ATOMIC);\n if (!skbn)\n return NULL;\n@@ -94,11 +98,5 @@ struct sk_buff *rmnet_map_deaggregate(struct sk_buff \n*skb)\n memcpy(skbn->data, skb->data, packet_len);\n skb_pull(skb, packet_len);\n\n- /* Some hardware can send us empty frames. Catch them */\n- if (ntohs(maph->pkt_len) == 0) {\n- kfree_skb(skb);\n- return NULL;\n- }\n-\n return skbn;\n }","headers":{"Return-Path":"","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=)","ozlabs.org; dkim=pass (1024-bit key;\n\tunprotected) header.d=codeaurora.org header.i=@codeaurora.org\n\theader.b=\"l3ktsSIB\"; \n\tdkim=pass (1024-bit key) header.d=codeaurora.org\n\theader.i=@codeaurora.org header.b=\"l3ktsSIB\"; \n\tdkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xpnph36srz9t16\n\tfor ;\n\tSat, 9 Sep 2017 05:45:00 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1757109AbdIHTo6 (ORCPT );\n\tFri, 8 Sep 2017 15:44:58 -0400","from smtp.codeaurora.org ([198.145.29.96]:52004 \"EHLO\n\tsmtp.codeaurora.org\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1757075AbdIHTow (ORCPT\n\t); Fri, 8 Sep 2017 15:44:52 -0400","by smtp.codeaurora.org (Postfix, from userid 1000)\n\tid 45DA8607C5; Fri, 8 Sep 2017 19:44:52 +0000 (UTC)","from mail.codeaurora.org (localhost.localdomain [127.0.0.1])\n\tby smtp.codeaurora.org (Postfix) with ESMTP id 0408060588;\n\tFri, 8 Sep 2017 19:44:52 +0000 (UTC)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=codeaurora.org;\n\ts=default; t=1504899892;\n\tbh=TzOb1/0kNT+Fpgx5tbVZdeBdxe+qoZOKnCIgGbbEZMU=;\n\th=Date:From:To:Cc:Subject:In-Reply-To:References:From;\n\tb=l3ktsSIBQOIPzBsL6f8np8WDk+AC5PjOnYtUawvtyZcQjDIA7gaRfxggHF06kIuQq\n\tXFx8JnQqZlzsHjaM/8N5NGJ+RpkLqlqkUbOYF4BNIVYZ/aNA0WvGEzen6O+xOs12FD\n\tZ9nt5+yiEToQT7Kgigvkp6eQa0+w7qSDLNhEgKSo=","v=1; a=rsa-sha256; c=relaxed/simple; d=codeaurora.org;\n\ts=default; t=1504899892;\n\tbh=TzOb1/0kNT+Fpgx5tbVZdeBdxe+qoZOKnCIgGbbEZMU=;\n\th=Date:From:To:Cc:Subject:In-Reply-To:References:From;\n\tb=l3ktsSIBQOIPzBsL6f8np8WDk+AC5PjOnYtUawvtyZcQjDIA7gaRfxggHF06kIuQq\n\tXFx8JnQqZlzsHjaM/8N5NGJ+RpkLqlqkUbOYF4BNIVYZ/aNA0WvGEzen6O+xOs12FD\n\tZ9nt5+yiEToQT7Kgigvkp6eQa0+w7qSDLNhEgKSo="],"X-Spam-Checker-Version":"SpamAssassin 3.4.0 (2014-02-07) on\n\tpdx-caf-mail.web.codeaurora.org","X-Spam-Level":"","X-Spam-Status":"No, score=-2.8 required=2.0 tests=ALL_TRUSTED,BAYES_00,\n\tDKIM_SIGNED,\n\tT_DKIM_INVALID autolearn=no autolearn_force=no version=3.4.0","MIME-Version":"1.0","Content-Type":"text/plain; charset=US-ASCII;\n format=flowed","Content-Transfer-Encoding":"7bit","Date":"Fri, 08 Sep 2017 13:44:52 -0600","From":"Subash Abhinov Kasiviswanathan ","To":"Dan Carpenter ","Cc":"\"David S. Miller\" , netdev@vger.kernel.org,\n\tkernel-janitors@vger.kernel.org","Subject":"Re: [PATCH net] net: qualcomm: rmnet: Fix a double free","In-Reply-To":"<20170908102356.tzysh6qaiesd2umz@mwanda>","References":"<20170908102356.tzysh6qaiesd2umz@mwanda>","Message-ID":"<2ddbb6da514108b4e70ccd8362292134@codeaurora.org>","X-Sender":"subashab@codeaurora.org","User-Agent":"Roundcube Webmail/1.2.5","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"netdev@vger.kernel.org"}}]