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:   Tue, 5 Jul 2022 12:05:28 +0300
From:   Dan Carpenter <dan.carpenter@oracle.com>
To:     Longfang Liu <liulongfang@huawei.com>
Cc:     Yishai Hadas <yishaih@nvidia.com>,
        Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>,
        Kevin Tian <kevin.tian@intel.com>,
        Longfang Liu <liulongfang@huawei.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: [PATCH] vfio: hisi_acc_vfio_pci: fix integer overflow check in
 hisi_acc_vf_resume_write()
Message-ID: <YsP+2CWqMudArkqF@kili>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?+x5ceaXlxJPjP5OUc1+bt+ZmeKCYRDh0Pf++sZuFgAZe8OFPtRe/lU8ojIPC?=
 =?us-ascii?Q?roeUMXJXkMoEKzzvTWQVTO8acqYX55/kOb9xDvaocMaVJfurlHZ49GHcxKP9?=
 =?us-ascii?Q?5/IU2pyzhN5PVuqRXG68YP0E/qyr7VfjvLHnSuyzogNI+FSVfwWl+rJ72MEe?=
 =?us-ascii?Q?wJ+DgaDWOTo8BSOA0VINNjW7VmYcO4hoTeqB8+75Q07bPjB01E9TD32roWUt?=
 =?us-ascii?Q?u1VtamMY/3MDDGiEhtyKi8mqH9XRq5Ot8YRwhwyoD0IkZ+jUQnSL0WhTG80I?=
 =?us-ascii?Q?78Isj9qm0/4CzfHOyNfEtAa4sIGkRbQPmfc0WD7bcE5uUro4u3Xvtc/zPf0o?=
 =?us-ascii?Q?5XiDOUwmwq/qDfh/Hy63geKKWln2H+KMrj26UD65RRqSYbW+a9uGlVJ39rIm?=
 =?us-ascii?Q?tEgFE0Vrn+4U1llGXANcjdKz/p8YyFJQGDca7pSXlzqfcNdy0ImzLG9xb6Kv?=
 =?us-ascii?Q?M9QxAD9fVXY8AMEkCmaOCg+vbfOQnCzqoXyYDPAxG9upW9Yq5nStDWgFE/uD?=
 =?us-ascii?Q?YjHIwWebxHC656C2bDIyMDd8hGn2wtn+3AcxVkZgomu6n8eWZ0jKNWauzqOb?=
 =?us-ascii?Q?5aZtS+C2Bw9X0S+ma4vbyG0+/mE52NKs+GFgDhau2ZUZbxHuXIQ3Tr0GInLP?=
 =?us-ascii?Q?aaR3wUqXNUm6/iug1G3uJTRH/tU8KSf+bSHB5pJo+9ujvplgUZZq45nHttgt?=
 =?us-ascii?Q?QOrAknVfXWuQ6VZOX+8OO2vkPlx1KdKI3dMFnObSXqQMTQmkNYZkJSj0SIJu?=
 =?us-ascii?Q?wpTL3d2QyWqfmYBzB6pcG8DHjSbVfs0Vx3soB9Desb552PFSrlt5eHUVDp18?=
 =?us-ascii?Q?e2O7xq/8vVdJB5GBv8WlQOIXYauOaINE3qkIn8RuHZBGVJZNhxHHBz0bGAdE?=
 =?us-ascii?Q?ge8LHa2+fAAS/ME2QfoPYqkGcKJjJhPSCXqnp6EBdliNIFjCQmrN8PZJHfFU?=
 =?us-ascii?Q?lE+AhmIeXrJRlvpRl5q57cVD2j7eMhcbkIrG96B24hZlzg+3ohe42OlDcEta?=
 =?us-ascii?Q?ofB56JgySj46Z8nokz8eW/VtNCz+r2n46N9GccaZ81QnVmepYJrG4JXTA2H3?=
 =?us-ascii?Q?wnW+rYg/QOoxupA9tGP1NpDMw2PnSauctWZsBqSsah8lghyXh4fJqDfYJnPW?=
 =?us-ascii?Q?lguAbtbTyqSkGWKkJi20dJQr4hCD7i2JacI6paUCtLxbB23nxI22tSzbecoe?=
 =?us-ascii?Q?Y30qDfrNpaeFA8UOZ6GGsI7H8ZJEIOnhlXjXGw2psGdx/J+iWPlO8qNGf0uh?=
 =?us-ascii?Q?m7oRSXMJ6Et/06juYyHZanBGcqIHJspDZfs4qW7oolw6aFg3O1VmvpnIX8U6?=
 =?us-ascii?Q?B7c9Ciu/PWfl6+pEXSXkUWTr6LHlWzUqaQvkELTVvJrNru9omrm0dubA9RF8?=
 =?us-ascii?Q?dSAWukUK7vyYXiA2Ln01d4H6ULu9OPA9ipiirGmPVJ9V+hcYx6JJt7Y98i7D?=
 =?us-ascii?Q?fAokPAAUFi1B7gbcXBKHt6M2uWUPscDRMJCSnS3CsDFQmT1SOss/YW9EsQWe?=
 =?us-ascii?Q?S5KogksPhJuLWhSTpNz6vx384t8MZTduizst2M2zajQNRvvqC24HAIwfd8+4?=
 =?us-ascii?Q?CY6EswsM4MvcRRl4v0+pxXvFUuuCTlztIe5DaQ15bHVwQXC/hEvEs2B59rqr?=
 =?us-ascii?Q?RkjIUCxF9HWSPWjCtWGMplU=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: fd2f61b7-ea11-4ad2-ea4e-08da5e6589ae
X-MS-Exchange-CrossTenant-AuthSource: MWHPR1001MB2365.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 05 Jul 2022 09:05:42.3495
 (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: hHtg7V6Hk7J3idpqAsoUQ5XItEXqG5YzGAvcNBNS6++eu+wMnxQjNgK5b/uh3Wl5IJmpbpsana3hTpAntnZe05VV5hM1HqTnD20O13i3tpA=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN6PR10MB1748
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.517,18.0.883
 definitions=2022-07-05_07:2022-06-28,2022-07-05 signatures=0
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 spamscore=0 adultscore=0 suspectscore=0
 malwarescore=0 bulkscore=0 mlxlogscore=999 phishscore=0 mlxscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2204290000
 definitions=main-2207050038
X-Proofpoint-GUID: 2b-4pNnahvIcQ8y3pPLAXdJlYkxFa2p9
X-Proofpoint-ORIG-GUID: 2b-4pNnahvIcQ8y3pPLAXdJlYkxFa2p9
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

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".  That's an integer overflow.  However, if we cast
the ULONG_MAX to long long then "-1 + 2 = 1".  That's not an integer
overflow.

It's simpler if "requested_length" length is an unsigned value so we
don't have to worry about negatives.

I believe that the checks in the VFS layer and the check for "*pos < 0"
probably prevent this bug in real life, but it's safer to just be sure.

Fixes: b0eed085903e ("hisi_acc_vfio_pci: Add support for VFIO live migration")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
It is strange that we are doing:

	pos = &filp->f_pos;

instead of using the passed in value of pos.  The VFS layer ensures
that the passed in value of "*pos + len" cannot overflow in
rw_verify_area() so normally this check could have been removed.

 drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
index ea762e28c1cc..dcc34488b0c0 100644
--- a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
+++ b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
@@ -701,7 +701,7 @@ static ssize_t hisi_acc_vf_resume_write(struct file *filp, const char __user *bu
 					size_t len, loff_t *pos)
 {
 	struct hisi_acc_vf_migration_file *migf = filp->private_data;
-	loff_t requested_length;
+	unsigned long requested_length;
 	ssize_t done = 0;
 	int ret;
 
@@ -709,8 +709,8 @@ static ssize_t hisi_acc_vf_resume_write(struct file *filp, const char __user *bu
 		return -ESPIPE;
 	pos = &filp->f_pos;
 
-	if (*pos < 0 ||
-	    check_add_overflow((loff_t)len, *pos, &requested_length))
+	if (*pos < 0 || *pos > ULONG_MAX ||
+	    check_add_overflow(len, (unsigned long)*pos, &requested_length))
 		return -EINVAL;
 
 	if (requested_length > sizeof(struct acc_vf_data))
-- 
2.35.1