Date:   Wed, 1 Dec 2021 12:56:02 +0300
From:   Dan Carpenter <dan.carpenter@oracle.com>
To:     Yang Yingliang <yangyingliang@huawei.com>
Cc:     linux-kernel@vger.kernel.org, linux-staging@lists.linux.dev,
        gregkh@linuxfoundation.org, paskripkin@gmail.com
Subject: Re: [PATCH -next 2/3] staging: rtl8192e: rtllib_module: fix error
 handle case in alloc_rtllib()
Message-ID: <20211201095602.GB18178@kadam>
References: <20211201095036.1763163-1-yangyingliang@huawei.com>
 <20211201095036.1763163-3-yangyingliang@huawei.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20211201095036.1763163-3-yangyingliang@huawei.com>
User-Agent: Mutt/1.9.4 (2018-02-28)
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?z9LWK5sjS1NSIPKvho+/pBMDMeMEDPGjHfoesB15Hv5ygC7+IwLnlbgAzM72?=
 =?us-ascii?Q?yuAnkxKErcoIK1jLj5O9YxV7Ti4Qlpt43FopW8oziYHBpR6m10iFcY3TPmus?=
 =?us-ascii?Q?4yy7SVAjuCbTgW7HzHYzJg15YBm14fOW+wNEYFBSSfTfVc/p+hTKdDoTrRZF?=
 =?us-ascii?Q?uoueLVOpxoRRAvsRAkFUA8mtMEmgqAaaNLg7dCS64/1k3u/qo+i6RKtmbFba?=
 =?us-ascii?Q?rnDPujl+2bF002aMGR7oqrT5aKfPbqwZEGnYYeG3PD9XCfDGT19+8LUnVbsW?=
 =?us-ascii?Q?VNEBpSQo2umGs9ovQSlWku4jv6LVeFMhWRsKBvQOJcFckOxLlUocDbkpX0bQ?=
 =?us-ascii?Q?uyKpWlRhdtCFMGwmDONGhvhACEgOhyOe6jCAmyIEnMMOCze66iTNl6cn1aEC?=
 =?us-ascii?Q?eKmIJbzfzUJXpdzWF+RqllCj+CjzRDztydqQej9PN/4nLkyhdP0lu00g6MX9?=
 =?us-ascii?Q?q0LWPx1M19Q5MM8ioJ3NCthRLlMKhmayvOzJJvtW//z6hYbHkw4XzZfHiNk2?=
 =?us-ascii?Q?uDQHegR/CzxmSB5O7EUpO88rW7CW9WFQ7qKivhC4Qrt/Gm1EoJddMMqOmEIr?=
 =?us-ascii?Q?63w+YvchmQGvKVES+OEopXNYYZrIe2uggX0Buw3oMhtc+29UUoX9w+3bIh1s?=
 =?us-ascii?Q?AB1Io1WsolojV1o5MQ8FoGl+Xjx0rWbhctxkX9sIPS2By4jsMwmrkuQI0Oq/?=
 =?us-ascii?Q?S5G2QQ48nYKZ6LyzHFL3f45CKKrnLIfTZZIP+WPVv6RdqQPI1BUiml9aom2X?=
 =?us-ascii?Q?02Dk2Unyfsdhlc0QqBxj1yHq4mZjw8zKiCB8Z3GREF6l8l1X9zLGPkchCdPP?=
 =?us-ascii?Q?OivaBYs+rgo+9tx/huF8QtW4454bbasorVsshBzd5X2mRY3p+gtJtWdURYFx?=
 =?us-ascii?Q?9W9HvPiaOTOj1JlMMzR9+XFX4/oI3CshM8+t5rt58z5N6KkNb9wsljP4F43I?=
 =?us-ascii?Q?+K2e2Nh1xZHzCfEWCp2ud8pilVEKXbMJuSsupMJrSE2bSmw7tkobcbmsjzwo?=
 =?us-ascii?Q?5Uqo7oEOnqmMkZhJDWwu+erbZ2iIEZcA/vlBKmw5MZjYZJeWITmRJaDzC1EA?=
 =?us-ascii?Q?Ev89J7cbJ7B9e5b4WqKjKEE6VOro9Gtifm6YP8xgs2vNq9amZzlpB3vzb4ua?=
 =?us-ascii?Q?8JLDiEUjzcnH8eaCHM/EySwgqt1fI4eN7CFyfassvngs+ALXx8gIgq2q2X9h?=
 =?us-ascii?Q?MGOFj7U2BwRAJ8kVwKgX2xqRj1f6rT9xcRcJlM1+1AkKlPHgGqOMiTomiWnW?=
 =?us-ascii?Q?IkW6J5dg4XIhMunAPqT6xIVuhK8x1sGsLXZEJEdVpeQwqCwtxfU+US+bLW2V?=
 =?us-ascii?Q?B8WLDevYvyI1BolYLUqa5+lOaW59n21YBunYGfyQM6lgJq+p33XrH6rqj9ul?=
 =?us-ascii?Q?Qna4T5pN4IWJdCOVSdhatolAbqWgDnpQeEH81h72fcXO1Y3MwQutXXkOqgIW?=
 =?us-ascii?Q?I07RKtGYUIRDK9MUxoUsPqPdeV+uslbBLvR9vMGKyEeb6hCbruDvXGThXLTg?=
 =?us-ascii?Q?x47pfcBWDJjzLm9say5VHlrXnzS1QPiXg4sGEdTOuwh842HAMA/n7SHvN8FI?=
 =?us-ascii?Q?AfDoICOzmQ4dsLwI6poZHDomDI9FJxrTM+Vl8SK19+CQ2D7NwCA568UhxO8f?=
 =?us-ascii?Q?oWbGH7K5kZvEbxQGgcWMqeHh4ugqixg8lm18QRCcqn3TX+GTM+FZz87EDnHD?=
 =?us-ascii?Q?tzjISiWcnuGew+aiZWcocuqVO4U=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 927f52c5-d65b-4d1e-bcb4-08d9b4b0d449
X-MS-Exchange-CrossTenant-AuthSource: MWHPR1001MB2365.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 01 Dec 2021 09:56:22.0360
 (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: OUilnlOblwqbTxJU1SuH76tlmiWCdd0kNuEvs1LecEC9VoRQHENjYR/ri5IK1HZ0yXGo5wVuvbvNIy/tjQcOfTQtg8xDTSoNRF6bCFNYET0=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MWHPR10MB1773
X-Proofpoint-Virus-Version: vendor=nai engine=6300 definitions=10184 signatures=668683
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 malwarescore=0 mlxlogscore=999
 phishscore=0 suspectscore=0 spamscore=0 adultscore=0 mlxscore=0
 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2110150000 definitions=main-2112010057
X-Proofpoint-GUID: bDhFDVmUvZJe8iQ6oOwhO019NxSwFTZu
X-Proofpoint-ORIG-GUID: bDhFDVmUvZJe8iQ6oOwhO019NxSwFTZu
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Almost perfect, but it needs one minor change.

On Wed, Dec 01, 2021 at 05:50:35PM +0800, Yang Yingliang wrote:
> Some variables are leaked in the error handling in alloc_rtllib(), free
> the variables in the error path.
> 
> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
> ---

Please send this as a v4 patch with a little note here:

v4: Fix crypt_info leak
v3: Fix more leaks.  Break it up into multple patches.
v2: Make rtllib_softmac_init() return error codes.

You can probably put that in the 0/3 email.


>  drivers/staging/rtl8192e/rtllib_module.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/rtl8192e/rtllib_module.c b/drivers/staging/rtl8192e/rtllib_module.c
> index 64d9feee1f39..a3c74fa25cfa 100644
> --- a/drivers/staging/rtl8192e/rtllib_module.c
> +++ b/drivers/staging/rtl8192e/rtllib_module.c
> @@ -88,7 +88,7 @@ struct net_device *alloc_rtllib(int sizeof_priv)
>  	err = rtllib_networks_allocate(ieee);
>  	if (err) {
>  		pr_err("Unable to allocate beacon storage: %d\n", err);
> -		goto failed;
> +		goto free_netdev;
>  	}
>  	rtllib_networks_initialize(ieee);
>  
> @@ -121,11 +121,13 @@ struct net_device *alloc_rtllib(int sizeof_priv)
>  	ieee->hwsec_active = 0;
>  
>  	memset(ieee->swcamtable, 0, sizeof(struct sw_cam_table) * 32);
> -	rtllib_softmac_init(ieee);
> +	err = rtllib_softmac_init(ieee);
> +	if (err)
> +		goto free_networks;

This needs to free crypt_info;  This was my mistake in the email I sent
earlier.  Sorry!

>  
>  	ieee->pHTInfo = kzalloc(sizeof(struct rt_hi_throughput), GFP_KERNEL);
>  	if (!ieee->pHTInfo)
> -		return NULL;
> +		goto free_softmac;
>  
>  	HTUpdateDefaultSetting(ieee);
>  	HTInitializeHTInfo(ieee);
> @@ -141,8 +143,14 @@ struct net_device *alloc_rtllib(int sizeof_priv)
>  
>  	return dev;
>  
> - failed:
> +free_softmac:
> +	rtllib_softmac_free(ieee);
> +	lib80211_crypt_info_free(&ieee->crypt_info);
> +free_networks:
> +	rtllib_networks_free(ieee);
> +free_netdev:
>  	free_netdev(dev);
> +
>  	return NULL;

Something like:

free_softmac:
	rtllib_softmac_free(ieee);
free_crypt_info:
	lib80211_crypt_info_free(&ieee->crypt_info);
	rtllib_networks_free(ieee);
free_netdev:
 	free_netdev(dev);

regards,
dan carpenter