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: <e983fecd-ff59-e97e-0099-b33685d45d00@suse.com>
Date:   Thu, 27 Jul 2023 17:31:41 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.13.0
Subject: Re: [PATCH] USB:bugfix a controller halt error
To:     Alan Stern <stern@rowland.harvard.edu>,
        liulongfang <liulongfang@huawei.com>
Cc:     gregkh@linuxfoundation.org, linux-usb@vger.kernel.org,
        linux-kernel@vger.kernel.org
References: <20230721100015.27124-1-liulongfang@huawei.com>
 <c3ab029f-f6ab-4b09-b2b5-1cc6a5370d0d@rowland.harvard.edu>
 <bfee90c1-a7ca-27e3-88f9-936f48cd2595@huawei.com>
 <bd440f1d-5ea4-485e-9924-433997765adc@rowland.harvard.edu>
 <77a8ecb4-8099-1826-abd8-4f080d80b07d@huawei.com>
 <73b58ff7-2a0a-43f7-bda9-52b9437f5bc0@rowland.harvard.edu>
Content-Language: en-US
From:   Oliver Neukum <oneukum@suse.com>
In-Reply-To: <73b58ff7-2a0a-43f7-bda9-52b9437f5bc0@rowland.harvard.edu>
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?TjA2Wm1MMkh2b0RJOERhOEx5ZzcyaFJWbEhDTlJBWGFqbUVPcWJqalljcFlI?=
 =?utf-8?B?Zk5wdnN4dHkzbWVacWxHMXViRWRxWE1jek5KNjZBNGRYaXVUNElUUzlrUElC?=
 =?utf-8?B?M3ZtcWdoTGZwd3c2bHhleDEwVElFb21ub3VUSnZoMVh5MW9pRFVOR3A3cStj?=
 =?utf-8?B?KzYwWnBKNUF4cVFoWFM3eVNPYmV6dGVFdVNCb2ljS29JaWtGdG50S0lva0Zx?=
 =?utf-8?B?R0pBOG9GeGFGT2Mwc0E1TENhUjBmbW9OeWVSNUx4NmFGZjNvNFBWdC9KYWpa?=
 =?utf-8?B?THB2MGxJQVNSZ2c1cW1NM2VwbGJTL05wNWhIZjl3WHQ5SWdFUzdEY0NVYU1V?=
 =?utf-8?B?dDJHck9zWXB1bkplc1NOZWJhcXpoME4xZDVmVnhtMHBiUmVDVm0zcG1Xa1ZN?=
 =?utf-8?B?clZRYnh6WHkwZCtXL1FlNytLQ1pKcit2SzBmYmsvdEpRSzhzOGpZODhZb3JR?=
 =?utf-8?B?MVAvTVlmZzRsNEhzR3JDempLd3k5V1Y3bGZheUFuUDBuZmNBSWdXcmZ0ais5?=
 =?utf-8?B?WHlZNXBOVzRUVjNEeXVwemw5b2hoRHFLbGc0djlNUGtMbThOKzdGSThHem9G?=
 =?utf-8?B?MXNLREZtbHNxQVJSRGhnSzBTWjZYYjhxNkxEdjU1aHU0aW5TVVlhOVhRL3hl?=
 =?utf-8?B?YzRyMXBvMmtDcWN0V2cwRk1va0NNRkJocjNlM2FnS0swdzV3SFU0NEgvRGhU?=
 =?utf-8?B?aEdPblhNbkplb0UrVnVsd0NjQkMyNnpwU09tNG5DMEFQSEZ6djZnR2gxa2dn?=
 =?utf-8?B?WnBkZnNFYkpQYkl4RzE4b2lBcTgyR0M2eWswSkM0cVJxd2FkZFhCb2VTZFRw?=
 =?utf-8?B?UXYvQXBLSkpSZFJ2R3Q1T1UwTXdoeDJOc2dNcmpsRjBBTjVTOTM2SXg2eUlI?=
 =?utf-8?B?NWpIVG92bXp6K1ZMNjViZ3I1ZTZCKytyaEo1UFlUeHJ5T0xtWlpJTGcwZGt5?=
 =?utf-8?B?bkJXcDdZL3JnZ080QmhjOWgraEFWZHVNZVZMZlJ5N0dITENoQmNZNjJickJH?=
 =?utf-8?B?WHFDOUl4ZW1jZHd6eEFTUXcwcE1zNU9MN1BGT1R2Y2pIaHZmOU1nZUxmSVVp?=
 =?utf-8?B?OGFaY3V2ZExhN2VuMEJFNnNKVndsOEoyQ1dyL2l2aGU1aDV2dE1Rd1Z1MkpR?=
 =?utf-8?B?THZOOXFJR1BxRy9OekZhUzNYbWZ2SlJPYTUwbDVrcWlQVGdIMkdpUXpkMi9I?=
 =?utf-8?B?dm16RkVWV292Ny94b09xdUVETFRGWXliU3pCalU0cDltVVVTa3Q1cWxZTDJ6?=
 =?utf-8?B?NzBZZEJvbzEzTUZrUVUya3FpM1pwaDVuaU5HdG9jQklZRmhNVzZDck85RDV2?=
 =?utf-8?B?emQ3Y1Y3cy9GZWdtbVNURFVHK2dGQVlMNXZHZ0hDVkZhcDgwUlV0ZEhmSEFu?=
 =?utf-8?B?SFlKd014K1ZVSnllcXdhdVhUVWduR0F6MUltcjNBV1dRbmVxcnVSRUhPY05n?=
 =?utf-8?B?VE5pMnNBUzNMTkpUczNLQkhWTkprUDYzRHdhaXBhQjhadHVWRitONTJyNEhC?=
 =?utf-8?B?eDVGVXJFc2NGOGZ1RWxJYWFEMm51NDA1dHFIZjAzL3VhbWR3MDhkTFREdVd5?=
 =?utf-8?B?eUVrSEdHem53UERJRmU2bzlzZ2gzQmtnOFEwMWRGSVBQUmxxVElRc0VoR3Z2?=
 =?utf-8?B?T0lNbVVXR2t2emlnS2tBay9LTGZFU0FHMll2QnU0REJ2dlVWc01iRkFRaEZ4?=
 =?utf-8?B?MmJIRitRd1VJY3BKdVFpbkg4S2k1SW9xZUllWmo1Ui9XaCtpeVZGWGJBL3B0?=
 =?utf-8?B?aTdmNGUxaWtJc2hTM0M2ek0veTU4YktSc1AxMS9WWFVBRHB2bEZGTDcxcDFp?=
 =?utf-8?B?RDdpa1JseWZ6aXBVZ3ovZCtUOENZVVd4TitlaDBTNUNIczNYUThuK01YNlpS?=
 =?utf-8?B?WUZNeFUwYnZ2eEI1K285VDB0Z3ZnRy9BdGxQMWpBYVNlTGpXNTQxZU9rNTRG?=
 =?utf-8?B?TEE3TjV0WWNCaWtSUEhwdmZ3bzZyOGh5V0g4Tk9IM3puLzBuZVpwbVdyMEM2?=
 =?utf-8?B?NGJsTlRiN0kyYjBjQXE5ZDBLVXcxUWJzRnhYSmxUNFlITFZGQWZRamZ5UHB3?=
 =?utf-8?B?L01KQ0o4RFVzUW5yZG9ZMTFlbnVnTS91Z1dENGZrSjFRcnVuSlBrTmVnQlcw?=
 =?utf-8?B?ejhzTUtNYWg5ZkhYOVZDS1pGZ2hkZnlON0ttaXR4TnR6bmNlY0QxNXdvWEVY?=
 =?utf-8?Q?jThpag+D4hNCF1UXCiHYx+s=3D?=
X-OriginatorOrg: suse.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 44818e8d-3eb6-4da2-b9d5-08db8eb694b2
X-MS-Exchange-CrossTenant-AuthSource: VI1PR04MB7104.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 27 Jul 2023 15:31:43.4675
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: f7a17af6-1c5c-4a36-aa8b-f5be247aa4ba
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: DchgO1zfXDn80MSq9wFBFmCGguU0phmlYD6KEdw4p3fXlZ5BDbpa50R3D9QN84dfMWeO91KIlD8y7EEQ8+75Jg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM8PR04MB7777
X-Spam-Status: No, score=-2.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS,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 27.07.23 16:42, Alan Stern wrote:
> On Thu, Jul 27, 2023 at 03:03:57PM +0800, liulongfang wrote:
>> On 2023/7/26 22:20, Alan Stern wrote:

>>> It seems to me that something along these lines must be necessary in
>>> any case.  Unless the bad memory is cleared somehow, it would never be
>>> usable again.  The kernel might deallocate it, then reallocate for
>>> another purpose, and then crash when the new user tries to access it.
>>>
>>> In fact, this scenario could still happen even with your patch, which
>>> means the patch doesn't really fix the problem.

I suppose in theory you could have something like a bad blocks list
just for RAM, but that would really hurt. You'd have to do something
about every DMA operation in every driver in theory.

Error handling would basically be an intentional memory leak.

>> This patch is only used to prevent data in the buffer from being accessed.
>> As long as the data is not accessed, the kernel does not crash.
> 
> I still don't understand.  You haven't provided nearly enough
> information.  You should start by answering the questions that Oliver
> asked.  Then answer this question:
> 
> The code you are concerned about is this:
> 
> 		r = usb_control_msg(udev, usb_rcvaddr0pipe(),
> 				USB_REQ_GET_DESCRIPTOR, USB_DIR_IN,
> 				USB_DT_DEVICE << 8, 0,
> 				buf, GET_DESCRIPTOR_BUFSIZE,
> 				initial_descriptor_timeout);
> 		switch (buf->bMaxPacketSize0) {
> 
> You're worried that if an ECC memory error occurs during the
> usb_control_msg transfer, the kernel will crash when the "switch"
> statement tries to read the value of buf->bMaxPacketSize0.  That's a
> reasonable thing to worry about.

Albeit unlikely. If the hardware and implementation are reasonable
you'd return a specific error code from the HCD and clean up the
RAM in your ecc driver.

The fix for USB would then conceptually be something like

retryio:
	r = usb_control_msg()
	if (r == -EMEMORYCORRUPTION)
		goto retryio;

	Regards
		Oliver