Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.31 as permitted sender) client-ip=23.128.96.31;
Message-ID: <48a51a78-da87-58f3-92b2-01db85e14cfb@intel.com>
Date:   Thu, 7 Dec 2023 10:55:03 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.3.1
Subject: Re: [PATCH] [v2] qed: Fix a potential use-after-free in
 qed_cxt_tables_alloc
Content-Language: en-US
To:     Dinghao Liu <dinghao.liu@zju.edu.cn>
CC:     Ariel Elior <aelior@marvell.com>,
        Manish Chopra <manishc@marvell.com>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        "Yuval Mintz" <Yuval.Mintz@qlogic.com>, <netdev@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>
References: <20231207093606.17868-1-dinghao.liu@zju.edu.cn>
From:   Przemek Kitszel <przemyslaw.kitszel@intel.com>
In-Reply-To: <20231207093606.17868-1-dinghao.liu@zju.edu.cn>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?VDkwcHJ5QkJmMHBid3J0WGV0Yk1mQ3lTMmdTR3BHY2VBbldHcW5UR0V4Y0JF?=
 =?utf-8?B?UWJTV2JPblFjVGpRSWdWcDNWbFZqVDJhYnQ5SWU0MEdBTEx1YkJoYmlTQTND?=
 =?utf-8?B?a2lNb2RKWDdZdnMvSHh3S0ZPaDlBSUpWcDlIeWIxbkRTb0FlSnFyNldmdVMw?=
 =?utf-8?B?WU1zNXdmTWt4TCt3QWx2eWhWUkVOSFdoUkY5ZGlMUmU4OVlpODhiYy91aENC?=
 =?utf-8?B?SUFDcHZIWmxPZXk5WUxQazJmRWtSdVVmeEVjd1grWlFlNHh1ZlhWdWpJRUk5?=
 =?utf-8?B?bmEvTldtSmg5Wks1VzZKTWUzb2dnTUJSdy8wSW8wLzJuSzdCUC9SNW9kV3N2?=
 =?utf-8?B?U1NNVWZyVHJvUVBRREtVU0kyVHlMSFp5MUNjK0FndkVFSC9nT1phdmNIdjdN?=
 =?utf-8?B?SVRXSkdYbzJTbUZUUzk3VUZRT2dCUkdoTjc0NER1SGM2UFdNWVk3enBWdnV6?=
 =?utf-8?B?SmdHNDkyeFhkTWNRdHN6cktJNDZ3SWJGanl4ejlXM3hRZUVwa3JEeW52cnl2?=
 =?utf-8?B?Q0pBR2xHajg2YVRPQ3hTYk9xWkpxS29RaVQxVkZ2UGZMTnMwdVV0OFRHNFI1?=
 =?utf-8?B?dU0vT1h1Z0doTW1VR1JGWWx2aGlhVW9vbnFXNnNwQXdLUW9hbVZCQXh2RHNQ?=
 =?utf-8?B?amRaVXBhMExOeWJpUUx5OTI3VUZkbms1V0Z4NDU2NEtHOGZpalBLRUxTWnRk?=
 =?utf-8?B?Lyt0OHltNTl0bjI1cUt2YTNHRUM3M3FpYU54U2Z2MXRLeXBvbFR5TEZDak1p?=
 =?utf-8?B?OHZpN3FHL2YrV3NLVUI0b0UvTmFMNVJDVElNbE10Q2JaVXBjMVhMWG42ck0z?=
 =?utf-8?B?QUdsWUJFbXlSajJ5aTNONzVLTzUwUDgwVHJLcGFnUFRRMk8yMnJOTFJUemdX?=
 =?utf-8?B?MFRYUDFZUzdQWHZoZzdaNkpQVnpjdVZVWWJTbnluOW52SjAwOVRURUFud3J5?=
 =?utf-8?B?UnBnZTZ5dm5SUEdIbGNOU041Yk82UWZtMXpQNkFWQzQ0TWNTTGJXYWxFcVJU?=
 =?utf-8?B?MXRkVHVxZVZMVWVWUk9QdWNydThpS2VtUExCdVVEK2pwR1M3WFhQZGJXNmd0?=
 =?utf-8?B?TUVkaEZZTVJUK2VRazYvRHdYWWtvMXYzOVlpdFd5MDByQVNoMkhyNHN3cklF?=
 =?utf-8?B?Z3FVVnRtUmdpQ3QrZVorMjlQdDU1eCtKWGIrZmtkd29NdVFlRUtyU1N5dnVQ?=
 =?utf-8?B?VEMydlZiWE1GSHQ4OHVrakNodGlhREtWclRZbGtiTWk3WjM3TmFaNm9XMlBq?=
 =?utf-8?B?QWd4QXcxcnBuZXFZYkVIVDgzQUZySWJ6Y0EzazRKVVE2U3VzaE5jK1kvWVoy?=
 =?utf-8?B?WDBlV1Jidm1TN0FiU3NOcklqRWFRK3lhVVV2OCt4WHdRMllQWThZRk1RS0gy?=
 =?utf-8?B?eDJqWll1M0Y3RGxGTFRxeDhLdVU4em5CdkMyZjJnNCtNQUZTL2J3VndLenNk?=
 =?utf-8?B?Mzl5NWRTZlBYWGptN0RyemUwa1lHUHFYaGZud1ladVZ6Q0hDUEp0Y090ajE4?=
 =?utf-8?B?NnJwY3hIQkg2WEpQbGwzSm1RMThjN2dVaE1ONGxQWnNjVzd5M1JlRVBiOENp?=
 =?utf-8?B?V2Fma3BZeDJ6dGp4T3NpaEk4SVh2WEtCQ0JJam5kbS9wWHBtSGFhZjhjTUMz?=
 =?utf-8?B?ejMzbGw1ZzZCakx2QmNBNFIxeGhrOGxGYXV1WS90NnBsVlNjdU5PUzUyNWdS?=
 =?utf-8?B?NjNzd3M1RkJ1RmdEZCsvbUFGMDV2bHlFZ2loYUlhb0hEK1h3NElKd1JEZGV6?=
 =?utf-8?B?dXV1LzBEditnOWlHbnlDVW1YcWlSYllKRW1MNE9lSkFHbWUxNC81RXRqYThk?=
 =?utf-8?B?ajdYUFZLaXczSFVYeDFtd3ZDZ3ZHUjIydTVrRjBDVzVrSlRWMU5FRmxuZDVZ?=
 =?utf-8?B?ay9YVzVSOEp0T01iczBzMVRKMUVxS1BOTjBLYkNYdU5Bc2ZtdU5OMDRxa0Ri?=
 =?utf-8?B?ZjhvUUkwaVdXOHJ6UTlZN2p2aWh3TXJiaEhUbDFtdENjQ3BaL3RaZnVWWVRO?=
 =?utf-8?B?aXhnbmpRSHNrNHRUanZBcWVUUHdrbEorTGVFTmNjRDA1YXg1aUR0Wm9KbkZ2?=
 =?utf-8?B?dEFFVlBNdDdGaHpmRGh3aUNFVGNBMzl2Z1VjTkZTMzFwRDVqYlYzR3BmL0FN?=
 =?utf-8?B?c0w4dEtqd3lFcmdzNHdES3lMT3pJUTNMSllCcU1uL2ZBUG5kaERVaytVYjlR?=
 =?utf-8?B?UVE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: a769edb1-5c98-4e4b-3383-08dbf70a9a5b
X-MS-Exchange-CrossTenant-AuthSource: BYAPR11MB3672.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 07 Dec 2023 09:55:11.6817
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: Vj8jK0AomGtC9J99wnA5JU1kNPjYROsgK5cQktDw2W92I7D7qO/BTINg5ffSPLCbzdSBm0niAj44EC70U+Z3P1v226zB4jLQqNnYhU9wCbk=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH0PR11MB5522
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-3.1 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,
	T_SCC_BODY_TEXT_LINE autolearn=unavailable autolearn_force=no
	version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on morse.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (morse.vger.email [0.0.0.0]); Thu, 07 Dec 2023 01:55:22 -0800 (PST)

On 12/7/23 10:36, Dinghao Liu wrote:
> qed_ilt_shadow_alloc() will call qed_ilt_shadow_free() to
> free p_hwfn->p_cxt_mngr->ilt_shadow on error. However,
> qed_cxt_tables_alloc() accesses the freed pointer on failure
> of qed_ilt_shadow_alloc() through calling qed_cxt_mngr_free(),
> which may lead to use-after-free. Fix this issue by setting
> p_hwfn->p_cxt_mngr->ilt_shadow to NULL in qed_ilt_shadow_free().
> 
> Fixes: fe56b9e6a8d9 ("qed: Add module with basic common support")
> Signed-off-by: Dinghao Liu <dinghao.liu@zju.edu.cn>
> ---
> 
> Changelog:

For future submissions please also provide links to previous versions
(would be to v1 in this case). No need to add this now for this one.

> 
> v2: -Change the bug type from double-free to use-after-free.
>      -Move the null check against p_mngr->ilt_shadow to the beginning
>       of the function qed_ilt_shadow_free().
>      -When kcalloc() fails in qed_ilt_shadow_alloc(), just return
>       because there is nothing to free.
> ---
>   drivers/net/ethernet/qlogic/qed/qed_cxt.c | 12 +++++++-----
>   1 file changed, 7 insertions(+), 5 deletions(-)
> 


Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>