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;
Message-ID: <39ab53c4-cb2f-82f4-1097-65d000284b23@oracle.com>
Date:   Tue, 15 Feb 2022 11:11:51 -0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.5.0
Subject: Re: [PATCH v5 5/7] mm: shmem: fix missing cache flush in
 shmem_mfill_atomic_pte()
Content-Language: en-US
To:     Muchun Song <songmuchun@bytedance.com>, akpm@linux-foundation.org,
        zi.yan@cs.rutgers.edu, kirill.shutemov@linux.intel.com,
        rientjes@google.com, lars.persson@axis.com, ziy@nvidia.com
Cc:     linux-mm@kvack.org, linux-kernel@vger.kernel.org,
        duanxiongchun@bytedance.com, fam.zheng@bytedance.com
References: <20220210123058.79206-1-songmuchun@bytedance.com>
 <20220210123058.79206-6-songmuchun@bytedance.com>
From:   Mike Kravetz <mike.kravetz@oracle.com>
In-Reply-To: <20220210123058.79206-6-songmuchun@bytedance.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?TnoxSGtTWVZPS3d5dlNvWjhmWkRMZzVUL0FwbHZ6ck5WVnVNZGdLYlk1Ui9u?=
 =?utf-8?B?ZHd6eGcwMUhjejhRdHYxdzA5TlFhanhqOUZpcCtmVmRJQXBsaGQ3TGRKb0Qr?=
 =?utf-8?B?aWxrZHUwWU9KRkhuMnMyWE40QzAvVGliMlFPbHpBL0JaNktsamNtY1pHZytW?=
 =?utf-8?B?WnZNVmlqc21Gd1ZtRFNvZURFelNySmJwZ3ZhNFU1NFprakU0Y2FMcUZsMzJT?=
 =?utf-8?B?ZlZCeVI1ZWU5dVpTMkUvS3dZNTJ3cUFsVDd5b1ZHUjlxQUM0UzRsZFBmK1g2?=
 =?utf-8?B?b0JRTTVaT2N6SzJQSC93ajRqcWx5emo0TUt5SVk3WUthV1RtV20zN2JnWU5P?=
 =?utf-8?B?SmhGc3hYT2FNYzBlMGNvNjEyamF5dHZIZzZVL2todU9qZGVCWTFZOFNkcVdU?=
 =?utf-8?B?RlZzakR1RXJGbFhNRUtUMUQ4N1BlZThOU0JWN3hqVFhnaDVRT2VzU053b3RZ?=
 =?utf-8?B?WDR3eXkzMnpzRTg1S2UwTXh5dUdRYTNFMmJpWlFsQkFwTEJEek9Kby9jQmxz?=
 =?utf-8?B?THBCUXY1aVhRN0F0S1llTUNPcHdobnpDMVdnYVF3UnR0N2loelRNRnREY0d4?=
 =?utf-8?B?MEE5czNyenNvVVJyTng5RlNlUGpYcitkUFZISHVHL2dkRVY5UFg4S0x3L2cw?=
 =?utf-8?B?OVhNVVNuVDV1ZW0vdXhtbndkNStwV0hWSi8zS3JLT282RUZEcWNPZXRpOElF?=
 =?utf-8?B?dlg1dk5IMm1tRUVCc1JhdFgvV1crd20xZERjQUZGOTgrS3lLdmpOeVpLUEIz?=
 =?utf-8?B?NmJUc2xxNFZtV1pvNG1kYlV0OUZLdWViUklOREc5N0R0UmxSK0h5bThGQVdl?=
 =?utf-8?B?WVdScW12aHF3c0tVUnQ3ZkViYTJCbW9ob05OKzRZT2cydTdvcjVoekpOK2lv?=
 =?utf-8?B?cG45NEp0NFhJTUpBT1dzc0UyVHUzSWVGN1NUNnliZERRVHZKYmdFeEVPUTUx?=
 =?utf-8?B?ZmMySkhsRzFlb0tDY20rby9iSkt6djJidDJOVnhvSFhGdUx5SlNlancxbmdV?=
 =?utf-8?B?Y0ErZEhCaGRwRkN3b3Yyd0sySmdZRFE5VlZ1OXZ1TjF5Um1xdFhaOGpHbGxp?=
 =?utf-8?B?VWUzbmoxNUlKejV0d05ES01pQWtZNEpiMGVWMDJENEZ5a2RKK2JRSS9vekNP?=
 =?utf-8?B?dFA4L3dLMmc5cU5WYkVOS3U2R0JPVU9pN1B6RXh4N1hFV0hpUEVlY1Y5cGpL?=
 =?utf-8?B?YWRZaEsycktQZHhpY3B6cndZRC80Ri85ZXRyUUIrY0xSUXJNOGUrUWNRRjd2?=
 =?utf-8?B?UUpmc0JzOWtNdVlkUkpHR3ZZQkUzVys5TEJBcXlzcjdUMlZWV0lxUUJxY05j?=
 =?utf-8?B?RGpRb0Q0a21nbkRlWmg0Y1NpbDZYeDVXN1R5VU56cWo3b3pBTEVUbmxGNEp1?=
 =?utf-8?B?a3Q1VG9reHNCbnJSSGFxMEg4d2hRdkJudHhDS3JKMmVXMWkwbUVsWW50OURS?=
 =?utf-8?B?bWlOVlZiMUJYa0VpczdLZDRBbXpGcFowV2J3cHlGTDU5by9VWlk5aDVoSkxY?=
 =?utf-8?B?c3BsRFB3N3F6SVViMWlEZU8xOTVKeGJqMVZKamxlL00zZ2E3ODJWMHJjaCtz?=
 =?utf-8?B?NStLS3JiL3ZqQ3lPNmQzTmhrdXZneGc5U0ozbTB5M0g5WW1mOWZsMS9leUZq?=
 =?utf-8?B?MnUyYjJBKy9PaU9IZ2l6S1cvR3RUQ2FQUjIrYlRJOGxUYW93VFBZSE1LY2FS?=
 =?utf-8?B?ZWNLK3FYYjlWQXE0dDAxeVIzWEpUeEpZbFZpUnVBOEh3SUlQUTlUTTBicWpZ?=
 =?utf-8?B?blh6eTZyK2dXdnFnOFZhV0VmSU01anNDTEtiWlluS3pvNzdHMFlCRkdDWUsy?=
 =?utf-8?B?cnN1L2JyRXBqdnQxVkc2NzJaNHNETUVESG5XS1FlTHZSRm90aXBXcEg5NzFY?=
 =?utf-8?B?VE43aHZOcXM1ZjEvVytRVGVVT1pFWWNBbFFSczhUaEFRTU1pb1JEQUhsNUFY?=
 =?utf-8?B?Y2xaUVIydXAzeEVhWEhoNWJJWlVjU09BNURXNEZUWFhRVWxFdGF1eWZrTC9x?=
 =?utf-8?B?NWJrR3daZzFGZjd6QTRHS041M0cvQVpvaG9BeVE2NzNtSHdaNGE4M09tRmJ3?=
 =?utf-8?B?OVU2akFrY09rbFFiODIxRG1sRklVWjZZWGRCanlDbTBkM3RZT1pxU0lVMU53?=
 =?utf-8?B?ZmpudklJc0VWN05SaW5nQTlDMjJoS3pLd2o2cWxFc0ZEL3pVbXZobzNsRWZm?=
 =?utf-8?Q?nWcpPHlOMCzgkw7pmeDa04M=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 95f01606-1ddf-47e3-5855-08d9f0b70768
X-MS-Exchange-CrossTenant-AuthSource: BY5PR10MB4196.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 15 Feb 2022 19:11:54.5469
 (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: c4pC+rbJyWb+xeucMQDpZhY3d9aQqomM7oC7AvAuzX+iieP07r1LiDcHvlGNLQaDVN3ZwdkOyOxCiJqn80Wt+Q==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY5PR10MB3906
X-Proofpoint-Virus-Version: vendor=nai engine=6300 definitions=10259 signatures=675924
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 spamscore=0 bulkscore=0 suspectscore=0
 phishscore=0 malwarescore=0 mlxscore=0 mlxlogscore=999 adultscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2201110000
 definitions=main-2202150112
X-Proofpoint-ORIG-GUID: kMx1Wr80miPWllpXJQPk2XjYZA4A4kyh
X-Proofpoint-GUID: kMx1Wr80miPWllpXJQPk2XjYZA4A4kyh
X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H5,RCVD_IN_MSPIKE_WL,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 2/10/22 04:30, Muchun Song wrote:
> The userfaultfd calls shmem_mfill_atomic_pte() which does not do any
> cache flushing for the target page.  Then the target page will be mapped
> to the user space with a different address (user address), which might
> have an alias issue with the kernel address used to copy the data from the
> user to.  Insert flush_dcache_page() in non-zero-page case.  And replace
> clear_highpage() with clear_user_highpage() which already considers
> the cache maintenance.
> 
> Fixes: 8d1039634206 ("userfaultfd: shmem: add shmem_mfill_zeropage_pte for userfaultfd support")
> Fixes: 4c27fe4c4c84 ("userfaultfd: shmem: add shmem_mcopy_atomic_pte for userfaultfd support")
> Signed-off-by: Muchun Song <songmuchun@bytedance.com>
> ---
>  mm/shmem.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

Thanks,

It might have been better to combine this and the next patch.  When looking
at this, I noted the 'fallback to copy_from_user outside mmap_lock' case needs
to be addressed as well.  It is in the next patch.  No need to change.

Reviewed-by: Mike Kravetz <mike.kravetz@oracle.com>

-- 
Mike Kravetz

> 
> diff --git a/mm/shmem.c b/mm/shmem.c
> index eb0fd9001130..2e17ec9231a2 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -2371,8 +2371,10 @@ int shmem_mfill_atomic_pte(struct mm_struct *dst_mm,
>  				/* don't free the page */
>  				goto out_unacct_blocks;
>  			}
> +
> +			flush_dcache_page(page);
>  		} else {		/* ZEROPAGE */
> -			clear_highpage(page);
> +			clear_user_highpage(page, dst_addr);
>  		}
>  	} else {
>  		page = *pagep;


-- 
Mike Kravetz