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:   Wed, 28 Jun 2023 09:13:30 -0400
From:   "Liam R. Howlett" <Liam.Howlett@Oracle.com>
To:     David Woodhouse <dwmw2@infradead.org>
Cc:     Linus Torvalds <torvalds@linux-foundation.org>,
        Vegard Nossum <vegard.nossum@oracle.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        "linux-mm@kvack.org" <linux-mm@kvack.org>,
        linux-kernel <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] mm/mmap: Fix error return in do_vmi_align_munmap()
Message-ID: <20230628131330.ufrp57vnfpynlapq@revolver>
Mail-Followup-To: "Liam R. Howlett" <Liam.Howlett@Oracle.com>,
        David Woodhouse <dwmw2@infradead.org>,
        Linus Torvalds <torvalds@linux-foundation.org>,
        Vegard Nossum <vegard.nossum@oracle.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        "linux-mm@kvack.org" <linux-mm@kvack.org>,
        linux-kernel <linux-kernel@vger.kernel.org>
References: <ef2c7c0eeb166acf050597f49eb118d94f18bd39.camel@infradead.org>
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
In-Reply-To: <ef2c7c0eeb166acf050597f49eb118d94f18bd39.camel@infradead.org>
User-Agent: NeoMutt/20220429
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?MVU2OGhQWkhhNkg2M1VvSllydTR4V1BUTzV5cVdGZXp6NmIrMHhQdzh0bDYz?=
 =?utf-8?B?cDJ4NWtNcUN6VzBrQ1ROcHFQbkhYM0R4NFZpWXFPY0ZaOXRmM3d5Q1kvbmEr?=
 =?utf-8?B?Y2M4SjdFczZMNTN3aGF6UFNlOGV5eW8weUhvb2JpQ242V0NyRlErVzJub2lq?=
 =?utf-8?B?bHg4UDh6ZDI0WmgyYnJrUDBFY1h3aTBVcjU1VWovQStwOVhZaXFIQ1lWRWxF?=
 =?utf-8?B?Q0cyeW1FdWgzMVV4ZXZ0VmI3Njh2MkROWGZwajd6MktBdkk3RW5iS254UzhY?=
 =?utf-8?B?eTJsVU9mck01LzRYNGpEd1ZVaUdiZlZBdXEvWFpqRTlKaTk3WFlTdlVMZXMx?=
 =?utf-8?B?V1BCWW1pY1ZvYlVQeFZ4bGlxWEVBUWpSVEtYWmp1U2I4MGNVb1Z2UWRFeTF0?=
 =?utf-8?B?K2FmZWJkY2pEL2t5VXUxamQ2MkUwL3VFVzh5Ni82MUxUa0cvWVdjZ2tCUmxt?=
 =?utf-8?B?VVJ6bW9UcFFEUmMyZGo4RUZ0TDIwSUo1TGZ0VmdVUjNKeHlGaiswVC9Da0Zl?=
 =?utf-8?B?N0YvQ0NBNGthbVZHUjdTamNOTzhwdCtjazFQWjhkRDhxdjZ5T3llaGpPM0dU?=
 =?utf-8?B?eDFOZ1hkdG9MSENBQ2k2VVJySWJUOElFV1VrR3dsM094QlZCNmkyVlJCMWdY?=
 =?utf-8?B?VDNMWWF5SWtwLy9tRkdkaUVONzB6eTZZMkxiclpRTmpPaUJTNXYwcU1jdElF?=
 =?utf-8?B?WlhwYk5aZkN1S295QWViNXh1ek96V1ZtOFRKY20xTmtxUmxZOUgydkdQSWJ5?=
 =?utf-8?B?MmUyUUxabStVbzlITVpSSlYyOVVydDVUTmZQLzNISTkrSlMxZFlVYlAxRFpu?=
 =?utf-8?B?bjM3TDVLdnVDRjNYQlFLUDQ0YTlLMXJHMVpKbnhTYWlYbEZyWjY5eUlNNmhT?=
 =?utf-8?B?UjdZRUkrS0VlQlpTa3BxYUk4b0ZwdUtjR3U5L29nNlJIVWh0VGlNdktxOE9X?=
 =?utf-8?B?RG1OQTFhajFzVC9lcEtuUDNHUmpLUG1wMkFMKzhUb045K1NTL0k0QzczaUlT?=
 =?utf-8?B?eVVsT0tBVzhzZjYzQW5rWkgveEk4cWtVSC9vMDBRKzBFWmlGL1VWdzJkZUlq?=
 =?utf-8?B?dHVMT0pOakc3OW5OOXVNdndqaTlzT0RLSEF4RitNWngxV3B3dTZaR1JEWHIw?=
 =?utf-8?B?aHRoOTZjNDFaSW9CRlQxVUFkU012UGhiMmFnUy9SOU51cG1PUHZjaDZRZ092?=
 =?utf-8?B?YWVkWEtFR3gxcFdOVHNGbUFDUGltbVVHbVJsSmVIRHc0Q0xtN285L3hleDZH?=
 =?utf-8?B?dmRkdTR1UHp1eWo2WUg5YW01ai9UN2FKcFI5aHBiTVprTmQ5bTUyU2tFQk5j?=
 =?utf-8?B?b2QyY3RPMHRFYi9ZdnRoMVB4ZkJGOGUxVklKTi8rRERFNjdjTlZjOElDQmVI?=
 =?utf-8?B?VzhMOWl4cFY2WlMraEZYc0FETVpLMFZjVURrMFl1ZW0wNHFDUlNqa1FTYlRt?=
 =?utf-8?B?dDNGYU4waVlIeVZxODByL0VTMHd1cnNSWitKRldaY1l3dWRoVTNMMlhuUkFm?=
 =?utf-8?B?a280ZFZzKzcwQ1BmbUZmenVGQWF2R21HQ1lpUlZLay9MVCswUHpVWlFzOUVQ?=
 =?utf-8?B?RGRIU2ViQlhZaGZwaUszbDRRbDVlZGVDT0k3UC9lVFI0M0VKb3FLMkdjVXlL?=
 =?utf-8?B?OTFJU01Zb0dWSkdTTDM0dWtTeUZ1SmhJSzloUmFVcHlKSnE1OCt1dmRmS1pJ?=
 =?utf-8?B?NG5qaEhrSnc5KzZkUGdqbDl4QURGaWd5NXFQRERQNFV0SDB1cHNFdnNNZFB1?=
 =?utf-8?B?WWJFYW44WFJIQmtaNmxzSU5sQTNuR3JSR1FXaVlhL1VuNk16OEhkSzl0cUp5?=
 =?utf-8?B?Nm16NnpDMlMrU0lKaHhNamhKK1BNeFcvaVZqZ0ZTcVNhWTJSb2dyK21NNTVX?=
 =?utf-8?B?a0srNTlDaGkwSUtVUCs5b1hScjdJaytXVzF1aU1jdnlxNFpoOFRxaWU0djVa?=
 =?utf-8?B?UU5GNlZYSVA5b0U5bG1xd2ZaWHc1NThRM2cxV0JiN2l2bk05TXhlWEo0RjY5?=
 =?utf-8?B?R2laUUVMbFk4cVE2Q05qdWV4eGtNWFdVSmt5c3MvZHZCQTVoNVVXTTZMWTlT?=
 =?utf-8?B?dnJJTEgxTDFhZVVQaUhTbk54U0hBbXd6TUR6MmFwc25vcUtTZHZvclduT21a?=
 =?utf-8?B?UGtsaGxYbzhneGJiMVBNbEFVVGkrQi9NSURESzhNUWN5blR5WTFNZWpGVS92?=
 =?utf-8?B?dkE9PQ==?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: WcyGu6wbRtHaxaZ8+Nr8Ophk84GzugwkxXAw/pD/nikOzeiF8+nJ10R9fOG0Ij6HCMkqy/lIiYezqm2f3pbqUL2jQBkO1AIOb8T2C1qcplm085HR/GlsgbNXEJi0+NwRGhHe8T/HuWp6aO8vgKi/w93IqADnzqi8BJBmmM2xpmykM8Yz8eJ7SULwbRvOa8ivdoSs7HQmQz+0/oB0okcn9gtQ0NjdCZwlt1uNQX5sCD6JXgDXjTgPNuNj/hl2+5pHMsoQsAgOjlfU8nYVP2ydhpsrt171/b6oaPOrI1nMju3Nu52tM+lIrVkmOBTH6lhgJSGpULh4Cg9FiZtWa/7dYO2pgNQnXwyBmQ+OwHMImPWYvvA4huFNnISQV7uvH6Gk6Hv/87LA0uwSu4qJbMmqBLuJpCXeStRzoRGxruN/pIufF7DRtw9WVz21SadltmuXg0D6nW0hrEXqqeEPv+Mmn6ANT4gO/p1l5n2VEkTuMk5RARvxFITkWl4AF80UvXCP8W4q7in/5qAAxYDfWFJ3iGfLZH3c/IPoJd5de9yp7iIz2aIQ4l4OPCfjbXhvzm9+pPPyIUQoKf7LPmfvBKf4Z++aIjzXY0HwUN9y15jZLEvcWCSR03Ag2Tk5XEnQP4CzQkSdJX0OpcsyPyxiwH3HUlxD6GekCr+WChcpAl0+LmjrAR2wPJnAMrXkPiU1s5uukGfzVwXZvtHvPwwg6yYKDDTSBYcEsNRffDZ2zYoGvll0iqDtGReIocL/i1cyb3Pz5A31wia2RC6yGFs5rcRxs6kZsgsswNhLaG8SnQZx3MOInzD9TAiwf64jDwhWjWk8J6vvKKcv4slhtcHwoB3eLDdHO8s7p60pAgTwp+Ybw6HtXhDwcsPDTKE1vA4lVVHd
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 7626e85d-359d-40e1-410a-08db77d979b0
X-MS-Exchange-CrossTenant-AuthSource: SN6PR10MB3022.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 28 Jun 2023 13:13:33.7342
 (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: A4muFoJFdCD9Ma7M0lbQGjrN8sbYjutdg/Rkuwp2aPDWTrv57Se2GcRVU1+GnEGproX/THmkzqawhyhOi1DUjA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA1PR10MB7472
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.254,Aquarius:18.0.957,Hydra:6.0.591,FMLib:17.11.176.26
 definitions=2023-06-28_08,2023-06-27_01,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 adultscore=0 spamscore=0
 mlxlogscore=884 malwarescore=0 phishscore=0 bulkscore=0 suspectscore=0
 mlxscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2305260000 definitions=main-2306280118
X-Proofpoint-GUID: oXacdQ-h-FlBBcwMSTGLAISZvXZW6821
X-Proofpoint-ORIG-GUID: oXacdQ-h-FlBBcwMSTGLAISZvXZW6821
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

* David Woodhouse <dwmw2@infradead.org> [230628 06:43]:
> From: David Woodhouse <dwmw@amazon.co.uk>
>=20
> If mas_store_gfp() in the gather loop failed, the 'error' variable that
> ultimately gets returned was not being set. In many cases, its original
> value of -ENOMEM was still in place, and that was fine. But if VMAs had
> been split at the start or end of the range, then 'error' could be zero.
>=20
> Change to the 'error =3D foo(); if (error) goto =E2=80=A6' idiom to fix t=
he bug.
>=20
> Also clean up a later case which avoided the same bug by *explicitly*
> setting error =3D -ENOMEM right before calling the function that might
> return -ENOMEM.
>=20
> In a final cosmetic change, move the 'Point of no return' comment to
> *after* the goto. That's been in the wrong place since the preallocation
> was removed, and this new error path was added.
>=20
> Fixes: 606c812eb1d5 ("mm/mmap: Fix error path in do_vmi_align_munmap()")
> Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>

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

> ---
>  mm/mmap.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
>=20
> diff --git a/mm/mmap.c b/mm/mmap.c
> index d600404580b2..13128e908470 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -2387,7 +2387,8 @@ do_vmi_align_munmap(struct vma_iterator *vmi, struc=
t vm_area_struct *vma,
>  		}
>  		vma_start_write(next);
>  		mas_set_range(&mas_detach, next->vm_start, next->vm_end - 1);
> -		if (mas_store_gfp(&mas_detach, next, GFP_KERNEL))
> +		error =3D mas_store_gfp(&mas_detach, next, GFP_KERNEL);
> +		if (error)
>  			goto munmap_gather_failed;
>  		vma_mark_detached(next, true);
>  		if (next->vm_flags & VM_LOCKED)
> @@ -2436,12 +2437,12 @@ do_vmi_align_munmap(struct vma_iterator *vmi, str=
uct vm_area_struct *vma,
>  		BUG_ON(count !=3D test_count);
>  	}
>  #endif
> -	/* Point of no return */
> -	error =3D -ENOMEM;
>  	vma_iter_set(vmi, start);
> -	if (vma_iter_clear_gfp(vmi, start, end, GFP_KERNEL))
> +	error =3D vma_iter_clear_gfp(vmi, start, end, GFP_KERNEL);
> +	if (error)
>  		goto clear_tree_failed;
> =20
> +	/* Point of no return */
>  	mm->locked_vm -=3D locked_vm;
>  	mm->map_count -=3D count;
>  	/*
> --=20
> 2.34.1
>=20
>=20