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;
Message-ID: <833d4fff-7fb9-31c5-3ed8-ed9a7753a5db@oracle.com>
Date:   Tue, 18 Oct 2022 11:47:50 -0500
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.3.3
Subject: Re: [PATCH] scsi: fix crash in scsi_remove_host after alloc failure
To:     Khazhismel Kumykov <khazhy@chromium.org>,
        "James E.J. Bottomley" <jejb@linux.ibm.com>,
        "Martin K. Petersen" <martin.petersen@oracle.com>
Cc:     Gabriel Krisman Bertazi <krisman@collabora.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org,
        Khazhismel Kumykov <khazhy@google.com>
References: <20221017171147.3300575-1-khazhy@google.com>
Content-Language: en-US
From:   Mike Christie <michael.christie@oracle.com>
In-Reply-To: <20221017171147.3300575-1-khazhy@google.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?L3V2ZHk2dFlJVnJneEVtQ3FIQ1M3UEpYWlUwZ1dCVXZ6bnZEY255S0hTUjV6?=
 =?utf-8?B?RURaS3E0Rk5WUEJBQnFxZmZVak40azBxcVVZbEhIRG9CWU5rL3NuQ2N2dXBJ?=
 =?utf-8?B?dkZjdEVYSWNpTGdqc2JRTFJGRnFBaVh0OVhsbWJ5SDJzaHh0bmV4V2V5bEEz?=
 =?utf-8?B?Qi9mZk9kVlBHL0xTT1JyM0N4YmNGcmRTUHhFRE14RUwwY2dHNTFhdDZGMTdI?=
 =?utf-8?B?TW1tUG9sOERLR05nUk9yeUIwZjQwbnk4aDN1dW01RmFubHF0MVY4T1JjbEls?=
 =?utf-8?B?MXcwdjYvdk9mQXlTVy9NZXhpYlNieUtybkJpcUdwZ2hLTk1sNk41U1FGR3Iy?=
 =?utf-8?B?SDJ1ejlNalY4NGxva0xZZENtc2w4ZStIR05sRGNBemMxMC9kcTVmaXEreFI1?=
 =?utf-8?B?b3dKcWRSeDNURWl6K25TV0xqV2ZoZHFTeForNnovM2xvRFlIdlBWZlhocmRP?=
 =?utf-8?B?V1hmZG1CbE9YVmZGSVNseVFpMXZCYUNjczk3am40Tm1GN09wdkVObkxOZDFo?=
 =?utf-8?B?dUJaRWY2MitRYTQ4ZW5Xa05wSkN6amJCTW9aNkhzUlQ3dEJnaEt0blV1RVAx?=
 =?utf-8?B?ZUdEbnVob09vaEVhR3hFem1keWVQMVZObndaOExpNmp3RWpNbTk1NXYyWkxT?=
 =?utf-8?B?UkMxWkMyTmdLdW93QUhPdnJjaDh1dHhoUW1WL1dxSnBNemJDTkpiSzdiMUhK?=
 =?utf-8?B?Zjg2Zi9JR1JFdlc1NzNXV2lqV3BKSHhienVDUGpKZE1vVXBSYTFUMFlzWTJn?=
 =?utf-8?B?YXVVbUhVT1EzK0FiYVI2Y3FFK2NPQ292bXhucjFmYzBLd1BRNTYwaVYzTWEw?=
 =?utf-8?B?a1NTYi9INk5SZ1hPTG50a0NaYlllTVB4YXA2WUxvTEVQZUlEcFBZeTRrSXRS?=
 =?utf-8?B?R1pPRGpsdS9uSjBEUG5tYzlxNWF3aStUWDdwYmp4TFVCQjROT2VrSnN5eVUx?=
 =?utf-8?B?VzVOczR4TVdUemp0OEcvUEFSTWNhVmhzTGViU0xKU1d1VzFXRTBlZ0dGRUda?=
 =?utf-8?B?U29QY3hHVWxNUlRDZlRsYm1IK2pDUGNmM0J4cVZreXl6bHJ6RHBFQ0hWNVpa?=
 =?utf-8?B?ZjFqSWl3SDhZUk4xVG9aVFY2dnJpOUZscmpRUzdMVm8yeVMyU3JjWnpFeXFG?=
 =?utf-8?B?S1orMk1kR1VGMytQSzQyTDlEUUlaUnE0Q0hFMHdrckNiaWFIRlFZbDJYOElL?=
 =?utf-8?B?bHBVQWU1ejlKMTlEamVlYnJFcG4xNVVVYjBNcExHOXFJVU9INi9Vb21yMmx4?=
 =?utf-8?B?eHBQYTl6a2lyTzFyT25lblQrUGpVRHRFbHg1Q0wxbTFobWhVUWZhVGtwSWtx?=
 =?utf-8?B?d1ppWTNaeUZLZDdKT1l2MGk0YUlhVGFNR3FWRzRhVjMyOEgvTFhVUXh2cXFT?=
 =?utf-8?B?OGRxcWpYTVdTN1ZsSzZPamJiWFIweWFUWUJ0YXNyZmczNWFnVmNsd1ZKTDA1?=
 =?utf-8?B?UWVCcENNOUZzb2hwU0xhaEI5K0FSd1lLSnpJZjJBNXlKbU0vUEFsUll5UHpJ?=
 =?utf-8?B?QUVyb1A3ZldaZFlHYUk3dlgxaGNLcTg0SWhwT1pMSXo5dnkwTVdlSEdvUGlq?=
 =?utf-8?B?aitsa0F0amtpSXBFUFNybjlyOWd2V0Izc3RvTFUzYTNQR3Y0T1p5ZHkySGYz?=
 =?utf-8?B?cnVsSjNsOXhXZmVGQzJ4eFV6cFFVQXczZTNpOWpUMnFEQjV3Tm1DTUFwbDR3?=
 =?utf-8?B?MnRTWVpVQ3ErdkR1b1RTS09nenJCeEZWbnVWRGpNdFNUVkdVbW1ObmVyQWhp?=
 =?utf-8?B?ZjhoYlY2bE1lUjFNeDVUVUFxQ1VTc1AzTWJWL1hyODhjNDhRZlE0WHNMYnU4?=
 =?utf-8?B?bUNtUHV1UkNxL25wYzVLNjh5bFFNdThWWVdEQm5VNnZZNTNJSFo4c1c2V0Va?=
 =?utf-8?B?aERlcE94L3RlVXRTQzQ5YVo3dThCRk51eDErYjc4QXQ2V0VWeHZNUEM3OHpS?=
 =?utf-8?B?QXNWd0VtZFk2YUM2dUxzOForbWNrZUNvdExONnFoQmJ5bURYOUlBaGtobTE5?=
 =?utf-8?B?LytqWWJVdTdIMDdlb0g3K3Z4bWdScUw0SDRXV1czUm80RkVpK2UxTGFDNldh?=
 =?utf-8?B?d3BGU3hsYUx0QXNxNzJoVHhWWnoxWjh2eG5QbDRaWE5qUEhJOG1QaXlBWUdF?=
 =?utf-8?B?SUsya2JkdmpNRksvYTNUb0dmcEVhRStjTlRVcE52UFFxblBiMHFaR2ZCVCtD?=
 =?utf-8?B?cVE9PQ==?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: c55fed95-a56d-464a-9c76-08dab1287f81
X-MS-Exchange-CrossTenant-AuthSource: DM5PR10MB1466.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 18 Oct 2022 16:47:52.6105
 (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: 4qDzpGcBMYpUCb00fRFIIvIJ53pId/nQlKnLYDuY9gInXKBL20bWKPGYWuJXLnCl2qJcP7luKrS0BMbHllmJcXQFQOmfPtXi/sxJC/1DOzQ=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR10MB6250
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.205,Aquarius:18.0.895,Hydra:6.0.545,FMLib:17.11.122.1
 definitions=2022-10-18_06,2022-10-18_01,2022-06-22_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 spamscore=0 bulkscore=0 mlxscore=0
 suspectscore=0 mlxlogscore=999 phishscore=0 malwarescore=0 adultscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2209130000
 definitions=main-2210180095
X-Proofpoint-ORIG-GUID: kW2diLIwI2npxxHXH3EGK9wzFUbrIfyl
X-Proofpoint-GUID: kW2diLIwI2npxxHXH3EGK9wzFUbrIfyl
X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,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 10/17/22 12:11 PM, Khazhismel Kumykov wrote:
> If transport_register_device returns error, shost_gendev has already
> been cleaned up - however since we ignore the error device setup
> continues happily. We will eventually call transport_unregister_device,
> attempting to delete shost_gendev again, resulting in a crash.
> 
> It looks like when this cleanup behavior was added, iscsi was updated,
> but scsi was missed.
> 
> Fixes: cd7ea70bb00a ("scsi: drivers: base: Propagate errors through the transport component")
> 

Where do you crash? Do we need to handle the cases transport_add_device
is called directly and we don't handle the failure then later call
transport_remove_device directly?

The thing is that transport device addition success was
supposed to be optional where we were supposed to be able to
still at least setup the device, boot and use it. Tools might
be missing some attrs but we were still supposed to run.
I think that's why we didn't propagate errors originally.
We were also supposed to also be able to call transport_configure_device
even if the transport_add_device call failed (see comment in
that function for info). Does that still work?