Re: [PATCH 3/4] iommufd: Do not corrupt the pfn list when doing batch carry

[Pengfei Xu <pengfei.xu@intel.com>]

Hi Jason,

On 2023-03-31 at 12:32:26 -0300, Jason Gunthorpe wrote:
> If batch->end is 0 then setting npfns[0] before computing the new value of
> pfns will fail to adjust the pfn and result in various page accounting
> corruptions. It should be ordered after.
> 
> This seems to result in various kinds of page meta-data corruption related
> failures:
> 
>   WARNING: CPU: 1 PID: 527 at mm/gup.c:75 try_grab_folio+0x503/0x740
>   Modules linked in:
>   CPU: 1 PID: 527 Comm: repro Not tainted 6.3.0-rc2-eeac8ede1755+ #1
>   Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.16.0-0-gd239552ce722-prebuilt.qemu.org 04/01/2014
>   RIP: 0010:try_grab_folio+0x503/0x740
>   Code: e3 01 48 89 de e8 6d c1 dd ff 48 85 db 0f 84 7c fe ff ff e8 4f bf dd ff 49 8d 47 ff 48 89 45 d0 e9 73 fe ff ff e8 3d bf dd ff <0f> 0b 31 db e9 d0 fc ff ff e8 2f bf dd ff 48 8b 5d c8 31 ff 48 89
>   RSP: 0018:ffffc90000f37908 EFLAGS: 00010046
>   RAX: 0000000000000000 RBX: 00000000fffffc02 RCX: ffffffff81504c26
>   RDX: 0000000000000000 RSI: ffff88800d030000 RDI: 0000000000000002
>   RBP: ffffc90000f37948 R08: 000000000003ca24 R09: 0000000000000008
>   R10: 000000000003ca00 R11: 0000000000000023 R12: ffffea000035d540
>   R13: 0000000000000001 R14: 0000000000000000 R15: ffffea000035d540
>   FS:  00007fecbf659740(0000) GS:ffff88807dd00000(0000) knlGS:0000000000000000
>   CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>   CR2: 00000000200011c3 CR3: 000000000ef66006 CR4: 0000000000770ee0
>   PKRU: 55555554
>   Call Trace:
>    <TASK>
>    internal_get_user_pages_fast+0xd32/0x2200
>    pin_user_pages_fast+0x65/0x90
>    pfn_reader_user_pin+0x376/0x390
>    pfn_reader_next+0x14a/0x7b0
>    pfn_reader_first+0x140/0x1b0
>    iopt_area_fill_domain+0x74/0x210
>    iopt_table_add_domain+0x30e/0x6e0
>    iommufd_device_selftest_attach+0x7f/0x140
>    iommufd_test+0x10ff/0x16f0
>    iommufd_fops_ioctl+0x206/0x330
>    __x64_sys_ioctl+0x10e/0x160
>    do_syscall_64+0x3b/0x90
>    entry_SYSCALL_64_after_hwframe+0x72/0xdc
> 
> Cc: <stable@vger.kernel.org>
> Fixes: f394576eb11d ("iommufd: PFN handling for iopt_pages")
> Reported-by: Pengfei Xu <pengfei.xu@intel.com>
> Link: https://lore.kernel.org/r/ZBExkEW/On0ue68q@xpf.sh.intel.com
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> ---
>  drivers/iommu/iommufd/pages.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/iommufd/pages.c b/drivers/iommu/iommufd/pages.c
> index b11aace836542d..3c47846cc5efe8 100644
> --- a/drivers/iommu/iommufd/pages.c
> +++ b/drivers/iommu/iommufd/pages.c
> @@ -294,9 +294,9 @@ static void batch_clear_carry(struct pfn_batch *batch, unsigned int keep_pfns)
>  			batch->npfns[batch->end - 1] < keep_pfns);
>  
>  	batch->total_pfns = keep_pfns;
> -	batch->npfns[0] = keep_pfns;
>  	batch->pfns[0] = batch->pfns[batch->end - 1] +
>  			 (batch->npfns[batch->end - 1] - keep_pfns);
> +	batch->npfns[0] = keep_pfns;
>  	batch->end = 0;
>  }
>  
> -- 
  I tested the reproduced code in the kernel with all 3 fixed patches.
  Syzkaller reproduced code: https://github.com/xupengfe/syzkaller_logs/blob/main/230313_234302_try_grab_folio/repro.c
  This issue was gone and the issue was fixed.

  Thanks!
  BR.

> 2.40.0
>