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, 25 Oct 2022 13:22:00 -0300
From:   Jason Gunthorpe <jgg@nvidia.com>
To:     Dan Carpenter <dan.carpenter@oracle.com>
Cc:     kbuild@lists.01.org, lkp@intel.com, kbuild-all@lists.01.org,
        linux-kernel@vger.kernel.org
Subject: Re: [jgunthorpe:vfio_iommufd 9/11] drivers/vfio/vfio_main.c:1690
 vfio_file_enforced_coherent() warn: assigning (-95) to unsigned variable
 'ret'
Message-ID: <Y1gNKJ1QhdcAn3rn@nvidia.com>
References: <202210252103.10UKM7Mu-lkp@intel.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <202210252103.10UKM7Mu-lkp@intel.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?QamzajhkLneBN58MsxeF0AwpTB4kbsUmk1x932FbD8D3+ZZ14xGMIxG0C/pH?=
 =?us-ascii?Q?YyC0zZVA8AU3EKsLpFL1HGhqutUILlNNVWJQ5Duqej76I2S00x6yUeOLRlyh?=
 =?us-ascii?Q?YlnvhfbM8wAuaX5kekO4a9GTc8XddQ1VZdG+Vhvxnod6EY1TdzJYLE48xUg1?=
 =?us-ascii?Q?fwBJwXmPiGWhK8sHpHjKSwP8D3ttxlsN/xZSAIiMACJCIs8yMAozNJ1CRRed?=
 =?us-ascii?Q?rgoTzejTed8C4l61UH/+hbFDwv6+BYwZ4V8Z4cQsZ2vmp4c0PpIfmCLpIa2M?=
 =?us-ascii?Q?oq0wDCDtHOut4EJLhvMO6+LiTouka5KqB0iYqEPSYBp99OzahExqH0PxH/vF?=
 =?us-ascii?Q?SVxtoZ/RsdH9+qA0JeWYBwSLznQb25uv+o1cYjB75Pve+AkHv+RXzoiMLw7T?=
 =?us-ascii?Q?XOWKB74bocDYMun+odzO3G8pGcY/gsZl/pE0Xos2eSzGsv8hpNiNLt2CPn9e?=
 =?us-ascii?Q?ea5hYDgGUOfHdVoQf1cFJw9D3QN25Nsh1KQHGz2g2YQgSy9TwwS50RqU/8FV?=
 =?us-ascii?Q?eC+qm2ITjhKBtXOZMQK2ul1Kcv9ix2XKv0EBt9ePa6LtIxrqEIpFzH0v1vbv?=
 =?us-ascii?Q?e+IQZCosMQmpigfqiaG0ZPjjQqEhprQ9hO8jDXa9ebwHE6ttuH1AQ0EQ8s3B?=
 =?us-ascii?Q?4qcKC5rD8kX2XL3u3qu4HONzs0B/N4F4mXQ330aDsclQ5r3Ea7T2XHe/A62z?=
 =?us-ascii?Q?u0gJxj4KYRu6ROQv8Fv2W1JjuVth9rFGCQC7tLdTwISASj61gIDvm9Qyfwce?=
 =?us-ascii?Q?rk57QG2LhGkKTDn4iVXrxL5NRDcL1p0PlStiQRiOxiiDuyYYzIXb077k2IX7?=
 =?us-ascii?Q?yrxDWJW/1tI7wM5e5q7+kO6lmIL5wMOc7+/Pp7/OS3HUrlijPqcWNmGVBSqT?=
 =?us-ascii?Q?V8SBUyikaJ5Yuu9tnf6SvUAKSBqtS2XxmByIoouL9v6Xc3b7YMOg3rF1Lub4?=
 =?us-ascii?Q?qU4aTebFLDx/JHK5ABf0VA8qh6cGG6vvlHh3ftbULK7oiC/uCvGCCizDfIAm?=
 =?us-ascii?Q?z01oIDwBkYpv/5FMIDpBbsQrMujVqELo+EFHIQq6170jayKD+oni3wgHkcrz?=
 =?us-ascii?Q?YyXXdT4jX5+Dh/9iFHrOB295UVAfMCy9KQtLRfuoY2uHab4axc5/PyyXKmCk?=
 =?us-ascii?Q?1T9Gd1QUdQAJ2HQAcsqu8BTXeiev4vIUHYMo44tmYDKU2M5kXompr3YBB77F?=
 =?us-ascii?Q?/kBYe9UOKpZAmx8DNeadis5Hnc1LQBp5yf3e0lq7Apv/TnBU/HkILFF8g2yW?=
 =?us-ascii?Q?UQwsaFOZDzGmFw83ohKciWIDNbhRiXtkjIkWCQ2MxSae6pOZYjazwk6op1Jx?=
 =?us-ascii?Q?Zld/x9sMSl0dqkdM+lUwsaHk1DbHuhOGV17oeIiaZrDFg/TvgXYWPVleCIDf?=
 =?us-ascii?Q?4bf4nLgwOAXRYT6PLJNdaDqXPSga36VqHLnR07aC8LvA2p3d4h5xSzbhyy2p?=
 =?us-ascii?Q?ZSzyJtTUQ47nlo5XJ9PoGn9Wcg8l5pnxmDJogE+w55egZuZ3Hsw4A3rvEY/p?=
 =?us-ascii?Q?9lnWpWPSfhvTgMDRuc/tRGOkMVUX4eo3HKB6Q2LOXzqf90KjvwyNVsKeLFRM?=
 =?us-ascii?Q?BRLyrO5oyreO4E0gbDo=3D?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: b79f355a-8f70-4ea5-e496-08dab6a50c02
X-MS-Exchange-CrossTenant-AuthSource: LV2PR12MB5869.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 25 Oct 2022 16:22:01.5283
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: uKNDhZmQhMlZBzsEsDgpH4FCza58v5tOIR2sgMyey7U4JugceQJIT1TZJCzEJVDq
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN0PR12MB5785
X-Spam-Status: No, score=-1.6 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FORGED_SPF_HELO,
        RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_NONE,
        URIBL_BLOCKED autolearn=no 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, Oct 25, 2022 at 05:21:36PM +0300, Dan Carpenter wrote:
> tree:   https://github.com/jgunthorpe/linux vfio_iommufd
> head:   a249441ba6fd9d658f4a1b568453e3a742d12686
> commit: e44299750e287f3d64d207a5af7abb021634a31b [9/11] vfio: Make vfio_container optionally compiled
> config: openrisc-randconfig-m041-20221024
> compiler: or1k-linux-gcc (GCC) 12.1.0

Dan! Thank you for looking at this branch, I'm going to be getting it
in linux-next very soon, so I will fix whatever your tools will spot!
Let me know


> New smatch warnings:
> drivers/vfio/vfio_main.c:1690 vfio_file_enforced_coherent() warn: assigning (-95) to unsigned variable 'ret'
> 
> Old smatch warnings:
> drivers/vfio/vfio_main.c:1907 vfio_pin_pages() warn: impossible condition '(iova > (~0)) => (0-u32max > u32max)'
> drivers/vfio/vfio_main.c:1974 vfio_dma_rw() warn: impossible condition '(iova > (~0)) => (0-u32max > u32max)'
> 
> vim +/ret +1690 drivers/vfio/vfio_main.c
> 
> a905ad043f32bb drivers/vfio/vfio.c      Jason Gunthorpe 2022-05-04  1671  /**
> a905ad043f32bb drivers/vfio/vfio.c      Jason Gunthorpe 2022-05-04  1672   * vfio_file_enforced_coherent - True if the DMA associated with the VFIO file
> a905ad043f32bb drivers/vfio/vfio.c      Jason Gunthorpe 2022-05-04  1673   *        is always CPU cache coherent
> 
> This comment sort of feels like returning false on error is the correct
> thing but in the rest of the code it seems like returning true on error
> is correct.

Oddly, true is the correct result. "false" means you have proven you
are worthy and that cannot happen on error cases.

> a905ad043f32bb drivers/vfio/vfio.c      Jason Gunthorpe 2022-05-04  1674   * @file: VFIO group file
> c0560f51cf7747 drivers/vfio/vfio.c      Yan Zhao        2020-03-24  1675   *
> a905ad043f32bb drivers/vfio/vfio.c      Jason Gunthorpe 2022-05-04  1676   * Enforced coherency means that the IOMMU ignores things like the PCIe no-snoop
> a905ad043f32bb drivers/vfio/vfio.c      Jason Gunthorpe 2022-05-04  1677   * bit in DMA transactions. A return of false indicates that the user has
> a905ad043f32bb drivers/vfio/vfio.c      Jason Gunthorpe 2022-05-04  1678   * rights to access additional instructions such as wbinvd on x86.
> c0560f51cf7747 drivers/vfio/vfio.c      Yan Zhao        2020-03-24  1679   */
> a905ad043f32bb drivers/vfio/vfio.c      Jason Gunthorpe 2022-05-04  1680  bool vfio_file_enforced_coherent(struct file *file)
> c0560f51cf7747 drivers/vfio/vfio.c      Yan Zhao        2020-03-24  1681  {
> a905ad043f32bb drivers/vfio/vfio.c      Jason Gunthorpe 2022-05-04  1682  	struct vfio_group *group = file->private_data;
> a905ad043f32bb drivers/vfio/vfio.c      Jason Gunthorpe 2022-05-04  1683  	bool ret;
> c0560f51cf7747 drivers/vfio/vfio.c      Yan Zhao        2020-03-24  1684  
> b1b8132a651cf6 drivers/vfio/vfio_main.c Alex Williamson 2022-10-07  1685  	if (!vfio_file_is_group(file))
> a905ad043f32bb drivers/vfio/vfio.c      Jason Gunthorpe 2022-05-04  1686  		return true;
> c0560f51cf7747 drivers/vfio/vfio.c      Yan Zhao        2020-03-24  1687  
> c82e81ab256955 drivers/vfio/vfio_main.c Jason Gunthorpe 2022-09-29  1688  	mutex_lock(&group->group_lock);
> e0e29bdb594adf drivers/vfio/vfio.c      Jason Gunthorpe 2022-05-16  1689  	if (group->container) {
> 1408640d578887 drivers/vfio/vfio_main.c Jason Gunthorpe 2022-09-22 @1690  		ret = vfio_container_ioctl_check_extension(group->container,
> e0e29bdb594adf drivers/vfio/vfio.c      Jason Gunthorpe 2022-05-16  1691  							   VFIO_DMA_CC_IOMMU);
> 
> But this returns true if vfio_container_ioctl_check_extension() returns
> a negative error code.  (I haven't looked at the git branch and I
> suspect it's different from linux-next)

Yes, I should not take this shortcut, we would be better to explicitly
return false on error return.

But good news, I just deleted this code, so all fixed :)

Jason