Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Message-ID: <00c1cd38-5164-edfa-6c47-606803629dcf@oracle.com>
Date:   Wed, 20 Apr 2022 12:13:50 -0400
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0)
 Gecko/20100101 Thunderbird/91.8.0
Subject: Re: [PATCH 4/4] xen/scsifront: harden driver against malicious
 backend
Content-Language: en-US
To:     Juergen Gross <jgross@suse.com>, xen-devel@lists.xenproject.org,
        linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org
Cc:     Stefano Stabellini <sstabellini@kernel.org>,
        "James E.J. Bottomley" <jejb@linux.ibm.com>,
        "Martin K. Petersen" <martin.petersen@oracle.com>
References: <20220420092503.11123-1-jgross@suse.com>
 <20220420092503.11123-5-jgross@suse.com>
From:   Boris Ostrovsky <boris.ostrovsky@oracle.com>
In-Reply-To: <20220420092503.11123-5-jgross@suse.com>
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?YitVZUszQWxGdFBibUFsVzFqK21BQ1dJT0gyZjJwTjRHTVBYeWhmdFYxdlJ3?=
 =?utf-8?B?cnlCRXlXOUlPdlhGYm9NcGoveWtFVHM2N0lsRE5lMEV3M3JTdllvODYzV2wr?=
 =?utf-8?B?eUJ5ZVpjS3hHOERmTG5UY2dRQjlMN3c1UnhZWmhoOS9rQ3d2NXlVdG1ZNnNH?=
 =?utf-8?B?aEdNdnBjcThEUWRiVHE3bW5KclRkTks2VXVobVZKVHNNek9wYVB3SDZuaGg1?=
 =?utf-8?B?OFEzK09qL3c2bTlyejFtZk5YdDQrUm9XOGt3endsa1diakFZQVowemRRQ3JE?=
 =?utf-8?B?eFBmK0NXUU9zRTFTc3pvM2lDL1Rvakw0aitRellPOTJsaTZjQ1NFd1hFNHRj?=
 =?utf-8?B?d3dwQnh6eUwyK0lLRkJhaDF6Rm00bTJrcnFXWUpMSHJRRTJBalFJR0phUVFh?=
 =?utf-8?B?NlhnRXRQNlo4RzZsV3dHQ1lHTlR2dmVzZHVNV09CeXZKUkFrdFJzL1FycDJk?=
 =?utf-8?B?d0ZMOGN5SzB6bDVMUXR5VGEwQTdBT1pqVUQzWlYwd3JubzF0Q3d3WlY2MXJE?=
 =?utf-8?B?cTlTdEplZGlUUVY5SnQvSWhzTFBkWmRZNnMzSmFnVVhzbFRSenJNUG5WeGJF?=
 =?utf-8?B?WlI0TGl2dGlkWWQ2OGM3ekNycHZBWDY3bzBuenRaeGtqVUQ4UEVsWXpzYmcr?=
 =?utf-8?B?YmlVQ2l1RkFpQ0NvZHNLM1IvNnJXRHJDNHRPNllleVQyTUNqaTByQzBnd0VV?=
 =?utf-8?B?OG82dUxwTmVyejlrL0RKVDVIaWROMzZMZEhQekhjcVhsenZ2TldmZlk5U1hr?=
 =?utf-8?B?THFxbGVWTzNubU1JNVo4YWI4emo3bHZuemxPOEl0QUFOdWxYaS85RWkyWGNu?=
 =?utf-8?B?S3pTUXR0SEJTVTNQUEJLckVKWmJDcXZ0VVc1OEVaR1Q5enFrQmVtdGg4RmR3?=
 =?utf-8?B?YU0vZHQ4K2l5cU8vOWZNNGxTbFplRUFKUDBTVGR2ME94VzU1S0w0eWlVT1J0?=
 =?utf-8?B?cmZyWENxaUlIVzVFYU5aWUwwSWNTOEFxalBwbDU5cGVPRFlCcDNoVDQvbDBQ?=
 =?utf-8?B?VTlsVDFaRDE5OUd5VnRraGE4WnNYUEtXMXBXVUtzNDFsdVY4SzViSWRaM2U1?=
 =?utf-8?B?a3Z3NDlPMXg1eWJMd1liUW5kY0M0eFJPM1hRb01ubS9tSTVObVNVeXRJSDZq?=
 =?utf-8?B?UnIzMXlsUlc3VjJxOWlnbjRrV1JmaWFsOWVIdGxtWHljbHhaWjZvNnlCZ3Bn?=
 =?utf-8?B?SjZYTk94QnIyTkpjV0piTHNOdjhEMHZqQjdlakxCYWQveW1nMGRpWmI5RUdi?=
 =?utf-8?B?clhRMm9nVG1WRWhTWHFwVllyQWhaNittamJkWENxanEvL0xqWmhpR253Nndk?=
 =?utf-8?B?QmdwSTRwRmhCZjZ1K2tMM0VYVGZTNnc3SU5hbVJyRlljL3UyWWpwb01HRHlV?=
 =?utf-8?B?ZEpmWHQ0ZVY0S1FhZlRWVU1pYXR2aVNwU2JlRzJudVZwSGZzMktGVVFSRVND?=
 =?utf-8?B?enpseXY4YXJ0RllKelJnT0JpM0FUUkZadVdub3V2V3dhbzY5SWVRNS9OSExs?=
 =?utf-8?B?Q1dWVFJaNVlKQ1Y5akp5L0g0c3d6OVF6UlVIRXhINWVINEpyVG5WeTE1M3V1?=
 =?utf-8?B?RkRVVUU4L1FJeHNXekJ1TnYxanZRZElWQTludndXOFFQNHdUWlNycHFNTTY2?=
 =?utf-8?B?L0lOYmRaNGM4eFZpUVNSUGxETExpNTBMbHNhTnFQU29PRjEvS2FUNjhzMU5D?=
 =?utf-8?B?V1lDb3lvZzhjcFpxdnlQWGlJaUxRU0puTnhnTnhmNS85TlpTRmoyWVNOYTNN?=
 =?utf-8?B?aWhRd2g0Sm94dzM0aWhnWjVQcHFsL0gxYUxNam8xem9GVXVNU2tST3JFR1FD?=
 =?utf-8?B?K055RDB0OE1oZlY3eTFUa2oxazhVMzJnZUdTbWk5L0NSS0U3ck43YTVLKzBY?=
 =?utf-8?B?U2hqTlZ5YWtnMEF4SVU1T2syS0Z0eVBnNFR2QkN1RGUwdHdIeHFwZlZBbmdV?=
 =?utf-8?B?bkdnOFRRbWFGL082SGJUOU54b2QvMnNXaGNHbkVXY0lHaFpkMjR2OVpZYThT?=
 =?utf-8?B?VUJTdXMxSHJaU1BBOUk1QjZ1MXNrVS9SNGJSK1lmYkdiWTFKaGduUmZaQkl1?=
 =?utf-8?B?YVJRNlM2MEY4VXdUWDdaVGxTZmNVZm5qR0ozQ3RnNHp4MjVYMHhNZlZscFBt?=
 =?utf-8?B?L3BmeXM5cklCdHpBOXpIUi9BN3duZ3NZbFB6b05Gb3pmYVc3eDBMMWhTQTNu?=
 =?utf-8?B?S1RqWjBLNm9teU5jNUY1REduNlFJb0pvaDAza3N5eU1lYlBCK2JSbUl1ZG1Z?=
 =?utf-8?B?SWs3U3JnNlF1VE56VzNnZ1dnTFJxMmZiUEloTmZaQ3VYT1VVY3ZHU2Zmcml4?=
 =?utf-8?B?Z1g0L2xMTWtMZTNQelloTitCZ3pzdWtCcU5MeGRYOVEzYjMwZmYwSHFuQy9u?=
 =?utf-8?Q?UpQFlkVdTII62WuU=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 93297cad-a2bd-45ea-b52b-08da22e8c428
X-MS-Exchange-CrossTenant-AuthSource: BLAPR10MB5009.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 20 Apr 2022 16:13:54.6199
 (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: Ja2wW0udeLSZSj/0MDSGYw0Dn5coA43XcPww7sB+bvvEWIVAAx93MJMfE/TT8lgRQr0G4mWuLs4bd4Swd9UKjRmX+vCrT/15ckZ6RqWpjRQ=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY4PR1001MB2280
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.486,18.0.858
 definitions=2022-04-20_04:2022-04-20,2022-04-20 signatures=0
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxscore=0 phishscore=0 malwarescore=0
 suspectscore=0 spamscore=0 mlxlogscore=999 adultscore=0 bulkscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2202240000
 definitions=main-2204200097
X-Proofpoint-GUID: ci7Wv1xx3pzIbiV1hcxAvsA6Hu_Dne3e
X-Proofpoint-ORIG-GUID: ci7Wv1xx3pzIbiV1hcxAvsA6Hu_Dne3e
X-Spam-Status: No, score=-3.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,NICE_REPLY_A,RDNS_NONE,SPF_HELO_NONE
	autolearn=unavailable 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

Just a couple of nits.


On 4/20/22 5:25 AM, Juergen Gross wrote:
>   
> -static int scsifront_ring_drain(struct vscsifrnt_info *info)
> +static int scsifront_ring_drain(struct vscsifrnt_info *info,
> +				unsigned int *eoiflag)
>   {
> -	struct vscsiif_response *ring_rsp;
> +	struct vscsiif_response ring_rsp;
>   	RING_IDX i, rp;
>   	int more_to_do = 0;
>   
> -	rp = info->ring.sring->rsp_prod;
> -	rmb();	/* ordering required respective to dom0 */
> +	rp = READ_ONCE(info->ring.sring->rsp_prod);
> +	virt_rmb();	/* ordering required respective to backend */
> +	if (RING_RESPONSE_PROD_OVERFLOW(&info->ring, rp)) {
> +		scsifront_set_error(info, "illegal number of responses");


In net and block drivers we report number of such responses. (But not in usb)


> +		return 0;
> +	}
>   	for (i = info->ring.rsp_cons; i != rp; i++) {
> -		ring_rsp = RING_GET_RESPONSE(&info->ring, i);
> -		scsifront_do_response(info, ring_rsp);
> +		RING_COPY_RESPONSE(&info->ring, i, &ring_rsp);
> +		scsifront_do_response(info, &ring_rsp);
> +		if (info->host_active == STATE_ERROR)
> +			return 0;
> +		*eoiflag = 0;


*eoiflags &= ~XEN_EOI_FLAG_SPURIOUS; ?


We also use eoi_flags name in other instances in this file.


-boris