From: Greg Kroah-Hartman Date: Wed, 1 Dec 2021 09:10:52 +0000 (+0100) Subject: 4.14-stable patches X-Git-Tag: v4.4.294~72 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=eceed991aa5bb7723b66ace77b983366ba5e15ed;p=thirdparty%2Fkernel%2Fstable-queue.git 4.14-stable patches added patches: ipc-warn-if-trying-to-remove-ipc-object-which-is-absent.patch --- diff --git a/queue-4.14/ipc-warn-if-trying-to-remove-ipc-object-which-is-absent.patch b/queue-4.14/ipc-warn-if-trying-to-remove-ipc-object-which-is-absent.patch new file mode 100644 index 00000000000..4e959b7b541 --- /dev/null +++ b/queue-4.14/ipc-warn-if-trying-to-remove-ipc-object-which-is-absent.patch @@ -0,0 +1,116 @@ +From 126e8bee943e9926238c891e2df5b5573aee76bc Mon Sep 17 00:00:00 2001 +From: Alexander Mikhalitsyn +Date: Fri, 19 Nov 2021 16:43:18 -0800 +Subject: ipc: WARN if trying to remove ipc object which is absent + +From: Alexander Mikhalitsyn + +commit 126e8bee943e9926238c891e2df5b5573aee76bc upstream. + +Patch series "shm: shm_rmid_forced feature fixes". + +Some time ago I met kernel crash after CRIU restore procedure, +fortunately, it was CRIU restore, so, I had dump files and could do +restore many times and crash reproduced easily. After some +investigation I've constructed the minimal reproducer. It was found +that it's use-after-free and it happens only if sysctl +kernel.shm_rmid_forced = 1. + +The key of the problem is that the exit_shm() function not handles shp's +object destroy when task->sysvshm.shm_clist contains items from +different IPC namespaces. In most cases this list will contain only +items from one IPC namespace. + +How can this list contain object from different namespaces? The +exit_shm() function is designed to clean up this list always when +process leaves IPC namespace. But we made a mistake a long time ago and +did not add a exit_shm() call into the setns() syscall procedures. + +The first idea was just to add this call to setns() syscall but it +obviously changes semantics of setns() syscall and that's +userspace-visible change. So, I gave up on this idea. + +The first real attempt to address the issue was just to omit forced +destroy if we meet shp object not from current task IPC namespace [1]. +But that was not the best idea because task->sysvshm.shm_clist was +protected by rwsem which belongs to current task IPC namespace. It +means that list corruption may occur. + +Second approach is just extend exit_shm() to properly handle shp's from +different IPC namespaces [2]. This is really non-trivial thing, I've +put a lot of effort into that but not believed that it's possible to +make it fully safe, clean and clear. + +Thanks to the efforts of Manfred Spraul working an elegant solution was +designed. Thanks a lot, Manfred! + +Eric also suggested the way to address the issue in ("[RFC][PATCH] shm: +In shm_exit destroy all created and never attached segments") Eric's +idea was to maintain a list of shm_clists one per IPC namespace, use +lock-less lists. But there is some extra memory consumption-related +concerns. + +An alternative solution which was suggested by me was implemented in +("shm: reset shm_clist on setns but omit forced shm destroy"). The idea +is pretty simple, we add exit_shm() syscall to setns() but DO NOT +destroy shm segments even if sysctl kernel.shm_rmid_forced = 1, we just +clean up the task->sysvshm.shm_clist list. + +This chages semantics of setns() syscall a little bit but in comparision +to the "naive" solution when we just add exit_shm() without any special +exclusions this looks like a safer option. + +[1] https://lkml.org/lkml/2021/7/6/1108 +[2] https://lkml.org/lkml/2021/7/14/736 + +This patch (of 2): + +Let's produce a warning if we trying to remove non-existing IPC object +from IPC namespace kht/idr structures. + +This allows us to catch possible bugs when the ipc_rmid() function was +called with inconsistent struct ipc_ids*, struct kern_ipc_perm* +arguments. + +Link: https://lkml.kernel.org/r/20211027224348.611025-1-alexander.mikhalitsyn@virtuozzo.com +Link: https://lkml.kernel.org/r/20211027224348.611025-2-alexander.mikhalitsyn@virtuozzo.com +Co-developed-by: Manfred Spraul +Signed-off-by: Manfred Spraul +Signed-off-by: Alexander Mikhalitsyn +Cc: "Eric W. Biederman" +Cc: Davidlohr Bueso +Cc: Greg KH +Cc: Andrei Vagin +Cc: Pavel Tikhomirov +Cc: Vasily Averin +Cc: +Signed-off-by: Andrew Morton +Signed-off-by: Linus Torvalds +Signed-off-by: Greg Kroah-Hartman + +--- + ipc/util.c | 6 +++--- + 1 file changed, 3 insertions(+), 3 deletions(-) + +--- a/ipc/util.c ++++ b/ipc/util.c +@@ -409,8 +409,8 @@ static int ipcget_public(struct ipc_name + static void ipc_kht_remove(struct ipc_ids *ids, struct kern_ipc_perm *ipcp) + { + if (ipcp->key != IPC_PRIVATE) +- rhashtable_remove_fast(&ids->key_ht, &ipcp->khtnode, +- ipc_kht_params); ++ WARN_ON_ONCE(rhashtable_remove_fast(&ids->key_ht, &ipcp->khtnode, ++ ipc_kht_params)); + } + + /** +@@ -425,7 +425,7 @@ void ipc_rmid(struct ipc_ids *ids, struc + { + int lid = ipcid_to_idx(ipcp->id); + +- idr_remove(&ids->ipcs_idr, lid); ++ WARN_ON_ONCE(idr_remove(&ids->ipcs_idr, lid) != ipcp); + ipc_kht_remove(ids, ipcp); + ids->in_use--; + ipcp->deleted = true; diff --git a/queue-4.14/series b/queue-4.14/series index 9e008409d3f..0d77b0695db 100644 --- a/queue-4.14/series +++ b/queue-4.14/series @@ -67,3 +67,4 @@ xen-netfront-disentangle-tx_skb_freelist.patch xen-netfront-don-t-trust-the-backend-response-data-blindly.patch tty-hvc-replace-bug_on-with-negative-return-value.patch shm-extend-forced-shm-destroy-to-support-objects-from-several-ipc-nses.patch +ipc-warn-if-trying-to-remove-ipc-object-which-is-absent.patch