Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH] xen/events: Fix race in set_evtchn_to_irq
To:     Maximilian Heyne <mheyne@amazon.de>
Cc:     Amit Shah <aams@amazon.de>, Juergen Gross <jgross@suse.com>,
        Stefano Stabellini <sstabellini@kernel.org>,
        Wei Liu <wei.liu@kernel.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Jan Beulich <jbeulich@suse.com>,
        Malcolm Crossley <malcolm.crossley@citrix.com>,
        David Vrabel <david.vrabel@citrix.com>,
        Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
        xen-devel@lists.xenproject.org, linux-kernel@vger.kernel.org
References: <20210811140806.75710-1-mheyne@amazon.de>
From:   Boris Ostrovsky <boris.ostrovsky@oracle.com>
Message-ID: <482af7c4-ad9e-6054-db45-ec05249a2517@oracle.com>
Date:   Wed, 11 Aug 2021 11:05:33 -0400
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:78.0)
 Gecko/20100101 Thunderbird/78.12.0
In-Reply-To: <20210811140806.75710-1-mheyne@amazon.de>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
Content-Language: en-US
MIME-Version: 1.0
X-MS-Exchange-MessageSentRepresentingType: 1
Received: from [10.74.99.104] (160.34.89.104) by SA9P221CA0012.NAMP221.PROD.OUTLOOK.COM (2603:10b6:806:25::17) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4415.13 via Frontend Transport; Wed, 11 Aug 2021 15:05:37 +0000
X-MS-PublicTrafficType: Email
X-MS-Office365-Filtering-Correlation-Id: 9421e239-e915-4cea-31a3-08d95cd97c71
X-MS-TrafficTypeDiagnostic: MN2PR10MB4062:
X-MS-Exchange-Transport-Forked: True
X-Microsoft-Antispam-PRVS: <MN2PR10MB40623EEE954B99B689F6730E8AF89@MN2PR10MB4062.namprd10.prod.outlook.com>
X-MS-Oob-TLC-OOBClassifiers: OLM:8882;
X-MS-Exchange-SenderADCheck: 1
X-MS-Exchange-AntiSpam-Relay: 0
X-Microsoft-Antispam: BCL:0;
X-Microsoft-Antispam-Message-Info: kIGKsSuwbk9wcAFbVc8LBE6/X+XLLBiSLxM6MHoAjQuUasTfcaHb3in83SV4nE/tqz1P9qlvMTH+v5IMf6Fe0/D6W7o4ZPauzNhX9tbpWIKgaMgrAkQG/FSb6Ya3l9ePclyzMSd/im9OI4uTjjntDX/jUHgnAKIOK/52OGde7UR3esJUtEidjlk8dlaLaMubsbVCCJ4o9Barxum17Fk9sgftGXfbgWwpsEEFrxHHi6q44d5EEmZeONu5Jy8HBrTpqOao4ZkmluVs+rlAYefziz8/alLXLLC9MHUHY9u6XMhsztfteA8HXID3+qHzeZoNHk8MWlaq78fQIMohEowwCqgpAzBTeOHGgT9yqnZk93bd5YfBeOT9TuIH579kWwObCxQ1IQhIpUjIIIpX0sApriVxeGGj9WxSkljh3pjP2HLQXnq2C2VXYQvMGVZ5AyclJt870Upl7X7+pGPrnOl/NzbgDfHmQIA5zYi7N7VcNhWywpana8btP7MUWzBtlWO1ZbQLvuGGUHnVE9PjBlkfEQfwV+i1ekPRxX2S9R2DPbA8DcxOYxVzr+j+INpqRzVjatxlMDHXXrzSy0YkVklqRhwU/KK/zoW644DKO1rGzVe15BbhsRjNFMslc5gaQb8QVqvTIxVD09Y1aB0eoxyOn/EFXJWgw+kwVVRaWndvnFmqyuhsb1zzSP2pDJsOP0fkNovOXQceZPn36ubDns4ek5kjusgIQ/8k95mmEpEBAWo=
X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:BLAPR10MB5009.namprd10.prod.outlook.com;PTR:;CAT:NONE;SFS:(136003)(396003)(39860400002)(346002)(366004)(376002)(31696002)(36756003)(83380400001)(4326008)(316002)(86362001)(7416002)(478600001)(8936002)(8676002)(6916009)(16576012)(6666004)(38100700002)(54906003)(66476007)(5660300002)(6486002)(66556008)(26005)(66946007)(53546011)(956004)(44832011)(2616005)(31686004)(186003)(2906002)(43740500002)(45980500001);DIR:OUT;SFP:1101;
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?bW5oN1l4SkhkaWNKZW9GR0hsb0NaeUhvby9IZWZFWmRzOWxBUmUzSEJpOGdJ?=
 =?utf-8?B?THoxMFlwRVJ5M0NMcEFJV1VzWjZoU2YyalpZcS9GcjZMdE1vMmZKejNPZGlZ?=
 =?utf-8?B?dDdSRURDWHg1R1c1bjVyQjB5Y2RoOUZUcHZuc3gwN0hJU2txdEhSMHdXTDFt?=
 =?utf-8?B?VHhyQWVSWWxRTDRVcTFmdHVROGpGNUlOekJiMHVrSytwRk5YUlFaZThxU0tP?=
 =?utf-8?B?czhaemlGcWFnSlA2OUdPRGhQSnN0M1dFUDM5S0owWnpkbzA2RXhNclJNc3FP?=
 =?utf-8?B?OGU2NTlOa2tEWlFFZlpMNm5jTnV5UjVTZVl5OElqbGdmZnBuQ0xYWGJSZll2?=
 =?utf-8?B?NlJnQy8zRWwwUWU2WEFkdkk0aEYzMFJJSFVrWlgxbTU3T0VNek5UTHEydkZi?=
 =?utf-8?B?ZVZvOTJ5Tnp2ME0rYUxNY1RUNW93ZmNsc0ZCbjNiUDJCbDNCL3FVMVMyaU5V?=
 =?utf-8?B?Mm9ER1NsUUZURzZRa1NwQWVkcnBCc21nSXhXYkd0eGNtQ3JzR0ZTTFRvN1ZY?=
 =?utf-8?B?bmxOenhVNll4UnZEUHphZFREYy9ZZVhuWEdVVDFLTHNVSnRDOGRmcUp5ZUEy?=
 =?utf-8?B?ZXB0czJTL3JpSzZiZ1dYb1k4cGptcGkyRUxiUEtENk5sVE9odVVVNHRqVk0w?=
 =?utf-8?B?djlPT2RSZXJQZEV4RThKd2RzMVY1WjhKek1ZVGJqV0JMSWNMZnQrWUxFYkkz?=
 =?utf-8?B?UlRKbFpPbU9IRW94Q2xQN05nLytKeEFYTEZORHFyeE1vYjIwWGVoYVRGNkpa?=
 =?utf-8?B?Y0lLQzB1UVEyZ1RueTFUNUovZlhybDdwZDVnV3ZqOUc3RFpRekFrNjFkUFlp?=
 =?utf-8?B?YzNKb0NPdldBNkNyK2Z6Y0ZvRU5VRFZ2NFBCMUMwNUJ4MTk4c25rZTBMdktZ?=
 =?utf-8?B?QW44V0oza1cwUWNUb21MczFSWENPWGRXTXY3SDhJZnp2eE5lQ2J2VmFSUDV4?=
 =?utf-8?B?YzQ5aW9XOFp1WnNUWTVIcGNHREdDZUd1T1ZxUDNJRUJXeVJXVnRmcTEvenVF?=
 =?utf-8?B?ZHdzaTJTZmd4TzJMeEM3bUdZSlFpWWVUNEt6QWJrSXpMWktILzQyaEI5cnFk?=
 =?utf-8?B?cmdibEtYaWRydlY0bld2cU00UW9IR25obFNlUlZJU1hRVFVzY3k0N210bndG?=
 =?utf-8?B?a0NPSm1FcE1mNWVtQk1OSlZBaEk2WmNhdXlucnhTYUxubFJ5QlJkUG5QZXMw?=
 =?utf-8?B?bnBnb1RUa3ZBMEFHaW5iZHoxMGhBZDYxRHZ6R0c5d3dONVAvbm1OaFQzVG1x?=
 =?utf-8?B?MUxvQWl3UkNvR1lURWJhT3Y5ajBWRjQvRUR0TkRJQWtCUXRuUEQyejErWFFl?=
 =?utf-8?B?b3UrdWpqaTFoS1RnZ1FmTyttMmhOQ3RVRE9rVWRxakRvcGIxMU8rQW5tR0hP?=
 =?utf-8?B?TlhyYW43alVSRDVsQ3k5RXg1R2haRjgwcWFkYTIvQ3JEWlJsSGRWdEwyRHFY?=
 =?utf-8?B?d3haMDBzNWhkTWpjeFRPbkRoN3ZaNlRXbHVyaEpKaHAraUR1bm5JNG90NWVJ?=
 =?utf-8?B?WEIxQU9qYm1QdThIeEhreW5ISUQvY2l0SlpGYzluM0hPbjQzVHVOTVFpbkRQ?=
 =?utf-8?B?ZDN0a2xyMTRkWmJLTEcwMm56bDc2YzV6R3o4VXRlVWZkYU01UUpvQ1BkN3Fn?=
 =?utf-8?B?K2RjSXpJQVFvUFQ2aDhSZ0VxZUZmZjd3MTh0a1llT1p1cFFJa0NOWWFiSWNQ?=
 =?utf-8?B?RGpHSFRTRE02Z09ldVkyNG04aU5zalZMM2VNdEFWbWw0R1BPUDNhbG5BNFdZ?=
 =?utf-8?Q?Kw9bN0Fw5M+WBqPd0IvqGOVIrH7VqmGIKcT3QIm?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 9421e239-e915-4cea-31a3-08d95cd97c71
X-MS-Exchange-CrossTenant-AuthSource: BLAPR10MB5009.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 11 Aug 2021 15:05:41.5744
 (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: IZpdoqsqxd1qjdb0iIoE1n22LkLGHTr8XvTw32+grNIefOskwvVSj1ql3vz1wChajdyZ79GHIAEC4Q8syHAKp1hDBZpblAIeNoqM/2viPrA=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN2PR10MB4062
X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=10072 signatures=668682
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 adultscore=0 mlxscore=0 bulkscore=0
 spamscore=0 phishscore=0 mlxlogscore=844 suspectscore=0 malwarescore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2107140000
 definitions=main-2108110101
X-Proofpoint-ORIG-GUID: YnaBpIpl3LJ3d-hhg-PW9E01XoZSRxLz
X-Proofpoint-GUID: YnaBpIpl3LJ3d-hhg-PW9E01XoZSRxLz
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On 8/11/21 10:08 AM, Maximilian Heyne wrote:
>
> This patch sets evtchn_to_irq rows via a cmpxchg operation so that they
> will be set only once. Clearing the row was moved up before writing the
> row to evtchn_to_irq in order to not create a race once the row is
> visible for other threads. Accesses to the rows are now guarded by
> READ_ONCE and WRITE_ONCE just as for the columns in the data structure.


Is this last part really needed? We needed to do that for array elements to avoid an interrupt handler from seeing a partially updated entry but I am not sure I see how this can happen to the row pointer. The only place where it might be important is when we update the pointer to the new page but you are using cmpxchg there already.


>  
> -		evtchn_to_irq[row] = (int *)get_zeroed_page(GFP_KERNEL);
> -		if (evtchn_to_irq[row] == NULL)
> +		evtchn_row = (int *) get_zeroed_page(GFP_KERNEL);


Not directly related to this patch but I don't think we need to get a zeroed page --- we will initialize it to -1 immediately below.


-boris