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:   Thu, 31 Mar 2022 18:11:52 +0300
From:   Dan Carpenter <dan.carpenter@oracle.com>
To:     James Bottomley <jejb@linux.ibm.com>
Cc:     Wenchao Hao <haowenchao@huawei.com>,
        syzkaller-bugs@googlegroups.com, fmdefrancesco@gmail.com,
        axboe@kernel.dk, linux-block@vger.kernel.org,
        linux-kernel@vger.kernel.org, linux-scsi@vger.kernel.org,
        martin.petersen@oracle.com,
        syzbot+f08c77040fa163a75a46@syzkaller.appspotmail.com,
        linfeilong@huawei.com
Subject: Re: [PATCH] scsi: sd: call device_del() if device_add_disk() fails
Message-ID: <20220331151152.GH12805@kadam>
References: <20220329154948.10350-1-fmdefrancesco@gmail.com>
 <20220331152622.616534-1-haowenchao@huawei.com>
 <20220331054156.GI3293@kadam>
 <fdebdbd3-575b-b30e-d37f-dcc6d53a4f53@huawei.com>
 <20220331134210.GF12805@kadam>
 <574d33978be5eb4732cd3bf61727e6b509a7e484.camel@linux.ibm.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <574d33978be5eb4732cd3bf61727e6b509a7e484.camel@linux.ibm.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?w1cdp3XuU2M7xq5qaTnpxocJsvwYi4eGdMmfnmkTpFsCElZCy0VZl70i3rP+?=
 =?us-ascii?Q?1Hj6Fc3JiObClSU0uETVI8pTRGaUJHQNngiD4Rqf6Rtl7p0KO9NK50aS4Q/h?=
 =?us-ascii?Q?dQatJh2UKmFhV9LLTRWlolOADLfIivpV+q97zbTdqaILgceHXISIt3XLryoA?=
 =?us-ascii?Q?Bf6jYhOg/BYBK2HswBOGCzNKMqzxrW8lvpKAjwwBziyoUSEW0jGxw/vTThaM?=
 =?us-ascii?Q?OlK0m5exs4zllUJY+a2ZvO1wNhdJUR+D69R7/+7JSzjp24zcHkoBhe2rWiq6?=
 =?us-ascii?Q?F6A3gTrSmDSfb1SHdFIXMCDbZc++VwIohWQJAbE6gRj0LGCQMFWRvlRycVaJ?=
 =?us-ascii?Q?FiafrB8HfsmXwhTveVZO5M7uFMEPVlR1Rhuw3cae3psOwILOkLVHpmrYPJF1?=
 =?us-ascii?Q?aM1pPUs6HQ5cIPO/SkTOCrHyPQPTWdOp7J/WWtd9+tDUuslN4mfdkB+f0R03?=
 =?us-ascii?Q?ZT3XZ7NW5G8KZaqgQIlql2axL8juZ4Iq8zg+lboB/lA9nSWUE+0DetJceJKq?=
 =?us-ascii?Q?d9x77R8mxE9DOlZ6YnQR1KoWmWhd4MIgwBvnFGS2ToDC14ru2PWGltk/w1LB?=
 =?us-ascii?Q?oYEN3HgpsE3oKv/S2jYmycAHwypWKd/+NBX1UDe3dU0WGMR/0EhYudEgj78K?=
 =?us-ascii?Q?UNKmCwbBLqZncYZ3dGI5nk+rFOwV+LiCqpGIt4im7tGdnG9WT0Vltsww3No3?=
 =?us-ascii?Q?oqQB6HDVsv6Wqr/TMqUmjsumJJ+3VuRllAVCvZnihZDTKRrXWHtpTW5pMpis?=
 =?us-ascii?Q?V+XBD2buK5taPT7FKvjUYZ1Wo8stkcZm6EycfIZRbUxRLM6nvnuxNBGke8CR?=
 =?us-ascii?Q?TLlW3I+rZcrq0xzet9fawTGZotWm4Lp4GAkonwn/RnQUwoKPh6cQXr0arHDX?=
 =?us-ascii?Q?8CAUoPwwc99TaZHXI5VO1IVXYRzIffZY+5wqNuhsz2EQR109VK3EO1yIV1R7?=
 =?us-ascii?Q?fZM2/5hdw8B/lxlH31b2RfPM0VyIn6tp6DfRLYMC+aBPvtdj79zLuJK/wggw?=
 =?us-ascii?Q?UkG2jJjvdZ6xyioQ3lFgVAG3cg2o4XuJOk8dCC8A/JLO+1n3T584a9xSzrou?=
 =?us-ascii?Q?3krbnhGFdLTjX/Jl8shq1+zvoneyLF4X2hdBE58tA5Rgr5dBAZ8AwDYe2Xyq?=
 =?us-ascii?Q?5JTvnfSL65GYVCe35XKEDeihwN/vvslMSgdrhjHvnrq6IEV9Skhy47n9Zqr1?=
 =?us-ascii?Q?xIiS3F7IKYs8ewhXrdTqATJw7gm0HCiWyaGvXE23QiaGHxCsSksqBqhiC/Fg?=
 =?us-ascii?Q?P9LbQdTXpOv083CUX3N8uyI/K5WxnhooWlUY/YKPQGnJE8IG813eeI7g7aZI?=
 =?us-ascii?Q?v0/Hk6s8XporUsVFoSb6lFDyfFTBJe0bcsy+M/LdjFdmcUYQuuBpeHJdfufZ?=
 =?us-ascii?Q?dVsjwwSWs+SW217+tWwXqnwOnciSgfEswtqmn7JbvTfQl0oeTuyk90FA9XjW?=
 =?us-ascii?Q?pubmm5epuKJz8P7eOMykiIJ7bnGwqP5MYbeVV/568qODp+nmneKs3wDD9LM1?=
 =?us-ascii?Q?qdPcKC2IpJ6nBHVI/MinHDDld41l7Lnn3Ir2UXfemb5iWewU3XWG44m+VT0q?=
 =?us-ascii?Q?brnHTLPYMDtmbHwQ0cCJbc88shyH0xCjFIqcg5Z5ZOPYf5gbLHTQAQWqhPmo?=
 =?us-ascii?Q?wP5bYL4LNxmU0cW0ie4UXNUKABC+m535qYJ0pKzyLvQHBsYDyO4SlHnDmZOk?=
 =?us-ascii?Q?J7gRDuH3aEqWInyu2jECmmKbCaPFT2bUYxXjWqI/FDM5XKYXSBlNseEHjReb?=
 =?us-ascii?Q?DCIDvNmOyFtXmX87q3r+rWBhzHFQVr4=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: ac741c05-8ea8-4fea-7296-08da1328d6d6
X-MS-Exchange-CrossTenant-AuthSource: MWHPR1001MB2365.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 31 Mar 2022 15:12:15.5812
 (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: wHbjYJUal2zJ+gFkgzgM+yoyOV1Kw5UkQqqn2rLU4kBZ5DsZsXcRE3AuyTXlBBvYz9S7EhL9m0+WolTL7OxK/uU1rUxOvXglsq0nk3MBHMk=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM5PR1001MB2218
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.425,18.0.850
 definitions=2022-03-31_05:2022-03-30,2022-03-31 signatures=0
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxlogscore=633 suspectscore=0
 spamscore=0 mlxscore=0 phishscore=0 malwarescore=0 bulkscore=0
 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2202240000 definitions=main-2203310085
X-Proofpoint-ORIG-GUID: sttJ3hF5Y9saA45qPnzJ-ltQNNBn9I3T
X-Proofpoint-GUID: sttJ3hF5Y9saA45qPnzJ-ltQNNBn9I3T
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_H5,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,
        T_SCC_BODY_TEXT_LINE 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 Thu, Mar 31, 2022 at 10:19:57AM -0400, James Bottomley wrote:
> On Thu, 2022-03-31 at 16:42 +0300, Dan Carpenter wrote:
> [...]
> > Also, I don't really understand why we don't have to call
> > put_device(&sdkp->disk_dev) at the end of sd_remove().
> 
> That's because the final put is done by the gendisk ->free_disk()
> function which is scsi_disk_free_disk().  Most of the gendisk functions
> we provide convert a gendisk to a scsi_disk (via the gendisk
> private_data), so the sdkp has to live as long as the gendisk.
> 
> James

Thanks James.

Ah...  And scsi_disk_free_disk() will not be called unless
device_add_disk() succeeds.  So Wenchao Hao's patch is correct.

And after that there is just a missing device_del() and also I see
another problem where if device_add() fails then we need to call
put_disk(gd); on that error path.

regards,
dan carpenter