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, 12 Jul 2022 10:39:20 -0700
From:   Mike Kravetz <mike.kravetz@oracle.com>
To:     Miaohe Lin <linmiaohe@huawei.com>
Cc:     akpm@linux-foundation.org, songmuchun@bytedance.com,
        Axel Rasmussen <axelrasmussen@google.com>,
        Peter Xu <peterx@redhat.com>, linux-mm@kvack.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH] mm/hugetlb: avoid corrupting page->mapping in
 hugetlb_mcopy_atomic_pte
Message-ID: <Ys2xyCUnqpJt0eIo@monkey>
References: <20220712130542.18836-1-linmiaohe@huawei.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220712130542.18836-1-linmiaohe@huawei.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?EO7pX0oFRNfDps9CZjSJHBHWPlNmd3DjNhdKQumdEac6W2fDd9Zgc736WdFe?=
 =?us-ascii?Q?GRyeJhstJuMXoe83l0DxZX0o5hD0RMcw0+eWptu5PLYA2rzs8E7Bz6VRqz0H?=
 =?us-ascii?Q?IDvzl/JWbolovQMGm1PyNG8m7YCDrNxVTvlBYzwoa1jvxLAg0ucBgo40AEh3?=
 =?us-ascii?Q?rYUB5UGXz//B1zRF6pP0WOth9VRQdYy+1vOqMoFsFtqbf6vrP4LSa5nl2YQ8?=
 =?us-ascii?Q?DJ8vWtCJroGZL59OsEGehPpIlZf6YfU/edErAg0ooRS44B8csu89fO0KocJ3?=
 =?us-ascii?Q?iH9wzWj0zD7bbDfcQYe25vUGSIbljdJ9e+hb9uk+92fbXsMLHDtjoVLanjJa?=
 =?us-ascii?Q?Zq+JD1i3TV7JXXKf5z2SZUa9lmc5V3eVO0PQkGi5hdlDrimKmbed+kd536ys?=
 =?us-ascii?Q?HCkcCgjngwakPa7/l/M1H/BOlmGqpMmRCBuL71A7eDeByw/6pEFAtaRcTj2Q?=
 =?us-ascii?Q?hfyPipQqAcdbBGQ4nYZ+x4bUBMZMlJMtDtVo9HL82bJBt/S0bMXhAXOoLz5R?=
 =?us-ascii?Q?wJuBRc2O2mqlkYZ2XEA900QLxEGwIpoGAcrZLaHNag0jC99bPb8r2LMkKW3e?=
 =?us-ascii?Q?RmhQZzq0RNQPW206+KdoEhvfgRz6fQ4hXnO2q08wJqt7sdn8S4QhXooBvCAm?=
 =?us-ascii?Q?F4vQpXJeCCAJnWxPJKNILOauMG+LlzWbLxPw4DZrY+i0hDAobhM1eawgRf6D?=
 =?us-ascii?Q?r3aBGra+1aWauMO1Drl9Sfgd1dHfg57GbFQlZPZ4k37i7kjS89vDTXok4Ao0?=
 =?us-ascii?Q?t1Kf2pI58pq3lsq4KZAvBSNHx5ExBWPgiEA5r2c6GPLz5wnWTfU/cYmbfFN7?=
 =?us-ascii?Q?euDddFSMXPb/2GqYTlGq8Yzi4QTpOr22682W/wNixGhVoV40U7bxYcQqpaf5?=
 =?us-ascii?Q?J76Jtx7fYqPEenl5/mqxCCl2ePd1VoCBADmj3gGSkPgZIwRImhaoHRj1qVBa?=
 =?us-ascii?Q?txzpVvkWM0eeVw6FjXOAdxGp6Uq2/vNvcYhFyT2j33Yn33EXNDaKZsWya9nw?=
 =?us-ascii?Q?dwNXqrdikmpANWIBLO7gskwwZAd3WMt2ccu4izqvnfpn+QB5HBoEQ0PJz7RG?=
 =?us-ascii?Q?H66SF3Y2LwO9sucaGRpjvrN+N0uTio7vWzQXmVM6mLd3BhxVmaOdTy3gbuXD?=
 =?us-ascii?Q?ROzvDXWpvh5obfekjk7rqF8Uy5TmqNaaghB1yMy+Zi4UyJf2JPnjH7cgEMhr?=
 =?us-ascii?Q?7tfHff/ck6nldK2s02IndzXBCVnAOb2cBFc/x/2fmAd/IcDR+jzsOVgNqf7T?=
 =?us-ascii?Q?vJW8ed1En42hvR70r5pnkxwWLUCcCbswwdu2oKQ938EeFqHpoX31FVVA2QTO?=
 =?us-ascii?Q?zzL8QjzrVLrrhAmmfllFE8me6BkoOh5blU/udsQW5IwnUokun2RsTvxKHZE2?=
 =?us-ascii?Q?7LlVBuhSBgb/lWT8ahfBehrZavsUEuJD/xbc8bazEbTzJCPuvI1vmgW97aM5?=
 =?us-ascii?Q?q5SMxkq6261dViGVarvK4xG2U6sZlgoebPiyrdBQHneac8oTWdS+0yRbcqWh?=
 =?us-ascii?Q?IPe7AwntzeUARRIkhIpGyKOhkdzZxWZNzFDOmil5avov+5vU1S3DlNoL11jf?=
 =?us-ascii?Q?SIZtrfqxI3MmIGiaPE8xYC8blnpF02fFxh4OuuIzg9Yf7WRqNTfATGe7XX0D?=
 =?us-ascii?Q?Xw=3D=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 67c571d1-244a-49db-e250-08da642d74f4
X-MS-Exchange-CrossTenant-AuthSource: BY5PR10MB4196.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 12 Jul 2022 17:39:22.6889
 (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: BzzA15jgeuYmZnbaHNM690YKoRqHTvgK0Oz5GNbuy9IKug7+1xYJtgaIMx0bd5wee60n2WEz+WM4lZgQ9PiPfw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MWHPR10MB1662
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.517,18.0.883
 definitions=2022-07-12_12:2022-07-12,2022-07-12 signatures=0
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 phishscore=0 malwarescore=0 spamscore=0
 adultscore=0 suspectscore=0 bulkscore=0 mlxlogscore=999 mlxscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2206140000
 definitions=main-2207120070
X-Proofpoint-ORIG-GUID: -tJKAjEsDhNQc2obYJmroFMhIf3TiYyh
X-Proofpoint-GUID: -tJKAjEsDhNQc2obYJmroFMhIf3TiYyh
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 07/12/22 21:05, Miaohe Lin wrote:
> In MCOPY_ATOMIC_CONTINUE case with a non-shared VMA, pages in the page
> cache are installed in the ptes. But hugepage_add_new_anon_rmap is called
> for them mistakenly because they're not vm_shared. This will corrupt the
> page->mapping used by page cache code.
> 
> Fixes: f619147104c8 ("userfaultfd: add UFFDIO_CONTINUE ioctl")
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> ---
>  mm/hugetlb.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

This looks correct to me.

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

However, I am having a hard time wrapping my head around how UFFDIO_CONTINUE
should work on non-anon private mappings.  For example, a private mapping of
a hugetlbfs file.  I think we just map the page in the file/cache and do not
set the write bit in the pte.  So, yes we would want page_dup_file_rmap()
in this case as shown below.

Adding Axel and Peter on Cc: as they were more involved in adding that code
and the design of UFFDIO_CONTINUE.
-- 
Mike Kravetz

> 
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 8d379e03f672..b232e1508e49 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -6038,7 +6038,7 @@ int hugetlb_mcopy_atomic_pte(struct mm_struct *dst_mm,
>  	if (!huge_pte_none_mostly(huge_ptep_get(dst_pte)))
>  		goto out_release_unlock;
>  
> -	if (vm_shared) {
> +	if (page_in_pagecache) {
>  		page_dup_file_rmap(page, true);
>  	} else {
>  		ClearHPageRestoreReserve(page);
> -- 
> 2.23.0
>