Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 27 Sep 2022 10:57:28 +0300
From:   Dan Carpenter <dan.carpenter@oracle.com>
To:     xkernel.wang@foxmail.com
Cc:     Larry.Finger@lwfinger.net, florian.c.schilhabel@googlemail.com,
        gregkh@linuxfoundation.org, linux-staging@lists.linux.dev,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3] staging: rtl8712: fix potential memory leak in
 _r8712_init_xmit_priv()
Message-ID: <YzKs6BYl4lr9eU35@kadam>
References: <tencent_F11EC221954CFE8F406298A3732539562809@qq.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <tencent_F11EC221954CFE8F406298A3732539562809@qq.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?Xy/vo5NXFSrmChBJsNFFG6L6jWGEZU7QRGx0MfKV+b5qWoHE/6WXDGCEV/JO?=
 =?us-ascii?Q?/PsZL1uudL7g1P/FBmBZMPA7vkG7NDd/ZdeFDRKRRfRD1mdBoyy/o6b1Da1W?=
 =?us-ascii?Q?LxkAuHGL6tfnVJNChbN8SxuBFNBh2Y1rsNOrN7SmyWCw0xPnqSfhW66k78bL?=
 =?us-ascii?Q?cAFM3a8KzMONi6mUqFFaq4tHU/6UuJ8QtQFBT0VrRibn+Jcug0bBuLExN2C6?=
 =?us-ascii?Q?T4Z9yMYbPbgxq0JMfX7nJXfDW0JYEYqdH18bhNp0x011xACmEtqnHpy9oZqW?=
 =?us-ascii?Q?IGydPlfG3gu+s7xeNeD7dmmVeVrYLZ5dgeradzSCabwBXhZeI4hHmzMovnBg?=
 =?us-ascii?Q?Z8ObVaJce5Cn4Qnep6lE3bRuvgvn71OWtekgI51Qi3tvQ8SGBY+ODu7GFyEV?=
 =?us-ascii?Q?Cj7+irSlstGtwtGW4E9mZX3KGZxpLmU4WHGFsvyIrSWAxNKYupvRjztYUZT+?=
 =?us-ascii?Q?4J1JnRDeBubz65FaSD1RAxJLBNmcTuI1h5w30kmpq8I2tJeM2QMgBZGrATaS?=
 =?us-ascii?Q?nXG0ADqfkyJe8/8EKKMBCTsBWW4+2gusX0uc2OIKMraGd2xfZp8ylnoK0I9M?=
 =?us-ascii?Q?9fSvdeKe775S3icBvyhCdokWC2SdapXYuHzMJJNH8KsDAeaiYDRXfkG7LK7B?=
 =?us-ascii?Q?WqI7+TuCLdAGr+KMty1ZEUfbOUdehGh/2Cgq6B9mjaaFyeRPsKxkW3m6jvYk?=
 =?us-ascii?Q?gRqxIpyEMOVen41eb6mhehgZl/+ApU9w9ESobMwDTV7otIJbP12NnofHyWm8?=
 =?us-ascii?Q?oNH6KNoPoTTyi1bZBf11ZmrGT59tsPpO1on4opVOp+W+yEfG0IsOqi23XSVi?=
 =?us-ascii?Q?zWn+7SKCUhxuRwrE1G3M5An3JX7QMQFdcILkKjdUjIscoW/Q08wNe/4gDIdp?=
 =?us-ascii?Q?p5ooL+nx/l18Mu2rhRucMZRxQJSH7Qjla6nivwFW1BphEQlhaKr3/MOFqmv5?=
 =?us-ascii?Q?JMf2vZS9fu2zH7IUglxjzGmfI64gL+mkNf1HZIj47FIr6EkfjHo+t/tMb7qL?=
 =?us-ascii?Q?+nOxKwwtY7QJYzJtZe9Qzt5ie0zGuYkRltimJ0qlIUxsypdjAblawC7A3ws6?=
 =?us-ascii?Q?8JkH3c/Y6IgYzzq+9q8ZUbnQ1EZjheZNsCl2koJ9DGERSqMEtPbHyp+tuIMk?=
 =?us-ascii?Q?zJYhI0mXXGOWtKoVBzHdA0yBoVS7OhkXCTx2ge1sO769Hr73NWMVtH9cJcCo?=
 =?us-ascii?Q?1B1fUTaSdVQohba4I0kRx65oEFRyXq+hh9dWzXnojt9EUexx65AeyuwiKFjW?=
 =?us-ascii?Q?dXuOPK0PFDvjvD6LccawfOoLt6yFFf7JcXdpNYrfTalj68G4QVXXMx9k/GYJ?=
 =?us-ascii?Q?OKVMmejkGAlK5mwzXZ8Z41ZWQGnZxUjKN24FqeAhw5KQu4xG39gKxn27XhDf?=
 =?us-ascii?Q?bu4SLxto8diEpfq4v6QW4SGZxUzwNedSMtUP3CAQ0Vpj8q0EatzzHZcTsNuL?=
 =?us-ascii?Q?SKH8RR//3OPK9HMxLb3NY3jGzQ7iGCCuuCTG+P2vloMFHnsFfJnH43qM4mr0?=
 =?us-ascii?Q?ckGAxDZqotHKJwMGk1qlM12t4P5S5r8qbFT5r2StiRF+9V90DYNDVly71Hbv?=
 =?us-ascii?Q?wjwBALDZXVLT8vu9I2SohHav4SOL43VrolTa1ufx?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 84b6ebdc-ada1-45c1-6a9e-08daa05df46f
X-MS-Exchange-CrossTenant-AuthSource: CY4PR1001MB2358.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 27 Sep 2022 07:57:47.7499
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: NoINvrWnXKQIFuM6P+7CbNNT5wMY7Txy4C3gLlsDnntjsHvRkatHbhVex1mUr5WW9dfx3JkLoL4/bI8WedlfyPNQVKi3tlsVPHVV8Z0GPSQ=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR10MB6154
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.205,Aquarius:18.0.895,Hydra:6.0.528,FMLib:17.11.122.1
 definitions=2022-09-27_02,2022-09-22_02,2022-06-22_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxlogscore=852 malwarescore=0
 suspectscore=0 adultscore=0 mlxscore=0 spamscore=0 bulkscore=0
 phishscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2209130000 definitions=main-2209270046
X-Proofpoint-GUID: xQcscyn2HE-emQ3aTYKpUOEDLQk6B0wL
X-Proofpoint-ORIG-GUID: xQcscyn2HE-emQ3aTYKpUOEDLQk6B0wL
X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Mon, Sep 26, 2022 at 03:06:05PM +0800, xkernel.wang@foxmail.com wrote:
> diff --git a/drivers/staging/rtl8712/rtl871x_xmit.c b/drivers/staging/rtl8712/rtl871x_xmit.c
> index 090345b..dcf3f76 100644
> --- a/drivers/staging/rtl8712/rtl871x_xmit.c
> +++ b/drivers/staging/rtl8712/rtl871x_xmit.c
> @@ -117,11 +117,8 @@ int _r8712_init_xmit_priv(struct xmit_priv *pxmitpriv,
>  	_init_queue(&pxmitpriv->pending_xmitbuf_queue);
>  	pxmitpriv->pallocated_xmitbuf =
>  		kmalloc(NR_XMITBUFF * sizeof(struct xmit_buf) + 4, GFP_ATOMIC);
> -	if (!pxmitpriv->pallocated_xmitbuf) {
> -		kfree(pxmitpriv->pallocated_frame_buf);
> -		pxmitpriv->pallocated_frame_buf = NULL;
> -		return -ENOMEM;
> -	}
> +	if (!pxmitpriv->pallocated_xmitbuf)
> +		goto free_frame_buf;
>  	pxmitpriv->pxmitbuf = pxmitpriv->pallocated_xmitbuf + 4 -
>  			      ((addr_t)(pxmitpriv->pallocated_xmitbuf) & 3);
>  	pxmitbuf = (struct xmit_buf *)pxmitpriv->pxmitbuf;
> @@ -130,12 +127,14 @@ int _r8712_init_xmit_priv(struct xmit_priv *pxmitpriv,
>  		pxmitbuf->pallocated_buf =
>  			kmalloc(MAX_XMITBUF_SZ + XMITBUF_ALIGN_SZ, GFP_ATOMIC);
>  		if (!pxmitbuf->pallocated_buf)
> -			return -ENOMEM;
> +			goto free_xmitbuf;
>  		pxmitbuf->pbuf = pxmitbuf->pallocated_buf + XMITBUF_ALIGN_SZ -
>  				 ((addr_t) (pxmitbuf->pallocated_buf) &
>  				 (XMITBUF_ALIGN_SZ - 1));
> -		if (r8712_xmit_resource_alloc(padapter, pxmitbuf))
> -			return -ENOMEM;
> +		if (r8712_xmit_resource_alloc(padapter, pxmitbuf)) {
> +			kfree(pxmitbuf->pallocated_buf);
> +			goto free_xmitbuf;
> +		}
>  		list_add_tail(&pxmitbuf->list,
>  				 &(pxmitpriv->free_xmitbuf_queue.queue));


pxmitbuf points to somewhere in the middle of pxmitpriv->pallocated_xmitbuf.
We add it to the list here.

>  		pxmitbuf++;
> @@ -146,6 +145,18 @@ int _r8712_init_xmit_priv(struct xmit_priv *pxmitpriv,
>  	init_hwxmits(pxmitpriv->hwxmits, pxmitpriv->hwxmit_entry);
>  	tasklet_setup(&pxmitpriv->xmit_tasklet, r8712_xmit_bh);
>  	return 0;
> +
> +free_xmitbuf:
> +	pxmitbuf = (struct xmit_buf *)pxmitpriv->pxmitbuf;
> +	while (i--) {
> +		r8712_xmit_resource_free(padapter, pxmitbuf);
> +		kfree(pxmitbuf->pallocated_buf);
> +		pxmitbuf++;
> +	}
> +	kfree(pxmitpriv->pallocated_xmitbuf);

But then we free pxmitpriv->pallocated_xmitbuf here but it the memory
is still on the list.  So that means there will be a use after free
eventually.

regards,
dan carpenter