From nobody Sun Apr 28 21:51:47 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of lists.xenproject.org designates 192.237.175.120 as permitted sender) client-ip=192.237.175.120; envelope-from=xen-devel-bounces@lists.xenproject.org; helo=lists.xenproject.org; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of lists.xenproject.org designates 192.237.175.120 as permitted sender) smtp.mailfrom=xen-devel-bounces@lists.xenproject.org; dmarc=pass(p=quarantine dis=none) header.from=suse.com ARC-Seal: i=1; a=rsa-sha256; t=1625748260; cv=none; d=zohomail.com; s=zohoarc; b=HfeBVRNpv6P7bqO32nvU5dHT49iornQcJnGLtQ+HO6TENv/8e9Xntyqkch8orJYxNUwgyNZah/nP6QucHfqWNtR1IA4e0yIBvxl6t1iPpMO9FG37/2OK20v5Y5dBgVvdcbsB2SXNQxhS6c3Sqgk9KglQXVjJMyBKPt+sUj8S1bc= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1625748260; h=Content-Transfer-Encoding:Cc:Date:From:In-Reply-To:List-Subscribe:List-Post:List-Id:List-Help:List-Unsubscribe:MIME-Version:Message-ID:References:Sender:Subject:To; bh=6XFRHPnaBepuoc+g0wkn7qafVkVXfKovIiQKKuCigY0=; b=SICr6VOoGtcQB0t4qnmKISPt6Otw7sPUAZL5TiCaulJsLfKsfaa60Y/nMdaZS4xK4B3hDPK+dPaLKqijiKejG6GAARfzPETglIWV4GQElihO9yS2ZFbFw7ZJRMMfK6YnaDkh4EpHmChyZ/y063+lo+ZWi7N9wASxwq92z6ICM98= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of lists.xenproject.org designates 192.237.175.120 as permitted sender) smtp.mailfrom=xen-devel-bounces@lists.xenproject.org; dmarc=pass header.from= (p=quarantine dis=none) Return-Path: Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) by mx.zohomail.com with SMTPS id 1625748260332376.7870280668991; Thu, 8 Jul 2021 05:44:20 -0700 (PDT) Received: from list by lists.xenproject.org with outflank-mailman.153072.282809 (Exim 4.92) (envelope-from ) id 1m1TNu-0006R1-Vn; Thu, 08 Jul 2021 12:44:06 +0000 Received: by outflank-mailman (output) from mailman id 153072.282809; Thu, 08 Jul 2021 12:44:06 +0000 Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1m1TNu-0006Qq-RH; Thu, 08 Jul 2021 12:44:06 +0000 Received: by outflank-mailman (input) for mailman id 153072; Thu, 08 Jul 2021 12:44:05 +0000 Received: from us1-rack-iad1.inumbo.com ([172.99.69.81]) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1m1TNt-0005Vi-Gv for xen-devel@lists.xenproject.org; Thu, 08 Jul 2021 12:44:05 +0000 Received: from smtp-out1.suse.de (unknown [195.135.220.28]) by us1-rack-iad1.inumbo.com (Halon) with ESMTPS id 42a12c1b-72d6-410e-91d3-2d6cb7fc8d93; Thu, 08 Jul 2021 12:43:54 +0000 (UTC) Received: from imap1.suse-dmz.suse.de (imap1.suse-dmz.suse.de [192.168.254.73]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id B74A62196E; Thu, 8 Jul 2021 12:43:53 +0000 (UTC) Received: from imap1.suse-dmz.suse.de (imap1.suse-dmz.suse.de [192.168.254.73]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap1.suse-dmz.suse.de (Postfix) with ESMTPS id 74C2912FF6; Thu, 8 Jul 2021 12:43:53 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap1.suse-dmz.suse.de with ESMTPSA id EARJGwnz5mCCYAAAGKfGzw (envelope-from ); Thu, 08 Jul 2021 12:43:53 +0000 X-Outflank-Mailman: Message body and most headers restored to incoming version X-BeenThere: xen-devel@lists.xenproject.org List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xenproject.org Precedence: list Sender: "Xen-devel" X-Inumbo-ID: 42a12c1b-72d6-410e-91d3-2d6cb7fc8d93 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1625748233; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=6XFRHPnaBepuoc+g0wkn7qafVkVXfKovIiQKKuCigY0=; b=BXFPPPGLMorJwQJ/GBAcYquuPoBZv0JZuWqXHV8Uakpm4PYoIFYsrVZaXWXbkIe5qsl0d2 moJj/mx6I15y4Zg+tYjoRLtiw/NSzyzKZN7QTWu0Wv936j8y3qEQ+YbeEsxVJyIy7y60m9 3wbGtStlqlhO9oeX43eCEorvroh4ff4= From: Juergen Gross To: xen-devel@lists.xenproject.org, linux-block@vger.kernel.org, linux-kernel@vger.kernel.org Cc: Juergen Gross , Boris Ostrovsky , Stefano Stabellini , Konrad Rzeszutek Wilk , =?UTF-8?q?Roger=20Pau=20Monn=C3=A9?= , Jens Axboe , Jan Beulich Subject: [PATCH v2 1/3] xen/blkfront: read response from backend only once Date: Thu, 8 Jul 2021 14:43:43 +0200 Message-Id: <20210708124345.10173-2-jgross@suse.com> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20210708124345.10173-1-jgross@suse.com> References: <20210708124345.10173-1-jgross@suse.com> MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-ZohoMail-DKIM: pass (identity @suse.com) X-ZM-MESSAGEID: 1625748261774100001 Content-Type: text/plain; charset="utf-8" In order to avoid problems in case the backend is modifying a response on the ring page while the frontend has already seen it, just read the response into a local buffer in one go and then operate on that buffer only. Signed-off-by: Juergen Gross Reviewed-by: Jan Beulich Acked-by: Roger Pau Monn=C3=A9 --- drivers/block/xen-blkfront.c | 35 ++++++++++++++++++----------------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c index 8d49f8fa98bb..86356014d35e 100644 --- a/drivers/block/xen-blkfront.c +++ b/drivers/block/xen-blkfront.c @@ -1539,7 +1539,7 @@ static bool blkif_completion(unsigned long *id, static irqreturn_t blkif_interrupt(int irq, void *dev_id) { struct request *req; - struct blkif_response *bret; + struct blkif_response bret; RING_IDX i, rp; unsigned long flags; struct blkfront_ring_info *rinfo =3D (struct blkfront_ring_info *)dev_id; @@ -1556,8 +1556,9 @@ static irqreturn_t blkif_interrupt(int irq, void *dev= _id) for (i =3D rinfo->ring.rsp_cons; i !=3D rp; i++) { unsigned long id; =20 - bret =3D RING_GET_RESPONSE(&rinfo->ring, i); - id =3D bret->id; + RING_COPY_RESPONSE(&rinfo->ring, i, &bret); + id =3D bret.id; + /* * The backend has messed up and given us an id that we would * never have given to it (we stamp it up to BLK_RING_SIZE - @@ -1565,39 +1566,39 @@ static irqreturn_t blkif_interrupt(int irq, void *d= ev_id) */ if (id >=3D BLK_RING_SIZE(info)) { WARN(1, "%s: response to %s has incorrect id (%ld)\n", - info->gd->disk_name, op_name(bret->operation), id); + info->gd->disk_name, op_name(bret.operation), id); /* We can't safely get the 'struct request' as * the id is busted. */ continue; } req =3D rinfo->shadow[id].request; =20 - if (bret->operation !=3D BLKIF_OP_DISCARD) { + if (bret.operation !=3D BLKIF_OP_DISCARD) { /* * We may need to wait for an extra response if the * I/O request is split in 2 */ - if (!blkif_completion(&id, rinfo, bret)) + if (!blkif_completion(&id, rinfo, &bret)) continue; } =20 if (add_id_to_freelist(rinfo, id)) { WARN(1, "%s: response to %s (id %ld) couldn't be recycled!\n", - info->gd->disk_name, op_name(bret->operation), id); + info->gd->disk_name, op_name(bret.operation), id); continue; } =20 - if (bret->status =3D=3D BLKIF_RSP_OKAY) + if (bret.status =3D=3D BLKIF_RSP_OKAY) blkif_req(req)->error =3D BLK_STS_OK; else blkif_req(req)->error =3D BLK_STS_IOERR; =20 - switch (bret->operation) { + switch (bret.operation) { case BLKIF_OP_DISCARD: - if (unlikely(bret->status =3D=3D BLKIF_RSP_EOPNOTSUPP)) { + if (unlikely(bret.status =3D=3D BLKIF_RSP_EOPNOTSUPP)) { struct request_queue *rq =3D info->rq; printk(KERN_WARNING "blkfront: %s: %s op failed\n", - info->gd->disk_name, op_name(bret->operation)); + info->gd->disk_name, op_name(bret.operation)); blkif_req(req)->error =3D BLK_STS_NOTSUPP; info->feature_discard =3D 0; info->feature_secdiscard =3D 0; @@ -1607,15 +1608,15 @@ static irqreturn_t blkif_interrupt(int irq, void *d= ev_id) break; case BLKIF_OP_FLUSH_DISKCACHE: case BLKIF_OP_WRITE_BARRIER: - if (unlikely(bret->status =3D=3D BLKIF_RSP_EOPNOTSUPP)) { + if (unlikely(bret.status =3D=3D BLKIF_RSP_EOPNOTSUPP)) { printk(KERN_WARNING "blkfront: %s: %s op failed\n", - info->gd->disk_name, op_name(bret->operation)); + info->gd->disk_name, op_name(bret.operation)); blkif_req(req)->error =3D BLK_STS_NOTSUPP; } - if (unlikely(bret->status =3D=3D BLKIF_RSP_ERROR && + if (unlikely(bret.status =3D=3D BLKIF_RSP_ERROR && rinfo->shadow[id].req.u.rw.nr_segments =3D=3D 0)) { printk(KERN_WARNING "blkfront: %s: empty %s op failed\n", - info->gd->disk_name, op_name(bret->operation)); + info->gd->disk_name, op_name(bret.operation)); blkif_req(req)->error =3D BLK_STS_NOTSUPP; } if (unlikely(blkif_req(req)->error)) { @@ -1628,9 +1629,9 @@ static irqreturn_t blkif_interrupt(int irq, void *dev= _id) fallthrough; case BLKIF_OP_READ: case BLKIF_OP_WRITE: - if (unlikely(bret->status !=3D BLKIF_RSP_OKAY)) + if (unlikely(bret.status !=3D BLKIF_RSP_OKAY)) dev_dbg(&info->xbdev->dev, "Bad return from blkdev data " - "request: %x\n", bret->status); + "request: %x\n", bret.status); =20 break; default: --=20 2.26.2 From nobody Sun Apr 28 21:51:47 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of lists.xenproject.org designates 192.237.175.120 as permitted sender) client-ip=192.237.175.120; envelope-from=xen-devel-bounces@lists.xenproject.org; helo=lists.xenproject.org; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of lists.xenproject.org designates 192.237.175.120 as permitted sender) smtp.mailfrom=xen-devel-bounces@lists.xenproject.org; dmarc=pass(p=quarantine dis=none) header.from=suse.com ARC-Seal: i=1; a=rsa-sha256; t=1625748257; cv=none; d=zohomail.com; s=zohoarc; b=mTnP6m6CSxyjrSMYqD/doSQnyaeS7To0m92g/d0vp1oTxJ9IDZ56FRWe8eXzCMwFC1fWYMM8zpz2ZXuvYNDHCrEw89PW0qsJvX3fUxk359lD5GVARNrzWIAo1CYsMvvJGdPsQzivzKmgo9Vyk9dRAbpXUMol8f/JSk7nmgMetsI= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1625748257; h=Content-Transfer-Encoding:Cc:Date:From:In-Reply-To:List-Subscribe:List-Post:List-Id:List-Help:List-Unsubscribe:MIME-Version:Message-ID:References:Sender:Subject:To; bh=P4npa89ZahTLG6OBPNclvRbrcVBcLwXWzLAhBZSh1SY=; b=FB2O/65Ya0+yahqfLg6Y20+wC8Hb2i9bl7gKn1V5gKNj4IwskIDoO3/DsMFIA0/3Em5xojQE3oQxVtUkP84/e9orBk08WF38vCCdIcxWW7KM2TcuyZ/TRN4RpzHbyjk8r7dwkk7MRB19Uk0V0wJBbR8oLJIu7obYZMet3Zom7RA= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of lists.xenproject.org designates 192.237.175.120 as permitted sender) smtp.mailfrom=xen-devel-bounces@lists.xenproject.org; dmarc=pass header.from= (p=quarantine dis=none) Return-Path: Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) by mx.zohomail.com with SMTPS id 1625748257914746.776386607181; Thu, 8 Jul 2021 05:44:17 -0700 (PDT) Received: from list by lists.xenproject.org with outflank-mailman.153071.282798 (Exim 4.92) (envelope-from ) id 1m1TNp-00064Y-K2; Thu, 08 Jul 2021 12:44:01 +0000 Received: by outflank-mailman (output) from mailman id 153071.282798; Thu, 08 Jul 2021 12:44:01 +0000 Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1m1TNp-00064R-Gf; Thu, 08 Jul 2021 12:44:01 +0000 Received: by outflank-mailman (input) for mailman id 153071; Thu, 08 Jul 2021 12:44:00 +0000 Received: from us1-rack-iad1.inumbo.com ([172.99.69.81]) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1m1TNo-0005Vi-Gc for xen-devel@lists.xenproject.org; Thu, 08 Jul 2021 12:44:00 +0000 Received: from smtp-out1.suse.de (unknown [195.135.220.28]) by us1-rack-iad1.inumbo.com (Halon) with ESMTPS id 2d8c5b08-7436-465e-b966-aadad158685b; Thu, 08 Jul 2021 12:43:54 +0000 (UTC) Received: from imap1.suse-dmz.suse.de (imap1.suse-dmz.suse.de [192.168.254.73]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id 0C3DB2198B; Thu, 8 Jul 2021 12:43:54 +0000 (UTC) Received: from imap1.suse-dmz.suse.de (imap1.suse-dmz.suse.de [192.168.254.73]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap1.suse-dmz.suse.de (Postfix) with ESMTPS id BC119133FE; Thu, 8 Jul 2021 12:43:53 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap1.suse-dmz.suse.de with ESMTPSA id ADi5LAnz5mCCYAAAGKfGzw (envelope-from ); Thu, 08 Jul 2021 12:43:53 +0000 X-Outflank-Mailman: Message body and most headers restored to incoming version X-BeenThere: xen-devel@lists.xenproject.org List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xenproject.org Precedence: list Sender: "Xen-devel" X-Inumbo-ID: 2d8c5b08-7436-465e-b966-aadad158685b DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1625748234; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=P4npa89ZahTLG6OBPNclvRbrcVBcLwXWzLAhBZSh1SY=; b=kQxNjstg82kL4O4ZbGnRoUnxhb1jfiG/cNcdtcWx6GOnVEHDD/BIvDQn1QL50wL2g7eREQ KqL6OG17bCJJPC/EAF6ca9HUJFjNj1LV9M3DbH3D37UtCY2ggjLGBQkostZ7F+AZ7jDqdi 6YlCyZaiFi82VevtT4x4ee0iMGO5qiQ= From: Juergen Gross To: xen-devel@lists.xenproject.org, linux-block@vger.kernel.org, linux-kernel@vger.kernel.org Cc: Juergen Gross , Konrad Rzeszutek Wilk , =?UTF-8?q?Roger=20Pau=20Monn=C3=A9?= , Boris Ostrovsky , Stefano Stabellini , Jens Axboe , Jan Beulich Subject: [PATCH v2 2/3] xen/blkfront: don't take local copy of a request from the ring page Date: Thu, 8 Jul 2021 14:43:44 +0200 Message-Id: <20210708124345.10173-3-jgross@suse.com> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20210708124345.10173-1-jgross@suse.com> References: <20210708124345.10173-1-jgross@suse.com> MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-ZohoMail-DKIM: pass (identity @suse.com) X-ZM-MESSAGEID: 1625748259149100003 Content-Type: text/plain; charset="utf-8" In order to avoid a malicious backend being able to influence the local copy of a request build the request locally first and then copy it to the ring page instead of doing it the other way round as today. Signed-off-by: Juergen Gross Reviewed-by: Jan Beulich Acked-by: Roger Pau Monn=C3=A9 --- V2: - init variable to avoid potential compiler warning (Jan Beulich) --- drivers/block/xen-blkfront.c | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c index 86356014d35e..80701860870a 100644 --- a/drivers/block/xen-blkfront.c +++ b/drivers/block/xen-blkfront.c @@ -546,7 +546,7 @@ static unsigned long blkif_ring_get_request(struct blkf= ront_ring_info *rinfo, rinfo->shadow[id].status =3D REQ_WAITING; rinfo->shadow[id].associated_id =3D NO_ASSOCIATED_ID; =20 - (*ring_req)->u.rw.id =3D id; + rinfo->shadow[id].req.u.rw.id =3D id; =20 return id; } @@ -554,11 +554,12 @@ static unsigned long blkif_ring_get_request(struct bl= kfront_ring_info *rinfo, static int blkif_queue_discard_req(struct request *req, struct blkfront_ri= ng_info *rinfo) { struct blkfront_info *info =3D rinfo->dev_info; - struct blkif_request *ring_req; + struct blkif_request *ring_req, *final_ring_req; unsigned long id; =20 /* Fill out a communications ring structure. */ - id =3D blkif_ring_get_request(rinfo, req, &ring_req); + id =3D blkif_ring_get_request(rinfo, req, &final_ring_req); + ring_req =3D &rinfo->shadow[id].req; =20 ring_req->operation =3D BLKIF_OP_DISCARD; ring_req->u.discard.nr_sectors =3D blk_rq_sectors(req); @@ -569,8 +570,8 @@ static int blkif_queue_discard_req(struct request *req,= struct blkfront_ring_inf else ring_req->u.discard.flag =3D 0; =20 - /* Keep a private copy so we can reissue requests when recovering. */ - rinfo->shadow[id].req =3D *ring_req; + /* Copy the request to the ring page. */ + *final_ring_req =3D *ring_req; =20 return 0; } @@ -703,6 +704,7 @@ static int blkif_queue_rw_req(struct request *req, stru= ct blkfront_ring_info *ri { struct blkfront_info *info =3D rinfo->dev_info; struct blkif_request *ring_req, *extra_ring_req =3D NULL; + struct blkif_request *final_ring_req, *final_extra_ring_req =3D NULL; unsigned long id, extra_id =3D NO_ASSOCIATED_ID; bool require_extra_req =3D false; int i; @@ -747,7 +749,8 @@ static int blkif_queue_rw_req(struct request *req, stru= ct blkfront_ring_info *ri } =20 /* Fill out a communications ring structure. */ - id =3D blkif_ring_get_request(rinfo, req, &ring_req); + id =3D blkif_ring_get_request(rinfo, req, &final_ring_req); + ring_req =3D &rinfo->shadow[id].req; =20 num_sg =3D blk_rq_map_sg(req->q, req, rinfo->shadow[id].sg); num_grant =3D 0; @@ -798,7 +801,9 @@ static int blkif_queue_rw_req(struct request *req, stru= ct blkfront_ring_info *ri ring_req->u.rw.nr_segments =3D num_grant; if (unlikely(require_extra_req)) { extra_id =3D blkif_ring_get_request(rinfo, req, - &extra_ring_req); + &final_extra_ring_req); + extra_ring_req =3D &rinfo->shadow[extra_id].req; + /* * Only the first request contains the scatter-gather * list. @@ -840,10 +845,10 @@ static int blkif_queue_rw_req(struct request *req, st= ruct blkfront_ring_info *ri if (setup.segments) kunmap_atomic(setup.segments); =20 - /* Keep a private copy so we can reissue requests when recovering. */ - rinfo->shadow[id].req =3D *ring_req; + /* Copy request(s) to the ring page. */ + *final_ring_req =3D *ring_req; if (unlikely(require_extra_req)) - rinfo->shadow[extra_id].req =3D *extra_ring_req; + *final_extra_ring_req =3D *extra_ring_req; =20 if (new_persistent_gnts) gnttab_free_grant_references(setup.gref_head); --=20 2.26.2 From nobody Sun Apr 28 21:51:47 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of lists.xenproject.org designates 192.237.175.120 as permitted sender) client-ip=192.237.175.120; envelope-from=xen-devel-bounces@lists.xenproject.org; helo=lists.xenproject.org; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of lists.xenproject.org designates 192.237.175.120 as permitted sender) smtp.mailfrom=xen-devel-bounces@lists.xenproject.org; dmarc=pass(p=quarantine dis=none) header.from=suse.com ARC-Seal: i=1; a=rsa-sha256; t=1625748257; cv=none; d=zohomail.com; s=zohoarc; b=W0EMYNiBV6dIkbKuyjdPgp5f2c5U/bgu6VlhmxpfSp+djcwrpa9dBX6U88J+YZLElL0P0KdDy7m9g/EWr+23GVc05Ge31hcxWNmjLTgMUssRmTeJ5jGyV/BpBba4xDVcwVO6qFg7+uutiXgNi8u+iN5/m2tR/Dbyc3JobFnrhWc= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1625748257; h=Content-Transfer-Encoding:Cc:Date:From:In-Reply-To:List-Subscribe:List-Post:List-Id:List-Help:List-Unsubscribe:MIME-Version:Message-ID:References:Sender:Subject:To; bh=z4HBJ9j3raPc5OQynJOO5Jq31LuNuXGgUK0tD1yAAz0=; b=U8k284UsrAjn8D2PWgWGqxvxguatdPOKz67lgbmpNK4j8mPT7wznIy1xjhv9+Pce0DR7OWXhwH/dJK5JTQiJyT28eBcy1At98NPN5SY1zlycrl+mhHaKpVsiFz7GVlWEseklMgPURINOjInFQMCDs45XOAbEyWlIDGfVSgRikW8= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of lists.xenproject.org designates 192.237.175.120 as permitted sender) smtp.mailfrom=xen-devel-bounces@lists.xenproject.org; dmarc=pass header.from= (p=quarantine dis=none) Return-Path: Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) by mx.zohomail.com with SMTPS id 1625748257154482.7544528523846; Thu, 8 Jul 2021 05:44:17 -0700 (PDT) Received: from list by lists.xenproject.org with outflank-mailman.153070.282787 (Exim 4.92) (envelope-from ) id 1m1TNm-0005mZ-A5; Thu, 08 Jul 2021 12:43:58 +0000 Received: by outflank-mailman (output) from mailman id 153070.282787; Thu, 08 Jul 2021 12:43:58 +0000 Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1m1TNm-0005mS-6v; Thu, 08 Jul 2021 12:43:58 +0000 Received: by outflank-mailman (input) for mailman id 153070; Thu, 08 Jul 2021 12:43:56 +0000 Received: from all-amaz-eas1.inumbo.com ([34.197.232.57] helo=us1-amaz-eas2.inumbo.com) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1m1TNk-0005Vo-J4 for xen-devel@lists.xenproject.org; Thu, 08 Jul 2021 12:43:56 +0000 Received: from smtp-out2.suse.de (unknown [195.135.220.29]) by us1-amaz-eas2.inumbo.com (Halon) with ESMTPS id 279d68c0-dfea-11eb-856d-12813bfff9fa; Thu, 08 Jul 2021 12:43:55 +0000 (UTC) Received: from imap1.suse-dmz.suse.de (imap1.suse-dmz.suse.de [192.168.254.73]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id 4F8CB201BF; Thu, 8 Jul 2021 12:43:54 +0000 (UTC) Received: from imap1.suse-dmz.suse.de (imap1.suse-dmz.suse.de [192.168.254.73]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap1.suse-dmz.suse.de (Postfix) with ESMTPS id 1286D12FF6; Thu, 8 Jul 2021 12:43:54 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap1.suse-dmz.suse.de with ESMTPSA id 0PxXAwrz5mCCYAAAGKfGzw (envelope-from ); Thu, 08 Jul 2021 12:43:54 +0000 X-Outflank-Mailman: Message body and most headers restored to incoming version X-BeenThere: xen-devel@lists.xenproject.org List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xenproject.org Precedence: list Sender: "Xen-devel" X-Inumbo-ID: 279d68c0-dfea-11eb-856d-12813bfff9fa DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1625748234; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=z4HBJ9j3raPc5OQynJOO5Jq31LuNuXGgUK0tD1yAAz0=; b=sWvKQnWc94bZ1fP4UEJt71N35uFn7oCLbc0csxkGx4o+LVmDT2pyO7HqvbXaKCwahkZKxF MkPJTMcVDWZxdBgJLBnDIKZ71EDBp41dd/wd5oYFGaNJXpG1ziztI6xniHN1m8IXcMSpqa uBLOyVv5yXoOiwRfosNF4MCI2cpnyRI= From: Juergen Gross To: xen-devel@lists.xenproject.org, linux-block@vger.kernel.org, linux-kernel@vger.kernel.org Cc: Juergen Gross , Boris Ostrovsky , Stefano Stabellini , Konrad Rzeszutek Wilk , =?UTF-8?q?Roger=20Pau=20Monn=C3=A9?= , Jens Axboe Subject: [PATCH v2 3/3] xen/blkfront: don't trust the backend response data blindly Date: Thu, 8 Jul 2021 14:43:45 +0200 Message-Id: <20210708124345.10173-4-jgross@suse.com> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20210708124345.10173-1-jgross@suse.com> References: <20210708124345.10173-1-jgross@suse.com> MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-ZohoMail-DKIM: pass (identity @suse.com) X-ZM-MESSAGEID: 1625748259061100002 Content-Type: text/plain; charset="utf-8" Today blkfront will trust the backend to send only sane response data. In order to avoid privilege escalations or crashes in case of malicious backends verify the data to be within expected limits. Especially make sure that the response always references an outstanding request. Introduce a new state of the ring BLKIF_STATE_ERROR which will be switched to in case an inconsistency is being detected. Recovering from this state is possible only via removing and adding the virtual device again (e.g. via a suspend/resume cycle). Signed-off-by: Juergen Gross Reported-by: kernel test robot Reviewed-by: Jan Beulich --- V2: - use READ_ONCE() for reading the producer index - check validity of producer index only after memory barrier (Jan Beulich) - use virt_rmb() as barrier (Jan Beulich) --- drivers/block/xen-blkfront.c | 66 ++++++++++++++++++++++++++---------- 1 file changed, 49 insertions(+), 17 deletions(-) diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c index 80701860870a..ecdbb0381b4c 100644 --- a/drivers/block/xen-blkfront.c +++ b/drivers/block/xen-blkfront.c @@ -80,6 +80,7 @@ enum blkif_state { BLKIF_STATE_DISCONNECTED, BLKIF_STATE_CONNECTED, BLKIF_STATE_SUSPENDED, + BLKIF_STATE_ERROR, }; =20 struct grant { @@ -89,6 +90,7 @@ struct grant { }; =20 enum blk_req_status { + REQ_PROCESSING, REQ_WAITING, REQ_DONE, REQ_ERROR, @@ -543,7 +545,7 @@ static unsigned long blkif_ring_get_request(struct blkf= ront_ring_info *rinfo, =20 id =3D get_id_from_freelist(rinfo); rinfo->shadow[id].request =3D req; - rinfo->shadow[id].status =3D REQ_WAITING; + rinfo->shadow[id].status =3D REQ_PROCESSING; rinfo->shadow[id].associated_id =3D NO_ASSOCIATED_ID; =20 rinfo->shadow[id].req.u.rw.id =3D id; @@ -572,6 +574,7 @@ static int blkif_queue_discard_req(struct request *req,= struct blkfront_ring_inf =20 /* Copy the request to the ring page. */ *final_ring_req =3D *ring_req; + rinfo->shadow[id].status =3D REQ_WAITING; =20 return 0; } @@ -847,8 +850,11 @@ static int blkif_queue_rw_req(struct request *req, str= uct blkfront_ring_info *ri =20 /* Copy request(s) to the ring page. */ *final_ring_req =3D *ring_req; - if (unlikely(require_extra_req)) + rinfo->shadow[id].status =3D REQ_WAITING; + if (unlikely(require_extra_req)) { *final_extra_ring_req =3D *extra_ring_req; + rinfo->shadow[extra_id].status =3D REQ_WAITING; + } =20 if (new_persistent_gnts) gnttab_free_grant_references(setup.gref_head); @@ -1402,8 +1408,8 @@ static enum blk_req_status blkif_rsp_to_req_status(in= t rsp) static int blkif_get_final_status(enum blk_req_status s1, enum blk_req_status s2) { - BUG_ON(s1 =3D=3D REQ_WAITING); - BUG_ON(s2 =3D=3D REQ_WAITING); + BUG_ON(s1 < REQ_DONE); + BUG_ON(s2 < REQ_DONE); =20 if (s1 =3D=3D REQ_ERROR || s2 =3D=3D REQ_ERROR) return BLKIF_RSP_ERROR; @@ -1436,7 +1442,7 @@ static bool blkif_completion(unsigned long *id, s->status =3D blkif_rsp_to_req_status(bret->status); =20 /* Wait the second response if not yet here. */ - if (s2->status =3D=3D REQ_WAITING) + if (s2->status < REQ_DONE) return false; =20 bret->status =3D blkif_get_final_status(s->status, @@ -1555,11 +1561,17 @@ static irqreturn_t blkif_interrupt(int irq, void *d= ev_id) =20 spin_lock_irqsave(&rinfo->ring_lock, flags); again: - rp =3D rinfo->ring.sring->rsp_prod; - rmb(); /* Ensure we see queued responses up to 'rp'. */ + rp =3D READ_ONCE(rinfo->ring.sring->rsp_prod); + virt_rmb(); /* Ensure we see queued responses up to 'rp'. */ + if (RING_RESPONSE_PROD_OVERFLOW(&rinfo->ring, rp)) { + pr_alert("%s: illegal number of responses %u\n", + info->gd->disk_name, rp - rinfo->ring.rsp_cons); + goto err; + } =20 for (i =3D rinfo->ring.rsp_cons; i !=3D rp; i++) { unsigned long id; + unsigned int op; =20 RING_COPY_RESPONSE(&rinfo->ring, i, &bret); id =3D bret.id; @@ -1570,14 +1582,28 @@ static irqreturn_t blkif_interrupt(int irq, void *d= ev_id) * look in get_id_from_freelist. */ if (id >=3D BLK_RING_SIZE(info)) { - WARN(1, "%s: response to %s has incorrect id (%ld)\n", - info->gd->disk_name, op_name(bret.operation), id); - /* We can't safely get the 'struct request' as - * the id is busted. */ - continue; + pr_alert("%s: response has incorrect id (%ld)\n", + info->gd->disk_name, id); + goto err; } + if (rinfo->shadow[id].status !=3D REQ_WAITING) { + pr_alert("%s: response references no pending request\n", + info->gd->disk_name); + goto err; + } + + rinfo->shadow[id].status =3D REQ_PROCESSING; req =3D rinfo->shadow[id].request; =20 + op =3D rinfo->shadow[id].req.operation; + if (op =3D=3D BLKIF_OP_INDIRECT) + op =3D rinfo->shadow[id].req.u.indirect.indirect_op; + if (bret.operation !=3D op) { + pr_alert("%s: response has wrong operation (%u instead of %u)\n", + info->gd->disk_name, bret.operation, op); + goto err; + } + if (bret.operation !=3D BLKIF_OP_DISCARD) { /* * We may need to wait for an extra response if the @@ -1602,7 +1628,8 @@ static irqreturn_t blkif_interrupt(int irq, void *dev= _id) case BLKIF_OP_DISCARD: if (unlikely(bret.status =3D=3D BLKIF_RSP_EOPNOTSUPP)) { struct request_queue *rq =3D info->rq; - printk(KERN_WARNING "blkfront: %s: %s op failed\n", + + pr_warn_ratelimited("blkfront: %s: %s op failed\n", info->gd->disk_name, op_name(bret.operation)); blkif_req(req)->error =3D BLK_STS_NOTSUPP; info->feature_discard =3D 0; @@ -1614,13 +1641,13 @@ static irqreturn_t blkif_interrupt(int irq, void *d= ev_id) case BLKIF_OP_FLUSH_DISKCACHE: case BLKIF_OP_WRITE_BARRIER: if (unlikely(bret.status =3D=3D BLKIF_RSP_EOPNOTSUPP)) { - printk(KERN_WARNING "blkfront: %s: %s op failed\n", + pr_warn_ratelimited("blkfront: %s: %s op failed\n", info->gd->disk_name, op_name(bret.operation)); blkif_req(req)->error =3D BLK_STS_NOTSUPP; } if (unlikely(bret.status =3D=3D BLKIF_RSP_ERROR && rinfo->shadow[id].req.u.rw.nr_segments =3D=3D 0)) { - printk(KERN_WARNING "blkfront: %s: empty %s op failed\n", + pr_warn_ratelimited("blkfront: %s: empty %s op failed\n", info->gd->disk_name, op_name(bret.operation)); blkif_req(req)->error =3D BLK_STS_NOTSUPP; } @@ -1635,8 +1662,8 @@ static irqreturn_t blkif_interrupt(int irq, void *dev= _id) case BLKIF_OP_READ: case BLKIF_OP_WRITE: if (unlikely(bret.status !=3D BLKIF_RSP_OKAY)) - dev_dbg(&info->xbdev->dev, "Bad return from blkdev data " - "request: %x\n", bret.status); + dev_dbg_ratelimited(&info->xbdev->dev, + "Bad return from blkdev data request: %x\n", bret.status); =20 break; default: @@ -1662,6 +1689,11 @@ static irqreturn_t blkif_interrupt(int irq, void *de= v_id) spin_unlock_irqrestore(&rinfo->ring_lock, flags); =20 return IRQ_HANDLED; + + err: + info->connected =3D BLKIF_STATE_ERROR; + pr_alert("%s disabled for further use\n", info->gd->disk_name); + return IRQ_HANDLED; } =20 =20 --=20 2.26.2