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, 2 May 2023 09:13:15 -0400
From:   "Liam R. Howlett" <Liam.Howlett@Oracle.com>
To:     Lorenzo Stoakes <lstoakes@gmail.com>
Cc:     linux-mm@kvack.org, linux-kernel@vger.kernel.org,
        Andrew Morton <akpm@linux-foundation.org>,
        Vlastimil Babka <vbabka@suse.cz>
Subject: Re: [PATCH] mm/mmap/vma_merge: always check invariants
Message-ID: <20230502131315.fiq3tbc46f4mozum@revolver>
Mail-Followup-To: "Liam R. Howlett" <Liam.Howlett@Oracle.com>,
        Lorenzo Stoakes <lstoakes@gmail.com>, linux-mm@kvack.org,
        linux-kernel@vger.kernel.org,
        Andrew Morton <akpm@linux-foundation.org>,
        Vlastimil Babka <vbabka@suse.cz>
References: <df548a6ae3fa135eec3b446eb3dae8eb4227da97.1682885809.git.lstoakes@gmail.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <df548a6ae3fa135eec3b446eb3dae8eb4227da97.1682885809.git.lstoakes@gmail.com>
User-Agent: NeoMutt/20220429
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?x1KB15lstY8Q9tSNTA7Tk5XRldQZlOI7iyjvAOwj5yHgI4Low+YFHZXPvfzq?=
 =?us-ascii?Q?7F/OOVFbJmUHO5cP5DjPM2Joo9WMZiXB/U1x2g5C6nOiC1yC7DJ6KJOqBSMh?=
 =?us-ascii?Q?r3c17D64IPCkuY4YtSdVUPPmeurTt9JLVlSuE5uIcLEdjK/P8uizVa0/uwcf?=
 =?us-ascii?Q?WDM0CtHs8FJTwYiHYED/zKAV7rmw3iKEnyvcAxiaHwRm0nWqNIF0TnqGPF5l?=
 =?us-ascii?Q?lkidYihYNvLNmPv22OhjFmGP8SL2yrSCeeg4FrUZbJfD4Lfb/GQKppxmv6lx?=
 =?us-ascii?Q?s78rnkTYS0e2Wj7y9ORJyO1xXR/3F6OStxx4M6/+SyyTIAoUvNp4Yw8At6MG?=
 =?us-ascii?Q?p1pwdUTJq5UIx9IDN4CW5J/Zt11TM6hqV3H8B1+mlVd17U2dq1+PdpSioYZk?=
 =?us-ascii?Q?zYuRVozJtU3emw+Y15UukkZ20NA1C05BlahV2t/SiUKdXiaWyCRGZtHnaZki?=
 =?us-ascii?Q?gaVUocPLNy2b39okU8erQRdszs5ojZHbYjkCsIuUbD6I3TCRAtCXpYMm/yXv?=
 =?us-ascii?Q?0zhOctkse76V9lcsdJfrUWXK+517JJdTvLBuvNPr6GpBZIpsEJo1tJQ+s7nS?=
 =?us-ascii?Q?SHop56Smcv39aFlwdCa0x2hJqP5jOXHTZl+zWNUpsDoPdhqEHzih0VjmL4uT?=
 =?us-ascii?Q?9s9yFj4qqz+j/gjpEHlWjRuQvtED5jxBFebxkfgInGWmfDMOPrkEnfKcx9iK?=
 =?us-ascii?Q?O50XOl4KNl+xcJJfxVT66czy9g0YMdFAApUJ7NsPIeqDNCBeLlGux0K4BCqc?=
 =?us-ascii?Q?XGkWCRhiUhb4MObgHinq8Kn/d/bURRSRv2xZM0yxPTfStvYR2IUYWAGyqByL?=
 =?us-ascii?Q?PHd/WOVJeaHTmnZRryRqfboBPRO+twi29TKjqraQ7KvtRBsrxIfQOwQ63ObG?=
 =?us-ascii?Q?9pYaCHcnjBrEeizjgHl7TJAaMhh5CnGCVlEP1O/DrLx2n+4ofPHcmCBnyfRm?=
 =?us-ascii?Q?d/pqe80P2aND1NPMlqybx4vBZ+mNdIQRWiSTecHflje8a26MudV/hNeRZSla?=
 =?us-ascii?Q?zxARMPRGTbde/1ax6hCmTqspZBj8oUeQ8xtDHaFSnkS723qh+2fEbTgY2knh?=
 =?us-ascii?Q?Hk6gu1wzUvpBly3IYGMUkZxkj0HGVA4LnuEfciJGNuuX5uE66RtmkHJo5DTs?=
 =?us-ascii?Q?OcKIHUInJ1lviVZMpX/dzMqENxGmOjHdVjeFPtFTWNE5dpm/YC89ahFdPfWW?=
 =?us-ascii?Q?R76Q7/2qX4KRNUi4YE7WBs7QSSumBuXkQfOkpZXtt9fyjmhc3U/oU/BgKKEN?=
 =?us-ascii?Q?Z1XLCDcKAElJ6EOuZCuqPJAMIvnyzonRBcYzu2LHCIjiGLx5fM3EP7itR4bn?=
 =?us-ascii?Q?TFeMhubf7ppOpUQV1piThmXQQvHecFU2ojxpyNcNRR03QVDlSU68LiFVqaYj?=
 =?us-ascii?Q?6m2GWDT1jwKuzZwfxb6KVsGcYe+0V7IMLJ/35U/IaonWGF+wdp8LYbLSO21w?=
 =?us-ascii?Q?E1EA2zsz3Fa48cMtI1djEgjZmCtEnZQ8kEHIW0+noE6hdMw3J76NERgXS1eq?=
 =?us-ascii?Q?0Y7rBlZaoxqNoyNoamyV1z0Ywsl2/YFzjfJZGeNGEG8HGWg31QWXqXtFqouC?=
 =?us-ascii?Q?VXlMTs7tRlebLWu9OOW5KosaiYsXRAOrj3Wj7bw2pg65G+xLOLu273M+5SKI?=
 =?us-ascii?Q?fw=3D=3D?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: TkeXA1rRWaFSnm2Gk4PNdOA6OqPf/TBsNwnZ4buAUwkaxCauHQSM/RI/fy425DY6PPQB1zPFOW4FAFx92ajTJ7XQGIsAN4nIhwbLL8ChcZBap2wGtJW8n1yMfgVPhpTrPp83BquNhjT9rhzU5NWb+Uv7VaqmscYgYcRFY31nVxwKRO7qOvTHuBXxE8v2faEh0UITsjjTDhpzjgC7L9L/s/A9qUpU1BvTD9Pf1N+w7oUeoSrd5T9d3aAgBF/2V6f5rzNWHkAO7ckW8WDr+ZoXUp4isZ/+mFxKTDaA4bAMyjuIq7Ki8hvchprLZQhYnV8Pe3IWDk6/kupOSAkCMhXWk08b7OB3DxFrXoe8AFft+60DcorrBKGcHGiYKCehlmMRVEBgju+dMM6LLp5VbzeOgwbGotFwo6qPL+nvJlu3MC8q5tXfxsyrXNju2G7jod4CH5m9Z46jymsTrAiaty08ZOEG+E+s1Skm49dtYuabJArWHv2hL/8UiEH1xyZXLqZMxRfZKCiofMuQ+VaIlmn5swXo7C61N1JUZH4ZeL1u57uF+5Y2+TCXlkVQ6Ocg7Oam2cj5d7nZA1lsrBLw9ubzM2Ai/1q9VzGUHXFTnTLHCk61Grg7FOPBLIRDzycwnb8ulH+GSGtyhD1ky3dCiaHkNd0Oa8ytcCFnNm9EqKrO1LJd6qAdMhCfjRtpj/m9Hy38R4bym7wjFHs9NCrCesF9Po0n2m0BKOWJfpl9ep1ownsO9QI0JyFGW4p8g2q+Sk8uzo92qeVzkPeaT1FTOzyw1gfkSc9Ifg0+/NLfskyqbHhTQpnO5UuX/gO1Zey0TJMidMzFOrIc5Znt8exKi0C3gyoRQNKqtWSj54qGLziFy+wg9OwjTxqcrt7A3bImi8b0UnIBjR4NmO7jI4PSoVR1NQ==
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 6f055dbb-8315-43d9-d786-08db4b0eff03
X-MS-Exchange-CrossTenant-AuthSource: SN6PR10MB3022.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 02 May 2023 13:13:18.4962
 (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: ZCEIA/GL0CjSQk3i4XljeKE+ZwMitsUJTAI/a0VgqEJLkhTCfA3/3lC+J3awUR4/uEI08brl5HPIKHm7rAOT9g==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ2PR10MB7670
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.254,Aquarius:18.0.942,Hydra:6.0.573,FMLib:17.11.170.22
 definitions=2023-05-02_08,2023-04-27_01,2023-02-09_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 adultscore=0 phishscore=0 spamscore=0
 mlxlogscore=785 bulkscore=0 suspectscore=0 mlxscore=0 malwarescore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2303200000
 definitions=main-2305020112
X-Proofpoint-GUID: Hk52TNFhTj1_furpYO21hhI4PzNWE20G
X-Proofpoint-ORIG-GUID: Hk52TNFhTj1_furpYO21hhI4PzNWE20G
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

* Lorenzo Stoakes <lstoakes@gmail.com> [230430 16:19]:
> We may still have inconsistent input parameters even if we choose not to
> merge and the vma_merge() invariant checks are useful for checking this
> with no production runtime cost (these are only relevant when
> CONFIG_DEBUG_VM is specified).
> 
> Therefore, perform these checks regardless of whether we merge.
> 
> This is relevant, as a recent issue (addressed in commit "mm/mempolicy:
> Correctly update prev when policy is equal on mbind") in the mbind logic
> was only picked up in the 6.2.y stable branch where these assertions are
> performed prior to determining mergeability.
> 
> Had this remained the same in mainline this issue may have been picked up
> faster, so moving forward let's always check them.
> 
> Signed-off-by: Lorenzo Stoakes <lstoakes@gmail.com>

Reviewed-by: Liam R. Howlett <Liam.Howlett@oracle.com>

> ---
>  mm/mmap.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/mm/mmap.c b/mm/mmap.c
> index 5522130ae606..13678edaa22c 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -960,17 +960,17 @@ struct vm_area_struct *vma_merge(struct vma_iterator *vmi, struct mm_struct *mm,
>  		merge_next = true;
>  	}
>  
> +	/* Verify some invariant that must be enforced by the caller. */
> +	VM_WARN_ON(prev && addr <= prev->vm_start);
> +	VM_WARN_ON(curr && (addr != curr->vm_start || end > curr->vm_end));
> +	VM_WARN_ON(addr >= end);
> +
>  	if (!merge_prev && !merge_next)
>  		return NULL; /* Not mergeable. */
>  
>  	res = vma = prev;
>  	remove = remove2 = adjust = NULL;
>  
> -	/* Verify some invariant that must be enforced by the caller. */
> -	VM_WARN_ON(prev && addr <= prev->vm_start);
> -	VM_WARN_ON(curr && (addr != curr->vm_start || end > curr->vm_end));
> -	VM_WARN_ON(addr >= end);
> -
>  	/* Can we merge both the predecessor and the successor? */
>  	if (merge_prev && merge_next &&
>  	    is_mergeable_anon_vma(prev->anon_vma, next->anon_vma, NULL)) {
> -- 
> 2.40.1
>