Date:   Fri, 10 Mar 2023 14:58:52 +0100
From:   Simon Horman <simon.horman@corigine.com>
To:     Zheng Wang <zyytlz.wz@163.com>
Cc:     davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
        pabeni@redhat.com, petrm@nvidia.com, thomas.lendacky@amd.com,
        wsa+renesas@sang-engineering.com, leon@kernel.org,
        shayagr@amazon.com, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org, hackerzheng666@gmail.com,
        1395428693sheep@gmail.com, alex000young@gmail.com
Subject: Re: [PATCH net] xirc2ps_cs: Fix use after free bug in xirc2ps_detach
Message-ID: <ZAs3nOoT0GmsLfGN@corigine.com>
References: <20230309165729.164440-1-zyytlz.wz@163.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230309165729.164440-1-zyytlz.wz@163.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?YUqRRU6F8Pd2QiTlF5s9C15emJe0+gwlfW8HprauxyHpl+6YO2zORND1lS0v?=
 =?us-ascii?Q?WKRhdUX4inoTxBo5wCCb4POY7PezzW6La+MavDE7OeZJlcWrpFE73RPINqsD?=
 =?us-ascii?Q?kYcOIy5h7sK0GrFbLSmjkeRwDs6G+5ew/6C8aPSj3d3vA+n2CJJ9YS2MVTpZ?=
 =?us-ascii?Q?NAeeUHhw3Ef9T1+gLhJKgqjx1iA/eAw7NXMQEGz4LYdW6RFWkh5kmX6+Dc1a?=
 =?us-ascii?Q?NCu1jUaPGVk3WEYtUbfGCUo1CmUNKVX0CAVPIkYeTsg3pZiuOLlM1jcxJBpV?=
 =?us-ascii?Q?ET9qcWkBuwPFTuLLwvXnmQYJTI4gDmFPT0o4vX9Cmw/EFxWVg37gNgw1+egw?=
 =?us-ascii?Q?FuK0Rodzd+u2wvFpv6Gs+h25WmAIy+i8yImjCEwV76ocMxUKiARqHj82AZlQ?=
 =?us-ascii?Q?8AzDXiqgreZUzGFfmbj7T1iBY/aziEs+0UezPwAglVECDrQj2k2pPiOpkmrn?=
 =?us-ascii?Q?zIQBojyYF4VKmvjYMKQKkMAqmKc2UAYcoUmKx46j52Elnus2YuLq90kzvxNq?=
 =?us-ascii?Q?Bgu6gMCgX7oXNtNJAMMIj6s1LeMnC6pvEFF1RihyQXFjgED8xNa/FhjF+ZuH?=
 =?us-ascii?Q?oN32kN/e84qZ2ey796/07ILP8hoAC36Vrw8+lerfZTc12TOBuq5Muy81e4Dd?=
 =?us-ascii?Q?r5gQtN4dbgomoR3gDeIZtzXHzMOyDyHryNPT6ZjcJRdXsIWj7VHnIGabSzPn?=
 =?us-ascii?Q?ie6aTMO2NPS8seXvwvWtaBFi6DfQWcqu1O1GEQb0ny9kLtPXcjhdHltyEc/M?=
 =?us-ascii?Q?jm9ml0K9FGrUIEkUHikUDumyNiPgn1fYoDgzMOGSZJYOE3DlvZ03Sc7PPK9h?=
 =?us-ascii?Q?7stsHUatFMQl2ouYb7P1ecnhQt0Buj+vG50t8Li+QQLJeosaClOie40cyRPh?=
 =?us-ascii?Q?Orr9M0Zu48V9I8jBpI4qeBSRbYhwJBsVO+oxjLQ6c9SoCimMqPdeq0UbHhIB?=
 =?us-ascii?Q?QsUZpH7KmIjK+qswyIw/8XmtJzpCr010oAS3398TLh2857yifC2AzrrFI2Kj?=
 =?us-ascii?Q?4Ogh9uQsoRpADBLDRfJTGQT+sRfRz3cEQDWizbnC7JHK+JLVCLgeimV/JYH7?=
 =?us-ascii?Q?EOJT+mctkX25cX/YixOlwt00OAwWihBPsRzW//WvpRbtDk7duNB5ZEZ6xPpG?=
 =?us-ascii?Q?fu0gw/vdsJTbeoDaNpJKSsiTjgEyK6slvgtgp4YdavjZImjchCySgkPzoSBX?=
 =?us-ascii?Q?B/FEeayGdw6Nb318Z/AezsqZ2yjiDg5NW+SnbQ1twmHG977NFrkUd2lncVvj?=
 =?us-ascii?Q?HE4iKerzL1PXdbgyev1JiKxe6BN2QSPkH/N4lNmnZgWvmgBocW2tatP0B4Rg?=
 =?us-ascii?Q?PuBAhivvoToymillMmYyT1fZ6V+QjC1bl4CumQEcJr15rBEUWSVYbkdsVrPa?=
 =?us-ascii?Q?MAYpa5trM+WAs1zqb4dHf9DOqG6kiqcJCT+freBQMU1bNRn1xM+7aX4H0WII?=
 =?us-ascii?Q?pV1RgGYdSm38h64EVcOngIECaVRgD1OS+gOADbKOO2zsZDjkUXgma0rlek/r?=
 =?us-ascii?Q?8JNI5ZL67CKsbSsGTVjR45LJDMp6tFy51cxkraU+l1w8zrCq60X7JB/tKALN?=
 =?us-ascii?Q?cLSa/nUTWxYF92JUfDpmbMDkJJdFFJd9oZ+7cvlr8vEfeNag8WDZv45jLs+O?=
 =?us-ascii?Q?rkApK/eKAB82SyFa2ys4K5T9cVDUuNdtqP4R6o2afluXq4xtTllqXidDIOtM?=
 =?us-ascii?Q?mjtD9A=3D=3D?=
X-OriginatorOrg: corigine.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 90e90db1-9ed4-461d-ac07-08db216f98af
X-MS-Exchange-CrossTenant-AuthSource: PH0PR13MB4842.namprd13.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 10 Mar 2023 13:58:59.1667
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: fe128f2c-073b-4c20-818e-7246a585940c
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: +rBD0YJ51L7/fYIgoWjIYNqsQMqpWAZSHMHSiregppdk5aKb9VOZaOnWxu+ZeJzFyb8EbfI0MAFOMJq0I266m2ZMioBtE0Rp8FhY8/c7ALQ=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW5PR13MB5855
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Fri, Mar 10, 2023 at 12:57:29AM +0800, Zheng Wang wrote:
> In xirc2ps_probe, the local->tx_timeout_task was bounded
> with xirc2ps_tx_timeout_task. When timeout occurs,
> it will call xirc_tx_timeout->schedule_work to start the
> work.
> 
> When we call xirc2ps_detach to remove the driver, there
> may be a sequence as follows:
> 
> Fix it by finishing the work before cleanup in xirc2ps_detach
> 
> CPU0                  CPU1
> 
>                     |xirc2ps_tx_timeout_task
> xirc2ps_detach      |
>   free_netdev       |
>     kfree(dev);     |
>                     |
>                     | do_reset
>                     |   //use
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Zheng Wang <zyytlz.wz@163.com>
> ---
>  drivers/net/ethernet/xircom/xirc2ps_cs.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/xircom/xirc2ps_cs.c b/drivers/net/ethernet/xircom/xirc2ps_cs.c
> index 894e92ef415b..ea7b06f75691 100644
> --- a/drivers/net/ethernet/xircom/xirc2ps_cs.c
> +++ b/drivers/net/ethernet/xircom/xirc2ps_cs.c
> @@ -503,7 +503,10 @@ static void
>  xirc2ps_detach(struct pcmcia_device *link)
>  {
>      struct net_device *dev = link->priv;
> -
> +		struct local_info *local;
> +
> +		local = netdev_priv(dev);
> +		cancel_work_sync(&local->tx_timeout_task)
>      dev_dbg(&link->dev, "detach\n");
>  
>      unregister_netdev(dev);

This doesn't compile.
Also, the indentation is incorrect.

I think what should have been posted is:

diff --git a/drivers/net/ethernet/xircom/xirc2ps_cs.c b/drivers/net/ethernet/xircom/xirc2ps_cs.c
index 894e92ef415b..b607fea486ab 100644
--- a/drivers/net/ethernet/xircom/xirc2ps_cs.c
+++ b/drivers/net/ethernet/xircom/xirc2ps_cs.c
@@ -503,7 +503,10 @@ static void
 xirc2ps_detach(struct pcmcia_device *link)
 {
     struct net_device *dev = link->priv;
+    struct local_info *local;
 
+    local = netdev_priv(dev);
+    cancel_work_sync(&local->tx_timeout_task);
     dev_dbg(&link->dev, "detach\n");
 
     unregister_netdev(dev);