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;
Date:   Tue, 13 Jul 2021 14:31:14 +0300
From:   Dan Carpenter <dan.carpenter@oracle.com>
To:     Xie Yongji <xieyongji@bytedance.com>
Cc:     mst@redhat.com, jasowang@redhat.com, stefanha@redhat.com,
        sgarzare@redhat.com, parav@nvidia.com, hch@infradead.org,
        christian.brauner@canonical.com, rdunlap@infradead.org,
        willy@infradead.org, viro@zeniv.linux.org.uk, axboe@kernel.dk,
        bcrl@kvack.org, corbet@lwn.net, mika.penttila@nextfour.com,
        joro@8bytes.org, gregkh@linuxfoundation.org, zhe.he@windriver.com,
        xiaodong.liu@intel.com, songmuchun@bytedance.com,
        virtualization@lists.linux-foundation.org, netdev@vger.kernel.org,
        kvm@vger.kernel.org, linux-fsdevel@vger.kernel.org,
        iommu@lists.linux-foundation.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v9 13/17] vdpa: factor out vhost_vdpa_pa_map() and
 vhost_vdpa_pa_unmap()
Message-ID: <20210713113114.GL1954@kadam>
References: <20210713084656.232-1-xieyongji@bytedance.com>
 <20210713084656.232-14-xieyongji@bytedance.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20210713084656.232-14-xieyongji@bytedance.com>
User-Agent: Mutt/1.9.4 (2018-02-28)
MIME-Version: 1.0
X-MS-Exchange-MessageSentRepresentingType: 1
Received: from kadam (102.222.70.252) by JNXP275CA0002.ZAFP275.PROD.OUTLOOK.COM (2603:1086:0:19::14) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4308.19 via Frontend Transport; Tue, 13 Jul 2021 11:31:29 +0000
X-MS-PublicTrafficType: Email
X-MS-Office365-Filtering-Correlation-Id: 1486f005-c550-4f4c-e5c7-08d945f1ca5a
X-MS-TrafficTypeDiagnostic: MWHPR10MB1998:
X-MS-Exchange-Transport-Forked: True
X-Microsoft-Antispam-PRVS: <MWHPR10MB1998E0A34923347C04F97C208E149@MWHPR10MB1998.namprd10.prod.outlook.com>
X-MS-Oob-TLC-OOBClassifiers: OLM:8273;
X-MS-Exchange-SenderADCheck: 1
X-Microsoft-Antispam: BCL:0;
X-Microsoft-Antispam-Message-Info: KyGf1+qMiqbMaCqElCTxg5vu421hIQ3v8NiO6khHpQSSJgPudmhfh0TmRGbqivTP/LIAun65G4wFrWfG3xNg7IN/Bx18oUw7eeFwo6YCPJkS4l5gzm+1uG7WOgf6bx28pab4QRdj9vbf3NWeuLKcSXb+5Q/KMmg4zozz7OJpXDnYLgG1L0X9hxSyntND1s1RcgKKd3q8LhtJGhg3CKzDc/5XiqlOVhR4Gy85jFFCkdimVVbRCZyNydYV4u+/+bwRspH3ar55/l4dH/caOtXT3tss4tvCb6f5yb3XGHWVeK/ybID1SvFrowirAJdiWJbTh6DdTglUdjpQBileQItWLfPt9IDNV3Y932Dw/KD5sah+dOmssSYYTScNFFGUHon3OhYkFf+nimwqMx/onR1PW2i8MHAafDdK0uvSFXrCcN7+q8b1hVfFukAvCYqUNz43NwLALuzwW6FLXZNVEL396tB/fPWJcWZBqRLgTWQSi0hO8GbiNhsI/fD9MD+OcDXspEUUPBbMAoRGRu6vPcl+1YHL/o/s56Nd074mVm3cB5jm/aM0UvcdLARYPemwTERqN4P+zXZjg/Ayjsk9cBgxc2iIiV0mvocmgkDYWGbKJMKh79dVhnrYklCItvgZLfL+UJNT1cCYTUuws/Pik0kEC935X0/l2GiF7Q41KO3auYecQoSGRk28Dw7EqNJudgIR2yH2rhS3aJoJAO0OuefJ7g==
X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:MWHPR1001MB2365.namprd10.prod.outlook.com;PTR:;CAT:NONE;SFS:(346002)(136003)(396003)(366004)(376002)(39860400002)(8676002)(956004)(83380400001)(1076003)(8936002)(66476007)(55016002)(66556008)(86362001)(2906002)(44832011)(9686003)(478600001)(33716001)(26005)(38100700002)(7416002)(4326008)(6916009)(33656002)(52116002)(316002)(38350700002)(5660300002)(66946007)(6666004)(9576002)(186003)(6496006);DIR:OUT;SFP:1101;
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?a+UE+ULTtPQNxdLminwpzRe6DS6/AIyfXQ7Py2/PJnaUm6yPjN+01YX5hrgJ?=
 =?us-ascii?Q?mdWoXHgNOV0Xmz5gYMxZbRs70DQX+tfjVvcBtYJdvynu+gzVELqyMKKJU4Sa?=
 =?us-ascii?Q?rOIhtrhYlUfWSB1kgKlbviMLvDTpa+W3sx3IBheHxJyxFY0WUTlVMUyadncM?=
 =?us-ascii?Q?K90nxzOXu5Oz4ttl/YB6n/UJCi5UUymViQIf4sp3RKUkzmpHyC3k0LsHW6cr?=
 =?us-ascii?Q?Ka2F4CjLcq90LjB5ucz8dcNqd69AQL88Ovaudkmk94WQX6WeZ/jOqNOGgiQU?=
 =?us-ascii?Q?h6+Hu6W5cFOizAm/XqyzUB8RDY1Mzn/iRPhvVQxvkfbxrQEF4ymd5vu6OzHA?=
 =?us-ascii?Q?d+ULsBo6h3LtyuI8SWjMTAxLplvpqOkAZYMJQTbB10Sxc48rD2ULYda81yMV?=
 =?us-ascii?Q?Vw8vt1pjb8TLOxu4SXTBywRI7194TV46Ox5wtB2ZMvJ2BIORsjnnHcWT9yFd?=
 =?us-ascii?Q?UxqbcUHNqdGh+dUzt0V3OmqyQ/r6VmfSpoNwUKWZwl7fBUSXT5vf4C13GoNc?=
 =?us-ascii?Q?T/h9Q0A3bXtTFtgEm7DD3i8BZJauTSaMH6iiu9Gujo/iwek7x47f6KQV4AFJ?=
 =?us-ascii?Q?ysIcLOHk4p9cPTkai6RXTij126eFXdZuD4ZpHHBVv8OWZwPupbnBbvtTArrU?=
 =?us-ascii?Q?ep7/UU+NTNf5xZ3UP880GlLpjrFgR6HttRiVIlmwXgGZ+KvJHsPoKSFh8jAY?=
 =?us-ascii?Q?7j8hpJGO9IoSHEOofHIP1R5w6aGBI9EGBL0Ix2/Xbn+YL3uIHK9e92LYXuUo?=
 =?us-ascii?Q?k/oEEYMpslnuwXBDFnhOpwKw9M4M8EU4bC54nnTvmSWoVAsrXa6fOBo+wKYv?=
 =?us-ascii?Q?kRoEVxLByetl3jEp0ggtH+AnkWwmE4vpr6CW1jSqZY/kM/SWpjA7+o3wE4QI?=
 =?us-ascii?Q?2qBetV1umrOTn0IXbt4ZFy2V+DeZXddGq9h+HuCueNo41fvcFzb2/NVI/uLO?=
 =?us-ascii?Q?G1R32Pimw05/23Wt6n5xSXZb3Fh6J4lwP7CfhA4RlmBxYHJV9IE6rzMEhGO/?=
 =?us-ascii?Q?UY6o9HvkldshYMpZ0eQjPjfolMAnaKt2AzBLnNYyA3lz3/LuV8FhNl1+3Fut?=
 =?us-ascii?Q?QBBcJ2BYB5nl5k/UDl1est9o/DSaYZLdxGBkk+JjUPE6HH3WZJXawqXlb/Nh?=
 =?us-ascii?Q?q10UvfnjPlT4jVt0oV4u4RrElYHU3TW2zNvUu8Ozt5QgUtxuI7IAQSKm2NXH?=
 =?us-ascii?Q?D0SrD8sTBnhUIQ0u/wyIC8LVIXQPEniT+CixwxUres9Z61FqhK/Sm9HvFY8O?=
 =?us-ascii?Q?+sPQcc8aC8Sf3Lreli/PftMkaVsInjtDosaPQTwNZod8xZIQLJJQur2AauvI?=
 =?us-ascii?Q?+8PebxTAydQB7bB3Fh8em+eZ?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 1486f005-c550-4f4c-e5c7-08d945f1ca5a
X-MS-Exchange-CrossTenant-AuthSource: MWHPR1001MB2365.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 13 Jul 2021 11:31:43.9262
 (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: 2RmxDmY9elJT+RvfP6Ppo6dX3NHbah61r4BCrdG1CS6+l888E7Bd2vHz+ZlQzEYgaH0ZSPOIm879QgoowmtBwXMoBwvdFrnN6g+mWORwcps=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MWHPR10MB1998
X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=10043 signatures=668682
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 phishscore=0 bulkscore=0 malwarescore=0
 spamscore=0 suspectscore=0 adultscore=0 mlxlogscore=837 mlxscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2104190000
 definitions=main-2107130073
X-Proofpoint-ORIG-GUID: ufjBCmPPwrf3ypIeGr2TuZm9onNmXi17
X-Proofpoint-GUID: ufjBCmPPwrf3ypIeGr2TuZm9onNmXi17
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Tue, Jul 13, 2021 at 04:46:52PM +0800, Xie Yongji wrote:
> @@ -613,37 +618,28 @@ static void vhost_vdpa_unmap(struct vhost_vdpa *v, u64 iova, u64 size)
>  	}
>  }
>  
> -static int vhost_vdpa_process_iotlb_update(struct vhost_vdpa *v,
> -					   struct vhost_iotlb_msg *msg)
> +static int vhost_vdpa_pa_map(struct vhost_vdpa *v,
> +			     u64 iova, u64 size, u64 uaddr, u32 perm)
>  {
>  	struct vhost_dev *dev = &v->vdev;
> -	struct vhost_iotlb *iotlb = dev->iotlb;
>  	struct page **page_list;
>  	unsigned long list_size = PAGE_SIZE / sizeof(struct page *);
>  	unsigned int gup_flags = FOLL_LONGTERM;
>  	unsigned long npages, cur_base, map_pfn, last_pfn = 0;
>  	unsigned long lock_limit, sz2pin, nchunks, i;
> -	u64 iova = msg->iova;
> +	u64 start = iova;
>  	long pinned;
>  	int ret = 0;
>  
> -	if (msg->iova < v->range.first ||
> -	    msg->iova + msg->size - 1 > v->range.last)
> -		return -EINVAL;

This is not related to your patch, but can the "msg->iova + msg->size"
addition can have an integer overflow.  From looking at the callers it
seems like it can.  msg comes from:
  vhost_chr_write_iter()
  --> dev->msg_handler(dev, &msg);
      --> vhost_vdpa_process_iotlb_msg()
         --> vhost_vdpa_process_iotlb_update()

If I'm thinking of the right thing then these are allowed to overflow to
0 because of the " - 1" but not further than that.  I believe the check
needs to be something like:

	if (msg->iova < v->range.first ||
	    msg->iova - 1 > U64_MAX - msg->size ||
	    msg->iova + msg->size - 1 > v->range.last)

But writing integer overflow check correctly is notoriously difficult.
Do you think you could send a fix for that which is separate from the
patcheset?  We'd want to backport it to stable.

regards,
dan carpenter