From patchwork Sun Jan 10 18:05:08 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Johannes Berg X-Patchwork-Id: 1424283 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (no SPF record) smtp.mailfrom=lists.infradead.org (client-ip=2001:8b0:10b:1231::1; helo=merlin.infradead.org; envelope-from=linux-um-bounces+incoming=patchwork.ozlabs.org@lists.infradead.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=sipsolutions.net Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; secure) header.d=lists.infradead.org header.i=@lists.infradead.org header.a=rsa-sha256 header.s=merlin.20170209 header.b=lR39o2gD; dkim-atps=neutral Received: from merlin.infradead.org (merlin.infradead.org [IPv6:2001:8b0:10b:1231::1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4DDPs558xTz9sW8 for ; Mon, 11 Jan 2021 05:05:45 +1100 (AEDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:MIME-Version:Message-Id:Date:Subject:To:From: Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender :Resent-To:Resent-Cc:Resent-Message-ID:In-Reply-To:References:List-Owner; bh=qZStMe5x0xM7xp8VlgUy8MVh/Bra4F3x3v8JyHPtNnY=; b=lR39o2gDzy3Stq0mVxYyL0Yngf CtopFITjdB/iUtBB5ubDiejvmxRo8oFnZPgGSq/WLhean2cSnjzUU7w6bXTKiUO2MbQzaKbTyqoPi WDjGY4D1SC3laao+XrKoehk/FDWd+RDJUFJqO2nbT5hRUSrsCHsG9waLW0+rr0z9Kn8UElIl0Y3S9 bGhYAFUVfqI131LBnyGSMhc5JZ+T15pHPe9wvxcAJ2/eLcP2cII1QphrkMPwhNHULxAcXnkX01h5L cvkQcAxX4uguY3brcGPimKsE0Co9LOVD0U3tzI8wNJ4doN+/qwyaIM/jVaZcS4gZULgNKeLzwu157 MjXJWxuw==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kyf5n-0006sE-EP; Sun, 10 Jan 2021 18:05:31 +0000 Received: from s3.sipsolutions.net ([2a01:4f8:191:4433::2] helo=sipsolutions.net) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1kyf5i-0006pQ-TL for linux-um@lists.infradead.org; Sun, 10 Jan 2021 18:05:29 +0000 Received: by sipsolutions.net with esmtpsa (TLS1.3:ECDHE_X25519__RSA_PSS_RSAE_SHA256__AES_256_GCM:256) (Exim 4.94) (envelope-from ) id 1kyf4x-004VXb-8h; Sun, 10 Jan 2021 19:05:18 +0100 From: Johannes Berg To: linux-um@lists.infradead.org Subject: [PATCH 1/2] Revert "um: allocate a guard page to helper threads" Date: Sun, 10 Jan 2021 19:05:08 +0100 Message-Id: <20210110190509.65a0c174fcca.I148fc32a2b85fd17746ca528f3811c80483232bd@changeid> X-Mailer: git-send-email 2.26.2 MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210110_130527_057284_FA6A726D X-CRM114-Status: GOOD ( 15.52 ) X-Spam-Score: 0.4 (/) X-Spam-Report: SpamAssassin version 3.4.4 on merlin.infradead.org summary: Content analysis details: (0.4 points) pts rule name description ---- ---------------------- -------------------------------------------------- 0.0 SPF_NONE SPF: sender does not publish an SPF Record 0.0 SPF_HELO_NONE SPF: HELO does not publish an SPF Record 0.4 KHOP_HELO_FCRDNS Relay HELO differs from its IP's reverse DNS X-BeenThere: linux-um@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Benjamin Berg , Johannes Berg Sender: "linux-um" Errors-To: linux-um-bounces+incoming=patchwork.ozlabs.org@lists.infradead.org From: Johannes Berg This reverts commit ef4459a6da09 ("um: allocate a guard page to helper threads"), it's broken in multiple ways: 1) the free no longer matches the alloc; and 2) more importantly, the set_memory_ro() causes allocation of page tables for the normal memory that doesn't have any, and that later causes corruption and crashes (usually but not always in vfree()). We could fix the first bug and use vmalloc() to work around the second, but set_memory_ro() actually doesn't do anything either so I'll just revert that as well. Reported-by: Benjamin Berg Fixes: ef4459a6da09 ("um: allocate a guard page to helper threads") Signed-off-by: Johannes Berg --- arch/um/drivers/ubd_kern.c | 2 +- arch/um/include/shared/kern_util.h | 2 +- arch/um/kernel/process.c | 11 ++++------- arch/um/os-Linux/helper.c | 4 ++-- 4 files changed, 8 insertions(+), 11 deletions(-) diff --git a/arch/um/drivers/ubd_kern.c b/arch/um/drivers/ubd_kern.c index 13b1fe694b90..7e07b321e847 100644 --- a/arch/um/drivers/ubd_kern.c +++ b/arch/um/drivers/ubd_kern.c @@ -1241,7 +1241,7 @@ static int __init ubd_driver_init(void){ /* Letting ubd=sync be like using ubd#s= instead of ubd#= is * enough. So use anyway the io thread. */ } - stack = alloc_stack(0); + stack = alloc_stack(0, 0); io_pid = start_io_thread(stack + PAGE_SIZE - sizeof(void *), &thread_fd); if(io_pid < 0){ diff --git a/arch/um/include/shared/kern_util.h b/arch/um/include/shared/kern_util.h index d8c279e3312f..2888ec812f6e 100644 --- a/arch/um/include/shared/kern_util.h +++ b/arch/um/include/shared/kern_util.h @@ -19,7 +19,7 @@ extern int kmalloc_ok; #define UML_ROUND_UP(addr) \ ((((unsigned long) addr) + PAGE_SIZE - 1) & PAGE_MASK) -extern unsigned long alloc_stack(int atomic); +extern unsigned long alloc_stack(int order, int atomic); extern void free_stack(unsigned long stack, int order); struct pt_regs; diff --git a/arch/um/kernel/process.c b/arch/um/kernel/process.c index 2a986ece5478..81d508daf67c 100644 --- a/arch/um/kernel/process.c +++ b/arch/um/kernel/process.c @@ -32,7 +32,6 @@ #include #include #include -#include /* * This is a per-cpu array. A processor only modifies its entry and it only @@ -63,18 +62,16 @@ void free_stack(unsigned long stack, int order) free_pages(stack, order); } -unsigned long alloc_stack(int atomic) +unsigned long alloc_stack(int order, int atomic) { - unsigned long addr; + unsigned long page; gfp_t flags = GFP_KERNEL; if (atomic) flags = GFP_ATOMIC; - addr = __get_free_pages(flags, 1); + page = __get_free_pages(flags, order); - set_memory_ro(addr, 1); - - return addr + PAGE_SIZE; + return page; } static inline void set_current(struct task_struct *task) diff --git a/arch/um/os-Linux/helper.c b/arch/um/os-Linux/helper.c index feb48d796e00..9fa6e4187d4f 100644 --- a/arch/um/os-Linux/helper.c +++ b/arch/um/os-Linux/helper.c @@ -45,7 +45,7 @@ int run_helper(void (*pre_exec)(void *), void *pre_data, char **argv) unsigned long stack, sp; int pid, fds[2], ret, n; - stack = alloc_stack(__cant_sleep()); + stack = alloc_stack(0, __cant_sleep()); if (stack == 0) return -ENOMEM; @@ -116,7 +116,7 @@ int run_helper_thread(int (*proc)(void *), void *arg, unsigned int flags, unsigned long stack, sp; int pid, status, err; - stack = alloc_stack(__cant_sleep()); + stack = alloc_stack(0, __cant_sleep()); if (stack == 0) return -ENOMEM;