linux-kernel - [RFC PATCH] memcg: adjust memcg used to charge for new simple

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <f58bd7c8-0053-316c-db06-ced13ffcf867@openvz.org>
Date:   Thu, 18 Aug 2022 12:10:45 +0300
From:   Vasily Averin <vvs@...nvz.org>
To:     Roman Gushchin <roman.gushchin@...ux.dev>, mkoutny@...e.com,
        tj@...nel.org
Cc:     gregkh@...uxfoundation.org, hannes@...xchg.org, kernel@...nvz.org,
        linux-kernel@...r.kernel.org, mhocko@...e.com, shakeelb@...gle.com,
        songmuchun@...edance.com, viro@...iv.linux.org.uk,
        Christian Brauner <brauner@...nel.org>
Subject: [RFC PATCH] memcg: adjust memcg used to charge for new simple_xattrs
 objects

The patch was announced here:
https://lore.kernel.org/all/62188f37-f816-08e9-cdd5-8df23131746d@openvz.org/
"3) adjust active memcg for simple_xattr accounting
  sysfs and tmpfs are in-memory file system, 
  for extended attributes they uses simple_xattr infrastructure.
  The patch forces sys_set[f]xattr calls to account xattr object
  to predictable memcg: for kernfs memcg will be taken from kernfs node,
  for shmem -- from shmem_info.
  Like 1) case, this allows to understand which memcg should be used
  for object accounting and simplify debugging of possible troubles."

It was compiled but was not tested yet.
---
sys_set[f]xattr uses simple_xattr infrastructure to create a new
extended attribute for in-memory file systems like sysfs and tmpfs.
Number and size of allocated objects are controlled by user space,
they are always living in memory and its lifetime is indefinitely long.
Therefore this memory should be properly accounted.

By default new memory is accounted to memcg of creator process.
As a result, neighboring xattrs of the same inode can be charged to
different memcgs. This looks unexpected and makes hard the
investigation of the memcg accounting issues.

This patch adjust memcg used for such allocations. For kernfs
it gives memcg from kernfs node, for shmem -- from shmem_info.
This allows to cahrge all inode-sepcific objects to the same
memory cgroup.

Signed-off-by: Vasily Averin <vvs@...nvz.org>
---
 fs/kernfs/inode.c | 31 ++++++++++++++++++++++++-------
 mm/shmem.c        | 10 +++++++++-
 2 files changed, 33 insertions(+), 8 deletions(-)

diff --git a/fs/kernfs/inode.c b/fs/kernfs/inode.c
index 3d783d80f5da..975532b32e7c 100644
--- a/fs/kernfs/inode.c
+++ b/fs/kernfs/inode.c
@@ -14,6 +14,7 @@
 #include <linux/slab.h>
 #include <linux/xattr.h>
 #include <linux/security.h>
+#include <linux/memcontrol.h>
 
 #include "kernfs-internal.h"
 
@@ -335,8 +336,16 @@ static int kernfs_vfs_xattr_set(const struct xattr_handler *handler,
 {
 	const char *name = xattr_full_name(handler, suffix);
 	struct kernfs_node *kn = inode->i_private;
+	struct mem_cgroup *old, *memcg;
+	int ret;
+
+	memcg = mem_cgroup_or_root(get_mem_cgroup_from_obj(kn));
+	old = set_active_memcg(memcg);
 
-	return kernfs_xattr_set(kn, name, value, size, flags);
+	ret = kernfs_xattr_set(kn, name, value, size, flags);
+	set_active_memcg(old);
+	mem_cgroup_put(memcg);
+	return ret;
 }
 
 static int kernfs_vfs_user_xattr_add(struct kernfs_node *kn,
@@ -403,21 +412,29 @@ static int kernfs_vfs_user_xattr_set(const struct xattr_handler *handler,
 	const char *full_name = xattr_full_name(handler, suffix);
 	struct kernfs_node *kn = inode->i_private;
 	struct kernfs_iattrs *attrs;
+	struct mem_cgroup *old, *memcg;
+	int ret = -ENOMEM;
 
 	if (!(kernfs_root(kn)->flags & KERNFS_ROOT_SUPPORT_USER_XATTR))
 		return -EOPNOTSUPP;
 
+	memcg = mem_cgroup_or_root(get_mem_cgroup_from_obj(kn));
+	old = set_active_memcg(memcg);
+
 	attrs = kernfs_iattrs(kn);
 	if (!attrs)
-		return -ENOMEM;
+		goto out;
 
 	if (value)
-		return kernfs_vfs_user_xattr_add(kn, full_name, &attrs->xattrs,
-						 value, size, flags);
-	else
-		return kernfs_vfs_user_xattr_rm(kn, full_name, &attrs->xattrs,
+		ret = kernfs_vfs_user_xattr_add(kn, full_name, &attrs->xattrs,
 						value, size, flags);
-
+	else
+		ret = kernfs_vfs_user_xattr_rm(kn, full_name, &attrs->xattrs,
+					       value, size, flags);
+out:
+	set_active_memcg(old);
+	mem_cgroup_put(memcg);
+	return ret;
 }
 
 static const struct xattr_handler kernfs_trusted_xattr_handler = {
diff --git a/mm/shmem.c b/mm/shmem.c
index 5783f11351bb..291c15774340 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -3255,9 +3255,17 @@ static int shmem_xattr_handler_set(const struct xattr_handler *handler,
 				   size_t size, int flags)
 {
 	struct shmem_inode_info *info = SHMEM_I(inode);
+	struct mem_cgroup *old, *memcg;
+	int ret;
+
+	memcg = mem_cgroup_or_root(get_mem_cgroup_from_obj(info));
+	old = set_active_memcg(memcg);
 
 	name = xattr_full_name(handler, name);
-	return simple_xattr_set(&info->xattrs, name, value, size, flags, NULL);
+	ret = simple_xattr_set(&info->xattrs, name, value, size, flags, NULL);
+	set_active_memcg(old);
+	mem_cgroup_put(memcg);
+	return ret;
 }
 
 static const struct xattr_handler shmem_security_xattr_handler = {
-- 
2.34.1