From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 65686C3DA7F for ; Wed, 31 Jul 2024 10:06:53 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id C9A2B6B0085; Wed, 31 Jul 2024 06:06:52 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id C49BE6B0088; Wed, 31 Jul 2024 06:06:52 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id B11C86B0089; Wed, 31 Jul 2024 06:06:52 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 91DE66B0085 for ; Wed, 31 Jul 2024 06:06:52 -0400 (EDT) Received: from smtpin06.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 308C412024D for ; Wed, 31 Jul 2024 10:06:52 +0000 (UTC) X-FDA: 82399619064.06.0285EC3 Received: from sin.source.kernel.org (sin.source.kernel.org [145.40.73.55]) by imf25.hostedemail.com (Postfix) with ESMTP id 0CE9EA001A for ; Wed, 31 Jul 2024 10:06:49 +0000 (UTC) Authentication-Results: imf25.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=qbqpl8h5; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf25.hostedemail.com: domain of vbabka@kernel.org designates 145.40.73.55 as permitted sender) smtp.mailfrom=vbabka@kernel.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1722420369; a=rsa-sha256; cv=none; b=rZ0TIgm9j3TZE738D41mAUGfgF4qvwGaqkpIiw834OIbeaeG+eS9TF1JniKeaXWmzn291Y 4VADiU/aIXH2fPi26YxHfSgmwC5ON+9Brxbgr2e+5Pc/k2NO6RMRqpWYzepeky7D+KpyKB H+tNdnaOErWw7FI4NheumtVU5uiwGIo= ARC-Authentication-Results: i=1; imf25.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=qbqpl8h5; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf25.hostedemail.com: domain of vbabka@kernel.org designates 145.40.73.55 as permitted sender) smtp.mailfrom=vbabka@kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1722420369; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=wfK4pkRP2nwRVpA0I85JrEYpSfNxasuemQKDcF3yJmg=; b=d7waTBVzFXnoKZYux60g1Hs1cIH7tx19YLGNz4yEmq3H+XiNWiH3a52KQziViUJdcxiU6H gjnkmfT0oWAtZfvFtWxWBvA8TE2uv/ksDjJ7oGJzCIOWpMBBu30M9Ie9xWoPM7ofuDljUH D3zXYXx8S6g7zTyzXzBhUI0OV+rZEE4= Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by sin.source.kernel.org (Postfix) with ESMTP id E07ADCE12CB; Wed, 31 Jul 2024 10:06:45 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id ABEC4C116B1; Wed, 31 Jul 2024 10:06:43 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1722420405; bh=AvjBWHvaQdv61maw9aImbeXOicW4mkIR+r2NHBwTos8=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=qbqpl8h5FcDPo9O0ygf4AR9FnAyWIdcLjjhithvfG4xnOgVWojiDlSdoVN+e7DHMr +YaXXZosDcT+Em9sMDft4T+LdZMsqAUfAcBguJ1fnAPZq49Tg7SvaqSsWp2c/0NgvF deEU1VG0fjnM88vxbKOCyz9OHHI0k6TMC7LepvivV3rwZpQLAE6H6ZbSM2vN/vuqd6 vNENol7GTiX8LDr5SvdX2e7Kbhhy6R6g/3iZwoiwLJ7naylEiU3gQKplSnHq9LpHBa Z0v4+yc9oJjDcU/uz8OtGHM3XtNFp8WWXUGYvEYW5pK60lzwlKywLqjiYlb78NYqPw T+oiIwECAxHhA== Message-ID: Date: Wed, 31 Jul 2024 12:06:41 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] mm: list_lru: fix UAF for memory cgroup Content-Language: en-US To: Muchun Song , Andrew Morton Cc: Muchun Song , Johannes Weiner , Nhat Pham , Linux Memory Management List , LKML References: <20240718083607.42068-1-songmuchun@bytedance.com> <20240723174540.18992614c476d77e7d9fb1e6@linux-foundation.org> <62BBC2A6-D6C3-48B8-B049-932E3BC16F31@linux.dev> From: "Vlastimil Babka (SUSE)" In-Reply-To: <62BBC2A6-D6C3-48B8-B049-932E3BC16F31@linux.dev> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Rspamd-Queue-Id: 0CE9EA001A X-Rspam-User: X-Rspamd-Server: rspam05 X-Stat-Signature: knbfht8hrp5ghpngbrceyxpcaq8qf3zw X-HE-Tag: 1722420409-24271 X-HE-Meta: U2FsdGVkX1/pWsKE5z0ZEYHt7txWIeanh/3eKv5dMl4nBlVeEeUjO8t57uYd9p6gi+RXUdlQa2pWoJmo4MZOkb5pVmMMjXOyWacqLYyd97RESPj3vW3SnqTCaMwu86fR5IjH4xrvsNl4v2q/yRLDAtDTarsSeicdy6ufYLy5xFgW0ifBuDGV2LNkR0XuSZipo9MxsRknju9AswxMCpyie+JM6hH83gu8aOiVARNLKDrIteY2stfFp2rCJvyPTPAiusMtgHwtFSFdunsRGLWUQhhjBCq9ijVcJMiAn/3h3sCEmvzJ2lnFxWGtmGw+DtPLtPcmtZfABfIXAKIBhL8F7Yt0hIdgPDYGSQ/yd2BAE7ORXp6vJ4WWZ53Ng+7aVWDnDGuiRAepyPp+qWNm6gNpxjkXImumg7JDgOMGNStXZv4sSzV8/2EwEk6SB9xTTvUUk4hwI6hsRadOG8u6i9YMkCuaaNhNkYZMW4YF+gYlK6/IQbvhab4G9VML2H+F7tGg6wNs1IYeXsspBSo3SNn5wT/V4nKX+IW5tT3kT4LStRf34HBcPzIj+2j5bf1bjMn3KuQ2B14uDuUhj3LYNYX9pkqgR68keWuOh9r2OqB0yiqSs0dUg/cg9ZMVx1627LCPn5g3KOg8rWnk8+pPFuGe96L0zIrVmHJdb1GM3Hs7p7+JaimnYT/V9qj+m1bkka7ogm8MeYKtu4g3yDAJF5oMyrKoqmdMOKtx+HJ1PaF+ZrIBFf29GgcYOBDdXNyyzqoImQuJgAj28Zcs58wbJ1t/V8HJDoheBWMlZ1koiD3akL/wuvWGwUANM/5W0AZ7gtamC1R2e96pbSGjsAGwhtREMc9lxAWzKXIemBNwbJ0/h00LY7WjRcRGhmDEw+XwFLWb92ofKXoQLLfilW6FnGwnTS473p+0H57Qn9MIhFT7swNSOJAzN7NElMkRg6a3aqa2pzj9jFATgNLBiNhGRHu UPa5zWeH VL3YksbmUgsqWCaWHpHUdWhbBK8+wF23ToAxHdFcyS4enB6jy2EVP7FCPDDnoe/XnptRnnKQPiV6uZr9/9S629v6Aod8ZVpUvtZ2CT2A9MAthnH30p/8VpWOGZ4gjten/vfeAHDmTr3FdCwsCJ689DjPVdamVKtLDMVisfxHPwlL3Sd7L2KDaPk84JF8604fSj0zxTskLQUVjszjSLcEKvGNhsJri0IyPjujxnqOkGUNhgOtXC2bX3K2GU6Toib+02/laQsHarLtJVGJc/L9KMr+LX4QdxrUmvEYiYg5SqxXvOF1KsEXzYQNVZKDvYGd2XnwG5dDQbpabmj1AMMyz9Cq3yt6euhLDJJaJGnmtFG31yJWEHznzYB82+m/eIP0H1fQF X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On 7/24/24 4:23 AM, Muchun Song wrote: > > >> On Jul 24, 2024, at 08:45, Andrew Morton wrote: >> >> On Thu, 18 Jul 2024 16:36:07 +0800 Muchun Song wrote: >> >>> The mem_cgroup_from_slab_obj() is supposed to be called under rcu >>> lock or cgroup_mutex or others which could prevent returned memcg >>> from being freed. Fix it by adding missing rcu read lock. >> >> "or others" is rather vague. What others? > > Like objcg_lock. I have added this one into obj_cgroup_memcg(). > >> >>> @@ -109,14 +110,20 @@ EXPORT_SYMBOL_GPL(list_lru_add); >>> >>> bool list_lru_add_obj(struct list_lru *lru, struct list_head *item) >>> { >>> + bool ret; >>> int nid = page_to_nid(virt_to_page(item)); >>> - struct mem_cgroup *memcg = list_lru_memcg_aware(lru) ? >>> - mem_cgroup_from_slab_obj(item) : NULL; >>> + struct mem_cgroup *memcg; >>> >>> - return list_lru_add(lru, item, nid, memcg); >>> + rcu_read_lock(); >>> + memcg = list_lru_memcg_aware(lru) ? mem_cgroup_from_slab_obj(item) : NULL; >>> + ret = list_lru_add(lru, item, nid, memcg); >>> + rcu_read_unlock(); >> >> We don't need rcu_read_lock() to evaluate NULL. >> >> memcg = NULL; >> if (list_lru_memcg_aware(lru)) { >> rcu_read_lock(); >> memcg = mem_cgroup_from_slab_obj(item); >> rcu_read_unlock(); > > Actually, the access to memcg is in list_lru_add(), so the rcu lock should > also cover this function rather than only mem_cgroup_from_slab_obj(). > Something like: > > memcg = NULL; > if (list_lru_memcg_aware(lru)) { > rcu_read_lock(); > memcg = mem_cgroup_from_slab_obj(item); > } > ret = list_lru_add(lru, item, nid, memcg); > if (list_lru_memcg_aware(lru)) > rcu_read_unlock(); > > Not concise. I don't know if this is good. At such point, it's probably best to just: if (list_lru_memcg_aware(lru)) { rcu_read_lock(); ret = list_lru_add(lru, item, nid, mem_cgroup_from_slab_obj(item)); rcu_read_unlock(); } else { list_lru_add(lru, item, nid, NULL); } ? > >> } >> >> Seems worthwhile? >> >> > >