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, 11 Apr 2023 13:41:52 -0400
From:   "Liam R. Howlett" <Liam.Howlett@Oracle.com>
To:     Peng Zhang <zhangpeng.00@bytedance.com>
Cc:     akpm@linux-foundation.org, linux-mm@kvack.org,
        linux-kernel@vger.kernel.org, maple-tree@lists.infradead.org,
        stable@vger.kernel.org
Subject: Re: [PATCH v2 1/2] maple_tree: Fix a potential memory leak, OOB
 access, or other unpredictable bug
Message-ID: <20230411174152.4j2clkhb5xxh5rkk@revolver>
Mail-Followup-To: "Liam R. Howlett" <Liam.Howlett@Oracle.com>,
        Peng Zhang <zhangpeng.00@bytedance.com>, akpm@linux-foundation.org,
        linux-mm@kvack.org, linux-kernel@vger.kernel.org,
        maple-tree@lists.infradead.org, stable@vger.kernel.org
References: <20230411041005.26205-1-zhangpeng.00@bytedance.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230411041005.26205-1-zhangpeng.00@bytedance.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?CZSnlC/d9AZimTeQ8t7/rBRhe0pI2IlTxMNysLkNFaMUV5o9J99Qily8ATUC?=
 =?us-ascii?Q?3PwMGkYCjY9esiDZexDMHJMT5IkAErVLW9fTdMih0pYlVMUUbZU9rBXCQQPT?=
 =?us-ascii?Q?GrHxOzewkyrz957E3rBPLlJW5x1YLtHB2rQlbvZc5qFQZaWeaHkiy8LAFa5E?=
 =?us-ascii?Q?xeeLBVk5oHoNRYUOwFwHsYJoj/C+KvFzMCddDcaInsePxfO0kHYLGD/HpAo9?=
 =?us-ascii?Q?ug2Ia1x9NuNGWXPPziruDRgu8XY0whNHUxUmCVuaLEeEWNNNSH2XJyUpZPQK?=
 =?us-ascii?Q?IJOVvYCvWDiNpWwHsBT3+StxQXTPOZgmyZ1aIUzVTmo67L78XgV21bIWqy4E?=
 =?us-ascii?Q?x1BResN/MzsGeVCGNp5gfFZXXhX7lHKQ0Jv5xQuy6AFTVwiafLcjckSLofua?=
 =?us-ascii?Q?4VnCeUl88ixUyiYoAx7qjGED4xI5Lj1O/msvszi08SqMAuQ9Cbf8ZxWaUVez?=
 =?us-ascii?Q?izsZycXxTy0i9Lf3mGFfwHxsbSMUHsRH4Zl+KSdVPHbHb2swcwsm8OWXXcrg?=
 =?us-ascii?Q?54LWBsKSZZJM32SOobG70tbmAEbC87Y2Nw7CbCdm+fDbE13KQsO+YCy8u7dd?=
 =?us-ascii?Q?rZD9vxSKaR8LzLREVGVjHZZ91hY251p9tUeB6BTwgJzDZTsqNEaeop1IWHU/?=
 =?us-ascii?Q?niZuqnuyb2PobxVJAxzwQML+SId5sbd7p21PNAnYNjtpAU7cCQtv/0Bkr45N?=
 =?us-ascii?Q?pVQN5JGqRUHbYTZJ+umz6enu3pZuzJSyU9jfmFfYItvgweBsi4YtDfOPx+Ko?=
 =?us-ascii?Q?CC9p0QaYC963fes9YG6Y+zHqRb358ls4/unIA22+Ayc362hGfd01AkUj/Jt9?=
 =?us-ascii?Q?nkv40uT8VKKRsJuLoRd/iaH7QwkCh0bGhE5Fr2BE2xzlEQa4ueUjbnrKu6FL?=
 =?us-ascii?Q?ERtN/uWC0ercYVVFO6EOCzvTJBHaKXv7lWpbLQqRermD8M22bPyaGDay/1x5?=
 =?us-ascii?Q?HGN7CHZb72At2TJk6vEFaOXyaLBQJTfgD/mf7BiNsP8nwTUYeQd9T+XRNkLy?=
 =?us-ascii?Q?o1lJ6RljpUVvftVaZ2SivTUKtGPIZ/yeiROVXkheO/bzeOfIebvOXtkG2upJ?=
 =?us-ascii?Q?tILC0EDmMvlyNpE6uCUH1yigmO6Blhgm7NwH3C15EAx1MUN1SsagOVXB+y1K?=
 =?us-ascii?Q?7hvmm/4rSsVqjF0b2Ue+wEHD+3uEMCSKuLGnoKzqaNWuxp6fHM/R4xwht28H?=
 =?us-ascii?Q?D8WdpANiiC/p08R5rWjt4gycpS8Y1XrNPYf1J8Q0ka30ZUYgYCkjlM1lsqdc?=
 =?us-ascii?Q?xg/jnjWo4A2DoKwg17T9AihvkNLdAA7k+l1d7TlNjiADZjeoEsCUunr7OWOB?=
 =?us-ascii?Q?GB9nHw8kdtxE63vXc3kr32JiilULOSdLGk6jlh4Sjv6B2TE+Wuk1kMHOAXTm?=
 =?us-ascii?Q?/luliHv6KwMZMZLPB3DgPta1LU2yqgKm1wGVOihR8jOhS+UutfqnSBYXD/Ch?=
 =?us-ascii?Q?awCNgS99Q7ArLMq3AFwtibTK0MFN13hGnwcIdSeY9ocz1n6MxlEDBHctR1Ii?=
 =?us-ascii?Q?ATA4kZbZl+q2so63AekpMw/ea07tmHWQ7ZZ3ZxlsgjpOiR2u5+xYA8ddeTgh?=
 =?us-ascii?Q?McRFzuE5IQ87ku4tQ9ahpAeq0qFmhl5b0nTH/Gi9VDtFxZX3RPGDIyOTJr2T?=
 =?us-ascii?Q?fw=3D=3D?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: =?us-ascii?Q?bzhCEEM2IU1aqciVLa2+ghMTalT3j5YZUTmHSYKXPIPd7RLP1rDPoSkqwH8M?=
 =?us-ascii?Q?PoxelSdoP+5ttLJL7v/hiUpvmrvWyhEE3JSRZQM2N4qRp1d7v8WwwjCK3I5j?=
 =?us-ascii?Q?n07HKfjFtdN/k8FNgi4oW+31wDW2iCsfxF4igFfmke3fg+1zsryCrC3vY9Gf?=
 =?us-ascii?Q?QbaNlEYEJPcwVI2v0L00FnbleAkn3fA7DJ0nmFzrIFM4MBjYhqg8FarXRGMx?=
 =?us-ascii?Q?vByVTyA2nfsdHLlyemOuNRNAWyFYnSq+qwdTUlezNfJhTxkBT40fqGu1qfD9?=
 =?us-ascii?Q?qbRTVlDZs1zDJ8E1SChos9NhYizDjAtLnpFNssbKD5xvAv8GNxoCDbeLW/qY?=
 =?us-ascii?Q?oCJS07TbL0UyYsnZF3+pZpAyX06BZbTN2ltd2hCxtvE5BWMU0JWciMfBMSxC?=
 =?us-ascii?Q?xQzI6iW+wDbUjRhVAo5qSMs8HcUwOwDeLsHmwInsKNsrZ8W/tsXIxtfJP9eo?=
 =?us-ascii?Q?E7ft3t99EO2qDz0JZAGITjgthcu5trNZq171sdA5HRPdW/2left/pTwNrQ9M?=
 =?us-ascii?Q?sTGOehJ7gybvbnOQN1kC1S6usZ7rHZWthodgqJP1ahSRYPUD6GmiYC0yfCup?=
 =?us-ascii?Q?cGCrjPw98IxR1F+DfNmsZb5bOsqMSP68tX1cj6JMQ3MkoLKN2o/cBJsQzqrt?=
 =?us-ascii?Q?6Fd2x1+CC8L6I2uuJnMaKzPlpjw2DbsbvnYx+dVFAYF/Mw8fEP3lDsx9ZKlx?=
 =?us-ascii?Q?jiGBbxfXVObHJNwM5/KeZtlqZxxl4M3LHzxQy/vcwTXjPjmJ5MH5WE6YY+vN?=
 =?us-ascii?Q?axnUmlI6+25tGtMOaXDjNbN2U2zGjTT8aIRzHPVhDmy6UiFxRLpTh/yyR153?=
 =?us-ascii?Q?xiqsbKiyVgL6YJUnV0bW/4KIIcOjiRW0gqRdrVEP7soPv27/NzCw/wuC5Aku?=
 =?us-ascii?Q?rpRxabnerkqht5mE8j8u7d9AsG1jIY19OW/7rY7qlYNa5mhwkSAE8QY8isEx?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: d9d86b7a-8058-4ec0-7d4b-08db3ab40ac5
X-MS-Exchange-CrossTenant-AuthSource: SN6PR10MB3022.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 11 Apr 2023 17:41:55.3708
 (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: MV8pqlb7SGn2tpHGPoIasPfYPapFW979Hpv2k1jZbcjJkw1EFzyB6h6oTK3TFXm0qQe1JsS4ifNarigk0HPPqg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA1PR10MB5784
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-04-11_11,2023-04-11_02,2023-02-09_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxlogscore=999 mlxscore=0
 malwarescore=0 suspectscore=0 bulkscore=0 spamscore=0 phishscore=0
 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2303200000 definitions=main-2304110161
X-Proofpoint-GUID: iHUyG9x5A_NUaRYLSkMyo-2Vx8WIUO33
X-Proofpoint-ORIG-GUID: iHUyG9x5A_NUaRYLSkMyo-2Vx8WIUO33
X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,
        DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_LOW,RCVD_IN_MSPIKE_H2,
        SPF_HELO_NONE,SPF_NONE autolearn=unavailable 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

* Peng Zhang <zhangpeng.00@bytedance.com> [230411 00:10]:
> In mas_alloc_nodes(), "node->node_count = 0" means to initialize the
> node_count field of the new node, but the node may not be a new node.
> It may be a node that existed before and node_count has a value, setting
> it to 0 will cause a memory leak. At this time, mas->alloc->total will
> be greater than the actual number of nodes in the linked list, which may
> cause many other errors. For example, out-of-bounds access in mas_pop_node(),
> and mas_pop_node() may return addresses that should not be used. Fix it
> by initializing node_count only for new nodes.
> 
> Also, by the way, an if-else statement was removed to simplify the code.
> 
> Fixes: 54a611b60590 ("Maple Tree: add new data structure")
> Signed-off-by: Peng Zhang <zhangpeng.00@bytedance.com>
> Cc: <stable@vger.kernel.org>

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

> ---
>  lib/maple_tree.c | 19 +++++++------------
>  1 file changed, 7 insertions(+), 12 deletions(-)
> 
> diff --git a/lib/maple_tree.c b/lib/maple_tree.c
> index dd1a114d9e2b..938634bea2d6 100644
> --- a/lib/maple_tree.c
> +++ b/lib/maple_tree.c
> @@ -1303,26 +1303,21 @@ static inline void mas_alloc_nodes(struct ma_state *mas, gfp_t gfp)
>  	node = mas->alloc;
>  	node->request_count = 0;
>  	while (requested) {
> -		max_req = MAPLE_ALLOC_SLOTS;
> -		if (node->node_count) {
> -			unsigned int offset = node->node_count;
> -
> -			slots = (void **)&node->slot[offset];
> -			max_req -= offset;
> -		} else {
> -			slots = (void **)&node->slot;
> -		}
> -
> +		max_req = MAPLE_ALLOC_SLOTS - node->node_count;
> +		slots = (void **)&node->slot[node->node_count];
>  		max_req = min(requested, max_req);
>  		count = mt_alloc_bulk(gfp, max_req, slots);
>  		if (!count)
>  			goto nomem_bulk;
>  
> +		if (node->node_count == 0) {
> +			node->slot[0]->node_count = 0;
> +			node->slot[0]->request_count = 0;
> +		}
> +
>  		node->node_count += count;
>  		allocated += count;
>  		node = node->slot[0];
> -		node->node_count = 0;
> -		node->request_count = 0;
>  		requested -= count;
>  	}
>  	mas->alloc->total = allocated;
> -- 
> 2.20.1
>