Message-ID: <bc47bbe2-b330-7744-8d6b-869e3c7523e4@suse.com>
Date:   Mon, 22 Nov 2021 08:57:32 +0100
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.3.1
Subject: Re: [PATCH v2] xen: detect uninitialized xenbus in xenbus_init
Content-Language: en-US
To:     Stefano Stabellini <sstabellini@kernel.org>
Cc:     boris.ostrovsky@oracle.com, xen-devel@lists.xenproject.org,
        linux-kernel@vger.kernel.org,
        Stefano Stabellini <stefano.stabellini@xilinx.com>,
        stable@vger.kernel.org, jgross@suse.com
References: <20211119202951.403525-1-sstabellini@kernel.org>
From:   Jan Beulich <jbeulich@suse.com>
In-Reply-To: <20211119202951.403525-1-sstabellini@kernel.org>
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?U0pPRDJjV1NtcEdvUStnR1VWUi9jSnQvRVVkYkpEMU9UcEprU2kreWxFR080?=
 =?utf-8?B?c09ITk9MYmtmWnhBU3E3aWRXeWZIT3lPckMyM3R2akdZdEZxV3VsMWJ4TEFR?=
 =?utf-8?B?MDRtUHM0Y1RxTUxSbElDMEY3TU5CQ2dvanBiTGhSMzhOa3dDa2wweGZiVjVW?=
 =?utf-8?B?a3FIUmNMQmtKM3duMVVqa0tpNlIrbTV5cGd5MnFhSHBiYXRVSnN4djFLUHRv?=
 =?utf-8?B?eXV6ck12NXJxLzFkeGtuQlYrc3A2Q200aDhiSHdKNFNJWVhhL1VmNW9HMDVF?=
 =?utf-8?B?TngvODFQT0NuRVY1Ym9qUkZWM1M4Q0FCSTJoSHNTV1Jma0RpTmZmcEFEaTVH?=
 =?utf-8?B?NHYwQ0VRVjNEalFZd1ZiNWtGbk81S09LU0F4K1VyS2lPU2lQVm9yTktzT1dZ?=
 =?utf-8?B?azd0TjZzU051VTIyVkFnN2hSS1NrR3lNbHZHL0EwRUZxbVQ3alEzaTVDTXFG?=
 =?utf-8?B?Uy9qa0ptRHR4Y2V0dTNRNDdiQWhKWDVzbWdIOUdMRFV3ODB6YitoVzhCOEhR?=
 =?utf-8?B?RzU3cUQreXAvYTh4b1RYblBCRlYramkwQWc2T1cxMG5RQW1YVGpxYklMaHFC?=
 =?utf-8?B?UkljdnljcDV6Qi93aEh3VHlYUHZpOW9tY2xaSjJxZzNBeXdTU3ZVK2U0bGpj?=
 =?utf-8?B?UmFJZ3V0UWVXdFFFV282d3lPQmV4d1kya1dKcmIxSi9ha1B3MldOaEhwbHRL?=
 =?utf-8?B?amI2OE4rdnJiQ0xISFpNSTFrZ01vbWlhRGl4Ui90bmphSE1oM2ZDRFQ5MzVW?=
 =?utf-8?B?MS9pZVhubHlkK2sxVFJnYm0zdDRmYkZ6amxhMnJjS2xrQXpEZ3VKK3h4Q08y?=
 =?utf-8?B?TlRDM0l2RHB4d0dUMUdoUDVENlR0YldRWnhZQjFDVGRncmNKWjJheS9OMlJ4?=
 =?utf-8?B?N0JKeE81VjdZV25JYk1ONW5nTi8xQlFvTUdpTjE2MUZtTlVRQ0NLQ29qMmlh?=
 =?utf-8?B?ejFmalJuSzJBYkMzeVdVVWd2RUgwUHY5eHR1U2tJRU1HQ0dmajYwby9XU3lF?=
 =?utf-8?B?eEhGQnAzcWM4anYzUE83eVhvZmlxU3BEQWx1bjNjRXpTRW5xV05iTGh6aU5n?=
 =?utf-8?B?Y1dzS00waXhwTEtla0RXL09hNnJ0VTIzTG9hQk95VlZ1Wkc2SjJrUitMRnda?=
 =?utf-8?B?aEhnSlQ5Y09tWjVHclVvNXU4ZnhDMko3TDIvUG5kc2hiWitnMEV0VDVtdkF5?=
 =?utf-8?B?YmVENzBPdHNqY09HWG1xNUEzSTFGN1ducnJvVHF3M1g4cXEyQjRTVUVNMUFR?=
 =?utf-8?B?U2dHSEx4dDkrR0duMmZUOUpQUFJNSi9BT210Nm9uNElZZ3h4TTI3bDNKMGt6?=
 =?utf-8?B?NTBzUUtrTVpXcTd0YnEyWWM2aVFLK1N0TFJUR0dhRnROSmtOUmh0OTU4SU5J?=
 =?utf-8?B?NWZHR2pPVkJCT1hmUHo4MHVBMTBvc251UjMxUHdOeW45MTYxSWdxN0FkdHox?=
 =?utf-8?B?TGh2aDMyQmhXYTBqSEtvY3VsY1BWcjNVTy9CMEEvV1gvZlN3VnlBN2V2b3d4?=
 =?utf-8?B?SWpBQlZHTzNXYU8wenJhQTF3MlMyQnNKTmloOHkrNnpGbzFrMmtLK0JEZVlk?=
 =?utf-8?B?Y3Z1dU5TM0FhMitOWTFqcVF4V1JybkxMbVNXWWVzRWxldmtTM1ZwN2FuVGtF?=
 =?utf-8?B?bnN2eHRZVURRUW9IY0dGZkpNSExQTTg4L1Y1cUt1TVIvb2J4cit6dllJbUZw?=
 =?utf-8?B?R2FZU2FvaTdmd3pRNitDZ2poSEJKc1VIR1RxdXFlT1ArZHJzdm9jdUUyOWU2?=
 =?utf-8?B?N295Zmp2OUc5WHdFNXJNc1FHRC8xUzd4M0FJOXBBZWx3WlRFcVhaM2lCTjRj?=
 =?utf-8?B?L05pRmZFQlVSMnduZllhTWQ3YVV3UTllZGV0NFV3QTk4V0E0bzhZN2FhalNK?=
 =?utf-8?B?eG1KU2x3endZMHFkNHplNHMyS0piT2pVa1k1S3BlcGhZb3hlZ0FjMTN5eGti?=
 =?utf-8?B?YWkvWU9PZm43emhDZS9XaEg1WU5YdlhkUXo3Vm5ZaGJiN2hTM3V0UGhFNm8r?=
 =?utf-8?B?TmdKVmtZNDJ6dzN6T2VtZitIQklvMUFqelgxQWZtcjlwSk1uL2JuMUxCVFA3?=
 =?utf-8?B?WjNiRkJXUjhsd3NldGErbkdjMlRMSHhxdzlNVXZHbE1LaTNIdnJ4dWVLbVpl?=
 =?utf-8?B?OVR1Nmh0aG1YSEJYMGFsS0Q4Q2d6Vm01M0ZnSklOdFh2eTV5Y04vTW5WZlAz?=
 =?utf-8?B?OFB1Y1hXa3FDcDBvOFRPcEpmQmR5N2RzZEJSZ3cxVyt0STR2SUMzbWtpZjdm?=
 =?utf-8?Q?+bW+VYKM1pyhIAMulOQot91l2G5Hm7hw++A8hh9IPA=3D?=
X-OriginatorOrg: suse.com
X-MS-Exchange-CrossTenant-Network-Message-Id: ce01e611-baa2-4c54-8d3e-08d9ad8dbdb4
X-MS-Exchange-CrossTenant-AuthSource: VI1PR04MB5600.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 22 Nov 2021 07:57:33.5645
 (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: v/gh0m98UAzlJOFPJrVDp5mtE+RfoiXfJqOvLpObK2UujR/Zs69LlbbmmNo2187bwg8+M65CGWXjZ29EYblaug==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: VE1PR04MB6383
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 19.11.2021 21:29, Stefano Stabellini wrote:
> --- a/drivers/xen/xenbus/xenbus_probe.c
> +++ b/drivers/xen/xenbus/xenbus_probe.c
> @@ -951,6 +951,20 @@ static int __init xenbus_init(void)
>  		err = hvm_get_parameter(HVM_PARAM_STORE_PFN, &v);
>  		if (err)
>  			goto out_error;
> +		/* Uninitialized. */
> +		if (v == 0 || v == ULLONG_MAX) {

Didn't you have a comment in v1 here regarding the check against 0? Or was that
just like now only in the description? IOW I think there ought to be a code
comment justifying the theoretically wrong check ...

Also, while I realize there are various other similar assumptions elsewhere, I
would generally recommend to avoid such: There's no guarantee that now and
forever unsigned long long and uint64_t are the same thing. And it's easy in
cases like this one:

		if (!v || !(v + 1)) {

Jan