Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 6 Jul 2022 08:51:24 +0300
From:   Dan Carpenter <dan.carpenter@oracle.com>
To:     Jason Gunthorpe <jgg@ziepe.ca>
Cc:     Longfang Liu <liulongfang@huawei.com>,
        Yishai Hadas <yishaih@nvidia.com>,
        Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>,
        Kevin Tian <kevin.tian@intel.com>,
        Alex Williamson <alex.williamson@redhat.com>,
        Cornelia Huck <cohuck@redhat.com>, kvm@vger.kernel.org,
        linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org
Subject: Re: [PATCH] vfio: hisi_acc_vfio_pci: fix integer overflow check in
 hisi_acc_vf_resume_write()
Message-ID: <20220706055124.GA2338@kadam>
References: <YsP+2CWqMudArkqF@kili>
 <20220705180649.GI23621@ziepe.ca>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220705180649.GI23621@ziepe.ca>
User-Agent: Mutt/1.9.4 (2018-02-28)
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?biuFbNpMcg6T8fOaaFNp6/Nhni+frgf7GhHGhlE6z7V5fXNxKPiXA4OuWZ1Z?=
 =?us-ascii?Q?bD1DYu7vjpHub2TV2xXTqAY8f+ozIz+O+BzvalCmcWiuekRqnL8H3TE3qRsP?=
 =?us-ascii?Q?f/9jcmiv5kJQknON3JKdjd/9EWbRqYe8U2mdCqrPucVbwqGrBJSB965i9FVs?=
 =?us-ascii?Q?mZtUjylUOpXAcMBokH5mthghZpIoHjnaEfJvahVTyEFXMnyK8gaHR5+LKCXr?=
 =?us-ascii?Q?/0kiQ+K0U85IOxqyk+E1AiEEcwMAqMWZP4rlCXWkgRK5i2uBrKonhQv1sgBm?=
 =?us-ascii?Q?Rp1yJwyIhq6ChHcoEq6I6HupbR8mnKZChV7S86dkEOWGxgZPrizNeM7lV+91?=
 =?us-ascii?Q?97jJH1mY+GYJpiBiGCY+dSTinKFNIuBqMguGs1SrPEr5xRcvH3ytwhzDLdi+?=
 =?us-ascii?Q?ulVcE+7Tr69qSPSRP2q7FNsm/mXSwfjKHll3WHk5NaR6shp8TDodl2nmyOoG?=
 =?us-ascii?Q?+Xtx482qcrM37vGUPyIHeO1eMOSFUwS0H7qe0LbbgJhHkmGbvzF852+dI3ff?=
 =?us-ascii?Q?CAKE5zXH88OBVKp7uuGo6IlII41CbXSQwICE5qCvDviBFpqQkWFvwo1pxUVa?=
 =?us-ascii?Q?WjqjuEcCRuinEplxD0dYyNze4nYpZSyA8Y6Is/+loqyHuqJe5NxSmByPZ5wA?=
 =?us-ascii?Q?TvJf+q63iKLuXRxoQFCT3iJnMNWmFnmbhLFLyKi2Csv/dnC4zdGqOzQXrVv2?=
 =?us-ascii?Q?232sIMIConRtpBDq8bDTQjjnq/rPX7uJiZVHc32GBWRlHBnTGvDn1GrIItSu?=
 =?us-ascii?Q?omvw1rV/cXiyuLlQ2OdIi2AO/b7xXfHemQJu1jQe5MiQ2R9AuyvhPbUdoxoB?=
 =?us-ascii?Q?JS/TFRcXg1PwCKavKHyfQqI6cq69HO9GYhnlETGkCtPyofUnxqXTd/JGF1LI?=
 =?us-ascii?Q?O0aZSoRo8tIzLwzzipDyytC2nzvv0ftXti+4P0qks/6TkcVp1yYI31DS8BPs?=
 =?us-ascii?Q?6CS/sK6+etjgXFNPO2Wr1rG1Rajrr43rtoRk+EPm3nfDrr6llyhzXnJ1E8CW?=
 =?us-ascii?Q?UdD0wZ46n37tMc80pBFZQjHCrN+Kl26I+iRRrOKdweEbmZ7guAiIc7nOiz9k?=
 =?us-ascii?Q?Rt2jHHTM5TM0Dkqa8ZXswIeeaqCdK+f+sQ0RRjj0oFjNLN4T4eWiePCvcxiF?=
 =?us-ascii?Q?nPdzoQ+LXVD49DGfgnfxsYezOLyGwFp2/c/kXQEFC30ExdYi0wyDX396x0A7?=
 =?us-ascii?Q?AoHlQZfQB1pabp7ELnUIit5RV7nQTD9c1sDyzLd8M+Y+4qVZTlsktftfSuUP?=
 =?us-ascii?Q?pVb/YB+D/DQzoHdiJsKj2uk8XHAs8n8cBAjEXNe9AUcCiQ86+NCxXWPNjALB?=
 =?us-ascii?Q?EPZ2/8lGyoXfKsvuHF5GWSqTv6axATp//KGyKJWFL4cqjw96z0iwXDNzrumc?=
 =?us-ascii?Q?yUFsvQLjZOk1exYKzkhHXsIBbNtTRKPPpg3eUmDcYrJB7njzRFY3D0Sj7oNy?=
 =?us-ascii?Q?4jk6iM2mvRWO5wO/Uz/k38fC8VVmEL1nZkKMlMUC2gc1+/qyrvXUvLppRGfU?=
 =?us-ascii?Q?UTEmVd27ebD/ff+WMblLyG+QAb6oFpKQKQhHAFQ7+Pg8ngeJ0Azw2pMYQE+p?=
 =?us-ascii?Q?u2n6AxceCNW7QuZ5IK3OWApdpVmfS67ODMKDK8LK?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: bf3e24ea-cef5-440b-a58d-08da5f139c12
X-MS-Exchange-CrossTenant-AuthSource: MWHPR1001MB2365.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 06 Jul 2022 05:51:45.8518
 (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: SB5Bnrke4epSiOdbXggOO1PBYN5YjujEETbJUHADiij5fk3egLt7hCKsYGi1bFBjgfxDx0k+wwiPr8m9CZOdQAcbPt0UJzUbTi9HROidDN8=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN6PR1001MB2226
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.517,18.0.883
 definitions=2022-07-06_02:2022-06-28,2022-07-06 signatures=0
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 bulkscore=0 suspectscore=0 spamscore=0
 mlxlogscore=999 phishscore=0 malwarescore=0 mlxscore=0 adultscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2206140000
 definitions=main-2207060021
X-Proofpoint-GUID: 1C3Rr0vS1IrZX-jQSSPDBsRBC8nEnvP-
X-Proofpoint-ORIG-GUID: 1C3Rr0vS1IrZX-jQSSPDBsRBC8nEnvP-
X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE
        autolearn=ham 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

On Tue, Jul 05, 2022 at 03:06:49PM -0300, Jason Gunthorpe wrote:
> On Tue, Jul 05, 2022 at 12:05:28PM +0300, Dan Carpenter wrote:
> > The casting on this makes the integer overflow check slightly wrong.
> > "len" is an unsigned long. "*pos" and "requested_length" are signed
> > long longs.  Imagine "len" is ULONG_MAX and "*pos" is 2.
> > "ULONG_MAX + 2 = 1". 
> 
> I wonder if this can happen, len is a kernel controlled value bounded
> by a memory allocation..
> 

Oh.  Smatch uses a model which says that all read/writes come from
vfs_write().  The problem with tracking kernel read/writes is that
recursion is tricky.  So Smatch just deletes those from the DB.

> > Fixes: b0eed085903e ("hisi_acc_vfio_pci: Add support for VFIO live migration")
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > ---
> 
> This code was copy and pasted from drivers/vfio/pci/mlx5/main.c, so it
> should be fixed too

Sure.

I created a static checker warning for this type of thing but it didn't
catch the issue in drivers/vfio/pci/mlx5/main.c because Smatch says that
the bug is impossible.  Which is true.

Smatch doesn't really parse rw_verify_area() accurately.  I just hard
coded that function as accepting values 0-1000000000 for both *ppos and
count.

regards,
dan carpenter