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;
From:   Liam Howlett <liam.howlett@oracle.com>
To:     "maple-tree@lists.infradead.org" <maple-tree@lists.infradead.org>,
        "linux-mm@kvack.org" <linux-mm@kvack.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Andrew Morton <akpm@linux-foundation.org>,
        Hugh Dickins <hughd@google.com>
Subject: Re: [PATCH] mm/mmap: Fix locking issues in vma_expand()
Thread-Topic: [PATCH] mm/mmap: Fix locking issues in vma_expand()
Thread-Index: AQHYlf6fdQQFbDqKqEWjS3P5pOojn61/yymA
Date:   Fri, 15 Jul 2022 18:49:21 +0000
Message-ID: <20220715184913.v75xv2na4dgz3ayb@revolver>
References: <20220712144926.4190769-1-Liam.Howlett@oracle.com>
In-Reply-To: <20220712144926.4190769-1-Liam.Howlett@oracle.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?aIFu1kzBP9B8YniH/amDWv6r2vH3XCGc1a01G7kdqwZeKAqmAGtgEEYOXw/D?=
 =?us-ascii?Q?XpO7UoM/YcWMTUGvI0GDGEUKRS/GxX3xUNcSuWuLRZds8+ipkAhT9SOyz3d4?=
 =?us-ascii?Q?XeHinnKulXywVO4slFCR41+hbLB9Pbytj6+zwJKKkljM5zbgbkDR9Bk2kyEH?=
 =?us-ascii?Q?GRTzClN0N2O0woQ2hWM2vEUVrfZOn9m2LOS38fJlyvJPHR++IzNRZJ9PiR7C?=
 =?us-ascii?Q?Rz1jwC+T93FF66P/kd6zXjVdnenLGcWRWzqAsxFJqwo5XKdx3iqlVJm7Pjif?=
 =?us-ascii?Q?4//lPWAmGwjIoV9OoA6m1HaukdcatP+8/6I/+FU9gz3N5GtFdMeIta12Rge7?=
 =?us-ascii?Q?8Yem+zg8fKlF5aaG8JovIrNjfzCqIRcsI/QDoF0vK+PN131s/O1vegJcsXZF?=
 =?us-ascii?Q?XxnriUmOODaiHh+1vBu9OKAl+FlEnr/8Yo7smROhK+HZ2ewsMEpUryjN2V92?=
 =?us-ascii?Q?p74yPE53xlF0ADpBX61YLIBjMqgWLrpEeWLz9DSXIuwmmP75m63hVdxcD5sJ?=
 =?us-ascii?Q?k0MHzoVKgV/4s1aIoCyMjwBxH1811UtgSrJ6q8vaE0fjqm+Ban+YaIj/LAh8?=
 =?us-ascii?Q?0a2gedFl8+70LC/e0KDFyxXAuuuTtjBTYAzuURVK5gqKKAcwB0id5GGf3jmU?=
 =?us-ascii?Q?EikKvs51l4JR4LF0m71yEFOGW/F+fB8Fpv/zrgk2097mTVVczMEZAQYui4va?=
 =?us-ascii?Q?dkJ7N66xU5UULwrRmm4OYunYtMFTbvbLcOp1bzhbGvBJxyg+lfzoxNTUfBag?=
 =?us-ascii?Q?0Y5Wp3hFt4tLdbgyUQFOIpnYoenXKT6XtH8yEK+/964B3DYkxzOs+XWZIuKs?=
 =?us-ascii?Q?1mA6w4RGlnEfou5jQpwfaUuShReOUE6vbCUuS3TuGIvN9G0cLsDe5OAwfCn+?=
 =?us-ascii?Q?x6ACShW7mw9GtU4EcWrhRBXhChjJsjiz93dPJuDV6gysdhk067AGvzjFhWiI?=
 =?us-ascii?Q?QU9v79AgbPNLRlCQGRql5quK1wR1c7GM3I7XBttVI9UWQfzpS8IwDlm3fuNI?=
 =?us-ascii?Q?OmiYhPQeqUjBsss5ltO6msnDL9M17QFDSg/qffBEveMAGMOReqAiqTjXO9It?=
 =?us-ascii?Q?xm8tqpm2+3TxhcQZ/tiiFza//TgIV7jBvNQJ9b3sQIFQNWWOiCexGiACC74V?=
 =?us-ascii?Q?1aDKsuAiNVJemv/coTddMVGEsefUxTsYpWvXxKkd8LLOcO6ojbqNAT4bsVo3?=
 =?us-ascii?Q?9/NSTWuTFK71KjSBwQE9sWJbwambnwSEZEIwlfGo+fC6Yg/rdc0U/vvuMPOf?=
 =?us-ascii?Q?imTR64dkyeRDuqpRnXBK3vl7gfj/uqUULssDWKAWFpyHOn6D2h4IBvDiE4V+?=
 =?us-ascii?Q?bBenZ0q63oORr3OahjAxxwFn36CIdIrCklYP7STXA8AibhOH8Ri8haaMYjIB?=
 =?us-ascii?Q?zWbqFI2V0DFgHTx+PD674iuYiYXnbx94KP8ZxHZUxxF2MDGxQKEcfZvDHD7W?=
 =?us-ascii?Q?AvmQoCJzdGoyzlwF2r0cB+rujRcAIpJZSsUY0tAewg9OJvWxwPYPUqoiZRez?=
 =?us-ascii?Q?HVunIkhvgf3A7OR7pAWYcB0wVaK7/bsJB/wWA8kMUYxweGxGhNGnEWQomcNe?=
 =?us-ascii?Q?rXhn8YC/SvV4sKqYNqMYXjI9vWe89eMUrV8RJyS07WqIZNWYfPKgRZg9dr/V?=
 =?us-ascii?Q?8Q=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <50A8F0DA2BD74242B2DF4A221E7961BC@namprd10.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: SN6PR10MB3022.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 923004b8-18bb-4e84-6db7-08da6692baff
X-MS-Exchange-CrossTenant-originalarrivaltime: 15 Jul 2022 18:49:21.4281
 (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: ZXKq5QYwOcCavbtu7g8EDOOalcnkAR8andw7Gr9hKOnCvWd4f28OgpWe+9Y8yLLVkNNhyxMSmFlnu1qtxzkRbQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY4PR10MB1366
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.517,18.0.883
 definitions=2022-07-15_10:2022-07-14,2022-07-15 signatures=0
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 malwarescore=0 phishscore=0
 mlxlogscore=767 suspectscore=0 adultscore=0 mlxscore=0 spamscore=0
 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2206140000 definitions=main-2207150084
X-Proofpoint-GUID: szQGBlqFVAV31ZBKw0HUvMNH3ap6a--k
X-Proofpoint-ORIG-GUID: szQGBlqFVAV31ZBKw0HUvMNH3ap6a--k
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 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


Andrew,

I think in my confusing post about ignoring the previous version of this
patch, you may have missed this completed solution?

Thanks,
Liam

* Liam R. Howlett <Liam.Howlett@oracle.com> [220712 10:49]:
> Hugh noted that the locking was mixed up in vma_expand(), revert to the
> correct order.
>=20
> Fixes: b2c87578e0ea (mm/mmap: use advanced maple tree API for mmap_region=
())
> Suggested-by: Hugh Dickins <hughd@google.com>
> Signed-off-by: Liam R. Howlett <Liam.Howlett@oracle.com>
> ---
>  mm/mmap.c | 23 +++++++++++++----------
>  1 file changed, 13 insertions(+), 10 deletions(-)
>=20
> diff --git a/mm/mmap.c b/mm/mmap.c
> index 9eb663cde5c7..9a07bc18e6aa 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -574,7 +574,8 @@ inline int vma_expand(struct ma_state *mas, struct vm=
_area_struct *vma,
>  		if (next->anon_vma && !vma->anon_vma) {
>  			int error;
> =20
> -			vma->anon_vma =3D next->anon_vma;
> +			anon_vma =3D next->anon_vma;
> +			vma->anon_vma =3D anon_vma;
>  			error =3D anon_vma_clone(vma, next);
>  			if (error)
>  				return error;
> @@ -592,16 +593,19 @@ inline int vma_expand(struct ma_state *mas, struct =
vm_area_struct *vma,
> =20
>  	vma_adjust_trans_huge(vma, start, end, 0);
> =20
> +	if (file) {
> +		mapping =3D file->f_mapping;
> +		root =3D &mapping->i_mmap;
> +		uprobe_munmap(vma, vma->vm_start, vma->vm_end);
> +		i_mmap_lock_write(mapping);
> +	}
> +
>  	if (anon_vma) {
>  		anon_vma_lock_write(anon_vma);
>  		anon_vma_interval_tree_pre_update_vma(vma);
>  	}
> =20
>  	if (file) {
> -		mapping =3D file->f_mapping;
> -		root =3D &mapping->i_mmap;
> -		uprobe_munmap(vma, vma->vm_start, vma->vm_end);
> -		i_mmap_lock_write(mapping);
>  		flush_dcache_mmap_lock(mapping);
>  		vma_interval_tree_remove(vma, root);
>  	}
> @@ -622,16 +626,15 @@ inline int vma_expand(struct ma_state *mas, struct =
vm_area_struct *vma,
>  		__remove_shared_vm_struct(next, file, mapping);
>  	}
> =20
> -	if (file) {
> -		i_mmap_unlock_write(mapping);
> -		uprobe_mmap(vma);
> -	}
> -
>  	if (anon_vma) {
>  		anon_vma_interval_tree_post_update_vma(vma);
>  		anon_vma_unlock_write(anon_vma);
>  	}
> =20
> +	if (file) {
> +		i_mmap_unlock_write(mapping);
> +		uprobe_mmap(vma);
> +	}
> =20
>  	if (remove_next) {
>  		if (file) {
> --=20
> 2.35.1=