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:   Florian Schmidt <flosch@nutanix.com>
To:     Johannes Weiner <hannes@cmpxchg.org>,
        Michal Hocko <mhocko@kernel.org>,
        Roman Gushchin <roman.gushchin@linux.dev>,
        Shakeel Butt <shakeelb@google.com>,
        Muchun Song <muchun.song@linux.dev>,
        Andrew Morton <akpm@linux-foundation.org>
Cc:     Florian Schmidt <flosch@nutanix.com>, cgroups@vger.kernel.org,
        linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: [RFC] memcg v1: provide read access to memory.pressure_level
Date:   Wed, 22 Mar 2023 14:25:25 +0000
Message-Id: <20230322142525.162469-1-flosch@nutanix.com>
Content-Transfer-Encoding: 8bit
Content-Type: text/plain
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?yorG/bcxx39CnBGxnMWjQ0uqVmaTChKf3DwWhO0/DTNENrM9uaPIkkFHo63f?=
 =?us-ascii?Q?DKCzec5pvR+dGbXg4tUPxL6QuDOJTuXN+Vp5h7rctneNEv7l9BLGWFekRpcD?=
 =?us-ascii?Q?E9XvbWqZ2GVPgpB5aHZwPzdnW3c+ro3yw4LQBNy77iRY7UoQ+yHPMsPyjZ5H?=
 =?us-ascii?Q?liCSeJeOYAZg2okx2VdL6nQSLI4jxgftVmGuJRpd0GptdQwdzOK5aED4ufru?=
 =?us-ascii?Q?sxQUdctWSnbvoIGrcqsun4em8pSca+5hMu3Lbe5HZHv6qvfy/rR6hRKpstYK?=
 =?us-ascii?Q?xTgjA+MU/MGo7hdcJr5xY23spWdqB4CNJBJ+3fDVT5yCvIJdloXHpjK8Z5fm?=
 =?us-ascii?Q?SLrgTr0S+yTdeoGP9L+D85tnTEsQM2RinGeBQO2Lg9lp/puEQTv8RpsPcbB+?=
 =?us-ascii?Q?9ly8TEQ2mF+1+l5x5lrRDaEn1MqmsafoPS6ZyZVPk3K7uhf3s1uzd6QTJ5bi?=
 =?us-ascii?Q?ZTzVrqim1l8zffO/xXGWnzLO41bZrlC2mSvklPJhmkFKj91FoALNDvyGOIc+?=
 =?us-ascii?Q?Jzw2aNsoD9VTi0lAbYt2HV+c2R5jTUKWvTM5jX+DVNLkj4j0F+nsmc4U3Zyt?=
 =?us-ascii?Q?rsLiDSgtiWg/rYD35S22SxVBK2miki6cz858KpmnybJqv+utjtD4l1fR0Yk/?=
 =?us-ascii?Q?kor+gWXrDEgy/388K0O6W0PwFp6aJULiA39uCLIbnKlvWRKZWXJbVbiXGLyJ?=
 =?us-ascii?Q?KxKSKue2BCjfXnMbHG3CH2jgBkhGgSGEKE852T/XRq79LPncxKy/ZBBfWHGc?=
 =?us-ascii?Q?kbCTNOuQ9qjReV6QYRvT6PxOsI/e5JXUMFlLiWZk1+/HwkhZA4TRzhtr0vp7?=
 =?us-ascii?Q?GHCEymUZDf1vE93iPHlDPW4AVGOKS8NSHsXI2k9cExDqS6Vj7No8xCPr1hyy?=
 =?us-ascii?Q?O8TTzZDBVEUysM8aoBq2zqHOg+OqX8m0EE/hR5+O6kgKfRg1jpASStoImtsP?=
 =?us-ascii?Q?EA3GgjsSCEZ7jknXrVDL4bKIRzdONnQfH/eOBrTOWxhPVg17EXCmfjuiiF7D?=
 =?us-ascii?Q?85J6gXjGNMtuNR4AhalxYFHfDt13EWpSXwqA41XFyYhMtIF/RKo4UJkYnArB?=
 =?us-ascii?Q?7M+YnISGhQZ/F6k4ghzhNWqPYPuQWXbXum3bG77di2N1bHZNfeKdpyPaPxqP?=
 =?us-ascii?Q?bGmjErvVgY5LDyFghATg56mwncXl9NZRCkWepPEVsvuMVkeLzXZGy2ER+bKJ?=
 =?us-ascii?Q?LgaUJ4tYszliX0lNrKsmgZijKpAkHFvqsgjeAl2UHLSvKvo+LM+nXNjzncrS?=
 =?us-ascii?Q?7TsjNfHV7nvF5RV7XMARsB1qAO2KxxhSEx1yxiMuNDLe4MvhtV+YqD8OuW0O?=
 =?us-ascii?Q?IeDgBngDg8tKEmsoWwWC/GnbBlz5ikog+tpUgsn8m0OP46DIA0K8pIYm7OCo?=
 =?us-ascii?Q?8VFFDByHAgSx4PHLGkOHVCDVdvue8Qb7Yoyhli3HvDSaeEHvCRffJ0F8Tfze?=
 =?us-ascii?Q?rGuEYR9Wi9NMHJpTr0KJDWLpTbrXFeD6jBO7jVclcFJJRhYirtY6e3tzGiC4?=
 =?us-ascii?Q?1RJXThsIH4ImEg/D377pxcg7TuYbWPnSbNzWf5abzsDoeSM8RA8CtIYdDabc?=
 =?us-ascii?Q?KB/rpiWCmD4C84mwdajsLtE0kbglOBlLlQuMX4ynasTE3FQ/zBFtI4zIk8b8?=
 =?us-ascii?Q?Gyh6Ty1yhDsvENsjZ1cef4o=3D?=
X-OriginatorOrg: nutanix.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 591a0444-8f06-46ca-966a-08db2ae14e89
X-MS-Exchange-CrossTenant-AuthSource: BL3PR02MB8217.namprd02.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 22 Mar 2023 14:25:37.8267
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: bb047546-786f-4de1-bd75-24e5b6f79043
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 8Zs9+kV2NFqlTkZzAsesxpkNXi8EqlPkpCoZWth5BZd0VfH27yN/f4L6CCf+00xEPhcANceovHxoWZiIwMb0t+c8VRx1aYwRhFq8bpj4luI=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH0PR02MB8289
X-Proofpoint-ORIG-GUID: fJRvEXj91nTNkxZdfht8Ee9FVVSoYQCk
X-Proofpoint-GUID: fJRvEXj91nTNkxZdfht8Ee9FVVSoYQCk
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-03-22_11,2023-03-22_01,2023-02-09_01
X-Proofpoint-Spam-Reason: safe
X-Spam-Status: No, score=-0.2 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,SPF_HELO_NONE,SPF_NONE,
        URIBL_BLOCKED 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

cgroups v1 has a unique way of setting up memory pressure notifications:
the user opens "memory.pressure_level" of the cgroup they want to
monitor for pressure, then open "cgroup.event_control" and write the fd
(among other things) to that file. memory.pressure_level has no other
use, specifically it does not support any read or write operations.
Consequently, no handlers are provided, and the file ends up with
permissions 000. However, to actually use the mechanism, the subscribing
user must have read access to the file and open the fd for reading, see
memcg_write_event_control().

This is all fine as long as the subscribing process runs as root and is
otherwise unconfined by further restrictions. However, if you add strict
access controls such as selinux, the permission bits will be enforced,
and opening memory.pressure_level for reading will fail, preventing the
process from subscribing, even as root.

There are several ways around this issue, but adding a dummy read
handler seems like the least invasive to me. I'd be interested to hear:
(a) do you think there is a less invasive way? Alternatively, we could
    add a flag in cftype in include/linux/cgroup-defs.h, but that seems
    more invasive for what is a legacy interface.
(b) would you be interested to take this patch, or is it too niche a fix
    for a legacy subsystem?
---
 mm/memcontrol.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 5abffe6f8389..e48c749d9724 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -3734,6 +3734,16 @@ static u64 mem_cgroup_read_u64(struct cgroup_subsys_state *css,
 	}
 }
 
+/*
+ * This function doesn't do anything useful. Its only job is to provide a read
+ * handler so that the file gets read permissions when it's created.
+ */
+static int mem_cgroup_dummy_seq_show(__always_unused struct seq_file *m,
+				     __always_unused void *v)
+{
+	return -EINVAL;
+}
+
 #ifdef CONFIG_MEMCG_KMEM
 static int memcg_online_kmem(struct mem_cgroup *memcg)
 {
@@ -5064,6 +5074,7 @@ static struct cftype mem_cgroup_legacy_files[] = {
 	},
 	{
 		.name = "pressure_level",
+		.seq_show = mem_cgroup_dummy_seq_show,
 	},
 #ifdef CONFIG_NUMA
 	{
-- 
2.32.0