memcg: enable accounting of ipc resources

PATCH In review Luiz Capitulino 2 participants 5 messages Nov 4, 2022 – Nov 8, 2022 Stable branch

lore

Share & tools

lore

Luiz Capitulino

author

2022-11-04 18:42 UTC #0 lore raw

PATCH 1/1 memcg: enable accounting of ipc resources

From: Vasily Averin <[email protected]>

Commit 18319498fdd4cdf8c1c2c48cd432863b1f915d6f upstream.

[ This backport fixes CVE-2021-3759 for 5.10 and 5.4. Please, note that
it caused conflicts in all files being changed because upstream changed ipc object allocation to and from kvmalloc() & friends (eg. commits bc8136a543aa and fc37a3b8b4388e). However, I decided to keep this backport about the memcg accounting fix only. ]

When user creates IPC objects it forces kernel to allocate memory for these long-living objects.

It makes sense to account them to restrict the host's memory consumption from inside the memcg-limited container.

This patch enables accounting for IPC shared memory segments, messages semaphores and semaphore's undo lists.

Link https://lkml.kernel.org/r/[email protected] open here

Signed-off-by Vasily Averin <[email protected]>

Reviewed-by Shakeel Butt <[email protected]>

Cc Alexander Viro <[email protected]>

Cc Alexey Dobriyan <[email protected]>

Cc Andrei Vagin <[email protected]>

Cc Borislav Petkov <[email protected]>

Cc Christian Brauner <[email protected]>

Cc Dmitry Safonov <[email protected]>

Cc "Eric W. Biederman" <[email protected]>

Cc Greg Kroah-Hartman <[email protected]>

Cc "H. Peter Anvin" <[email protected]>

Cc Ingo Molnar <[email protected]>

Cc "J. Bruce Fields" <[email protected]>

Cc Jeff Layton <[email protected]>

Cc Jens Axboe <[email protected]>

Cc Jiri Slaby <[email protected]>

Cc Johannes Weiner <[email protected]>

Cc Kirill Tkhai <[email protected]>

Cc Michal Hocko <[email protected]>

Cc Oleg Nesterov <[email protected]>

Cc Roman Gushchin <[email protected]>

Cc Serge Hallyn <[email protected]>

Cc Tejun Heo <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc Vladimir Davydov <[email protected]>

Cc Yutian Yang <[email protected]>

Cc Zefan Li <[email protected]>

Signed-off-by Andrew Morton <[email protected]>

Signed-off-by Linus Torvalds <[email protected]>

Signed-off-by Luiz Capitulino <[email protected]>

---
 ipc/msg.c | 2 +-
 ipc/sem.c | 9 +++++----
 ipc/shm.c | 2 +-
 3 files changed, 7 insertions(+), 6 deletions(-)

Reviewers,

Some important details:

o While doing this backport I realized that Vasily worked on a large accounting overhaul which may include more instances of this problem (and possibly more unfixed CVEs). This brings the question whether we should only fix concrete/reproducible accounting issues or bring Vasily's entire overhaul. I'm choosing to fix only concrete cases

o 4.19 and 4.9 should also have this issue, but I haven't tried the backport there yet

o For testing, I did two things:

1. Reproduced the issue as described in the link below, with and without this patch. Without the patch I can pretty clearly see the kernel allocating several gigas of memory that are not accounted for by memcg. With the patch the memory is accounted correctly

Reproducer: https://lore.kernel.org/linux-mm/[email protected]/

2. I ran LTP's ipc test-cases (which simple, but hopefully good enough)

ipc/msg.c +1 -1

index 6e6c8e0c9380..8ded6b8f10a2 100644
@@ -147,7 +147,7 @@ static int newque(struct ipc_namespace *ns, struct ipc_params *params)
 	key_t key = params->key;
 	int msgflg = params->flg;
 
-	msq = kvmalloc(sizeof(*msq), GFP_KERNEL);
+	msq = kvmalloc(sizeof(*msq), GFP_KERNEL_ACCOUNT);
 	if (unlikely(!msq))
 		return -ENOMEM;

ipc/sem.c +5 -4

index 7d9c06b0ad6e..d3b9b73cd9ca 100644
@@ -511,7 +511,7 @@ static struct sem_array *sem_alloc(size_t nsems)
 	if (nsems > (INT_MAX - sizeof(*sma)) / sizeof(sma->sems[0]))
 		return NULL;
 
-	sma = kvzalloc(struct_size(sma, sems, nsems), GFP_KERNEL);
+	sma = kvzalloc(struct_size(sma, sems, nsems), GFP_KERNEL_ACCOUNT);
 	if (unlikely(!sma))
 		return NULL;

@@ -1852,7 +1852,7 @@ static inline int get_undo_list(struct sem_undo_list **undo_listp)
 
 	undo_list = current->sysvsem.undo_list;
 	if (!undo_list) {
-		undo_list = kzalloc(sizeof(*undo_list), GFP_KERNEL);
+		undo_list = kzalloc(sizeof(*undo_list), GFP_KERNEL_ACCOUNT);
 		if (undo_list == NULL)
 			return -ENOMEM;
 		spin_lock_init(&undo_list->lock);

@@ -1937,7 +1937,7 @@ static struct sem_undo *find_alloc_undo(struct ipc_namespace *ns, int semid)
 	rcu_read_unlock();
 
 	/* step 2: allocate new undo structure */
-	new = kzalloc(sizeof(struct sem_undo) + sizeof(short)*nsems, GFP_KERNEL);
+	new = kzalloc(sizeof(struct sem_undo) + sizeof(short)*nsems, GFP_KERNEL_ACCOUNT);
 	if (!new) {
 		ipc_rcu_putref(&sma->sem_perm, sem_rcu_free);
 		return ERR_PTR(-ENOMEM);

@@ -2001,7 +2001,8 @@ static long do_semtimedop(int semid, struct sembuf __user *tsops,
 	if (nsops > ns->sc_semopm)
 		return -E2BIG;
 	if (nsops > SEMOPM_FAST) {
-		sops = kvmalloc_array(nsops, sizeof(*sops), GFP_KERNEL);
+		sops = kvmalloc_array(nsops, sizeof(*sops),
+				      GFP_KERNEL_ACCOUNT);
 		if (sops == NULL)
 			return -ENOMEM;
 	}

ipc/shm.c +1 -1

index 471ac3e7498d..b418731d66e8 100644
@@ -711,7 +711,7 @@ static int newseg(struct ipc_namespace *ns, struct ipc_params *params)
 			ns->shm_tot + numpages > ns->shm_ctlall)
 		return -ENOSPC;
 
-	shp = kvmalloc(sizeof(*shp), GFP_KERNEL);
+	shp = kvmalloc(sizeof(*shp), GFP_KERNEL_ACCOUNT);
 	if (unlikely(!shp))
 		return -ENOMEM;

Signature · 1 line

2.24.4.AMZN

Greg Kroah-Hartman

maintainer

2022-11-07 09:14 UTC #1 lore raw

Quoted 9 lines · Luiz Capitulino

From: Vasily Averin <[email protected]>
Commit 18319498fdd4cdf8c1c2c48cd432863b1f915d6f upstream.
[ This backport fixes CVE-2021-3759 for 5.10 and 5.4. Please, note that
it caused conflicts in all files being changed because upstream changed ipc object allocation to and from kvmalloc() & friends (eg. commits bc8136a543aa and fc37a3b8b4388e). However, I decided to keep this backport about the memcg accounting fix only. ]

Looks good, now queued up, thanks.

greg k-h

Greg Kroah-Hartman

maintainer

2022-11-08 12:46 UTC #2 lore raw

Quoted 5 lines · Greg Kroah-Hartman

Quoted 4 lines · Luiz Capitulino
From: Vasily Averin <[email protected]>
Commit 18319498fdd4cdf8c1c2c48cd432863b1f915d6f upstream.

Quoted 7 lines

Quoted 5 lines
[ This backport fixes CVE-2021-3759 for 5.10 and 5.4. Please, note that
it caused conflicts in all files being changed because upstream changed ipc object allocation to and from kvmalloc() & friends (eg. commits bc8136a543aa and fc37a3b8b4388e). However, I decided to keep this backport about the memcg accounting fix only. ]
Looks good, now queued up, thanks.

Ah, but you missed a fix-up patch for this one {sigh}

I've now queued up 18319498fdd4 ("memcg: enable accounting of ipc resources") as well. Please be more careful in the future when backporting changes that you also include the fixes for those changes.

thanks,

greg k-h

Luiz Capitulino

author

2022-11-08 16:52 UTC #3 lore raw

Quoted 14 lines · Greg Kroah-Hartman

Quoted 11 lines · Greg Kroah-Hartman
Quoted 9 lines · Luiz Capitulino
From: Vasily Averin <[email protected]>
Commit 18319498fdd4cdf8c1c2c48cd432863b1f915d6f upstream.
[ This backport fixes CVE-2021-3759 for 5.10 and 5.4. Please, note that
it caused conflicts in all files being changed because upstream changed ipc object allocation to and from kvmalloc() & friends (eg. commits bc8136a543aa and fc37a3b8b4388e). However, I decided to keep this backport about the memcg accounting fix only. ]

Quoted 7 lines

Quoted 1 line
Looks good, now queued up, thanks.
Ah, but you missed a fix-up patch for this one {sigh}
I've now queued up 18319498fdd4 ("memcg: enable accounting of ipc resources") as well. Please be more careful in the future when backporting changes that you also include the fixes for those changes.

Good catch, the fixup is actually 6a4746ba06191e23d30230738e94334b26590a8a

Will try to be more careful next time.

- Luiz

Quoted 4 lines

thanks,
greg k-h

Greg Kroah-Hartman

maintainer

2022-11-08 17:10 UTC #4 lore raw

Quoted 18 lines · Luiz Capitulino

Quoted 16 lines · Greg Kroah-Hartman
Quoted 12 lines · Greg Kroah-Hartman
Quoted 9 lines · Luiz Capitulino
From: Vasily Averin <[email protected]>
Commit 18319498fdd4cdf8c1c2c48cd432863b1f915d6f upstream.
[ This backport fixes CVE-2021-3759 for 5.10 and 5.4. Please, note that
it caused conflicts in all files being changed because upstream changed ipc object allocation to and from kvmalloc() & friends (eg. commits bc8136a543aa and fc37a3b8b4388e). However, I decided to keep this backport about the memcg accounting fix only. ]
Looks good, now queued up, thanks.

Quoted 7 lines

Quoted 5 lines
Ah, but you missed a fix-up patch for this one {sigh}
I've now queued up 18319498fdd4 ("memcg: enable accounting of ipc resources") as well. Please be more careful in the future when backporting changes that you also include the fixes for those changes.
Good catch, the fixup is actually 6a4746ba06191e23d30230738e94334b26590a8a

Oops, yes, wrong git id in my clipboard buffer.