Virtually mapped stacks with guard pages (x86, core)
Subsystems (21)
Andy Lutomirski
maintainerv4 cover Virtually mapped stacks with guard pages (x86, core)
Since the dawn of time, a kernel stack overflow has been a real PITA to debug, has caused nondeterministic crashes some time after the actual overflow, and has generally been easy to exploit for root.
With this series, arches can enable HAVE_ARCH_VMAP_STACK. Arches that enable it (just x86 for now) get virtually mapped stacks with guard pages. This causes reliable faults when the stack overflows.
If the arch implements it well, we get a nice OOPS on stack overflow (as opposed to panicing directly or otherwise exploding badly). On x86, the OOPS is nice, has a usable call trace, and the overflowing task is killed cleanly.
On my laptop, this adds about 1.5µs of overhead to task creation, which seems to be mainly caused by vmalloc inefficiently allocating individual pages even when a higher-order page is available on the freelist.
This does not address interrupt stacks. It also does not address the possibility of privilege escalation by a controlled stack overflow that overwrites thread_info without hitting the guard page. I'll send patches to address the latter issue once this series lands.
It's worth noting that s390 has an arch-specific gcc feature that detects stack overflows by adjusting function prologues. Arches with features like that may wish to avoid using vmapped stacks to minimize the performance hit.
Ingo, would it make sense to throw it into a seaparate branch in -tip? I wouldn't mind seeing some -next testing to give people a chance to shake out problems. I'm particularly interested in whether there are any drivers that expect virt_to_phys to work on stack addresses. (I know that virtio-net used to, but I fixed that a while back.)
Once this lands in -tip, I'm planning on attacking thread_info. Once thread_info is under control, we can start caching a couple of stacks per cpu, and that should get us most of the performance back.
Changes from v3:
- Fix rxrpc and bluetooth, which used scatterlists pointed at the stack
- Add some acks and cc's
Changes from v2:
- Delete kernel_unmap_pages_in_pgd rather than hardening it (Borislav)
- Fix sub-page stack accounting better (Josh)
Changes from v1:
- Fix rewind_stack_and_do_exit (Josh)
- Fix deadlock under load
- Clean up generic stack vmalloc code
- Many other minor fixes
Andy Lutomirski (14):
bluetooth: Switch SMP to crypto_cipher_encrypt_one() x86/cpa: In populate_pgd, don't set the pgd entry until it's populated x86/mm: Remove kernel_unmap_pages_in_pgd() and
efi_cleanup_page_tables()
mm: Track NR_KERNEL_STACK in KiB instead of number of stacks
mm: Fix memcg stack accounting for sub-page stacks
dma-api: Teach the "DMA-from-stack" check about vmapped stacks
fork: Add generic vmalloced stack support
x86/die: Don't try to recover from an OOPS on a non-default stack x86/dumpstack: When OOPSing, rewind the stack before do_exit x86/dumpstack: When dumping stack bytes due to OOPS, start with
regs->sp
x86/dumpstack: Try harder to get a call trace on stack overflow x86/dumpstack/64: Handle faults when printing the "Stack:" part of an
OOPS
x86/mm/64: Enable vmapped stacks
x86/mm: Improve stack-overflow #PF handling
Herbert Xu (1):
rxrpc: Avoid using stack memory in SG lists in rxkad
Ingo Molnar (1):
x86/mm/hotplug: Don't remove PGD entries in remove_pagetable()
arch/Kconfig | 29 ++++++++++ arch/ia64/include/asm/thread_info.h | 2 +- arch/x86/Kconfig | 1 + arch/x86/entry/entry_32.S | 11 ++++ arch/x86/entry/entry_64.S | 11 ++++ arch/x86/include/asm/efi.h | 1 - arch/x86/include/asm/pgtable_types.h | 2 - arch/x86/include/asm/switch_to.h | 28 +++++++++- arch/x86/include/asm/traps.h | 6 ++ arch/x86/kernel/dumpstack.c | 19 ++++++- arch/x86/kernel/dumpstack_32.c | 4 +- arch/x86/kernel/dumpstack_64.c | 16 +++++- arch/x86/kernel/traps.c | 32 +++++++++++ arch/x86/mm/fault.c | 39 +++++++++++++ arch/x86/mm/init_64.c | 27 --------- arch/x86/mm/pageattr.c | 32 +---------- arch/x86/mm/tlb.c | 15 +++++ arch/x86/platform/efi/efi.c | 2 - arch/x86/platform/efi/efi_32.c | 3 - arch/x86/platform/efi/efi_64.c | 5 -- drivers/base/node.c | 3 +- fs/proc/meminfo.c | 2 +- include/linux/memcontrol.h | 2 +- include/linux/mmzone.h | 2 +- include/linux/sched.h | 15 +++++ kernel/fork.c | 86 ++++++++++++++++++++++------- lib/dma-debug.c | 39 +++++++++++-- mm/memcontrol.c | 2 +- mm/page_alloc.c | 3 +- net/bluetooth/smp.c | 67 ++++++++++------------- net/rxrpc/ar-internal.h | 1 + net/rxrpc/rxkad.c | 103 +++++++++++++++-------------------- 32 files changed, 400 insertions(+), 210 deletions(-)
Signature · 1 line
2.5.5
Andy Lutomirski
maintainerPATCH v4 1/16 bluetooth: Switch SMP to crypto_cipher_encrypt_one()
SMP does ECB crypto on stack buffers. This is complicated and fragile, and it will not work if the stack is virtually allocated.
Switch to the crypto_cipher interface, which is simpler and safer.
--- net/bluetooth/smp.c | 67 ++++++++++++++++++++++------------------------------- 1 file changed, 28 insertions(+), 39 deletions(-)
net/bluetooth/smp.c +28 -39
@@ -22,9 +22,9 @@#include <linux/debugfs.h> #include <linux/scatterlist.h> +#include <linux/crypto.h> #include <crypto/b128ops.h> #include <crypto/hash.h> -#include <crypto/skcipher.h> #include <net/bluetooth/bluetooth.h> #include <net/bluetooth/hci_core.h>
@@ -88,7 +88,7 @@ struct smp_dev {u8 min_key_size;u8 max_key_size; -struct crypto_skcipher *tfm_aes; +struct crypto_cipher *tfm_aes;struct crypto_shash *tfm_cmac; };
@@ -127,7 +127,7 @@ struct smp_chan {u8 dhkey[32 ];u8 mackey[16 ]; -struct crypto_skcipher *tfm_aes; +struct crypto_cipher *tfm_aes;struct crypto_shash *tfm_cmac; };
@@ -361,10 +361,8 @@ static int smp_h6(struct crypto_shash *tfm_cmac, const u8 w[16],* s1 and ah. */ -static int smp_e (struct crypto_skcipher *tfm,const u8 *k,u8 *r) +static int smp_e (struct crypto_cipher *tfm,const u8 *k,u8 *r) { -SKCIPHER_REQUEST_ON_STACK (req, tfm); -struct scatterlist sg;uint8_t tmp[16 ], data[16 ];int err;
@@ -378,7 +376,7 @@ static int smp_e(struct crypto_skcipher *tfm, const u8 *k, u8 *r)/* The most significant octet of key corresponds to k[0] */ swap_buf (k, tmp,16 ); - err =crypto_skcipher_setkey (tfm, tmp,16 ); + err =crypto_cipher_setkey (tfm, tmp,16 );if (err) {BT_ERR ("cipher setkey failed: %d" , err);return err;
@@ -387,16 +385,7 @@ static int smp_e(struct crypto_skcipher *tfm, const u8 *k, u8 *r)/* Most significant octet of plaintextData corresponds to data[0] */ swap_buf (r, data,16 ); -sg_init_one (&sg, data,16 ); - -skcipher_request_set_tfm (req, tfm); -skcipher_request_set_callback (req,0 ,NULL ,NULL ); -skcipher_request_set_crypt (req, &sg, &sg,16 ,NULL ); - - err =crypto_skcipher_encrypt (req); -skcipher_request_zero (req); -if (err) -BT_ERR ("Encrypt data error %d" , err); +crypto_cipher_encrypt_one (tfm, data, data);/* Most significant octet of encryptedData corresponds to data[0] */ swap_buf (data, r,16 );
@@ -406,7 +395,7 @@ static int smp_e(struct crypto_skcipher *tfm, const u8 *k, u8 *r)return err; } -static int smp_c1 (struct crypto_skcipher *tfm_aes,const u8 k[16 ], +static int smp_c1 (struct crypto_cipher *tfm_aes,const u8 k[16 ],const u8 r[16 ],const u8 preq[7 ],const u8 pres[7 ],u8 _iat,const bdaddr_t *ia,u8 _rat,const bdaddr_t *ra,u8 res[16 ]) {
@@ -455,7 +444,7 @@ static int smp_c1(struct crypto_skcipher *tfm_aes, const u8 k[16],return err; } -static int smp_s1 (struct crypto_skcipher *tfm_aes,const u8 k[16 ], +static int smp_s1 (struct crypto_cipher *tfm_aes,const u8 k[16 ],const u8 r1[16 ],const u8 r2[16 ],u8 _r[16 ]) {int err;
@@ -471,7 +460,7 @@ static int smp_s1(struct crypto_skcipher *tfm_aes, const u8 k[16],return err; } -static int smp_ah (struct crypto_skcipher *tfm,const u8 irk[16 ], +static int smp_ah (struct crypto_cipher *tfm,const u8 irk[16 ],const u8 r[3 ],u8 res[3 ]) {u8 _res[16 ];
@@ -759,7 +748,7 @@ static void smp_chan_destroy(struct l2cap_conn *conn)kzfree (smp->slave_csrk);kzfree (smp->link_key); -crypto_free_skcipher (smp->tfm_aes); +crypto_free_cipher (smp->tfm_aes);crypto_free_shash (smp->tfm_cmac);/* Ensure that we don't leave any debug key around if debug key
@@ -1359,9 +1348,9 @@ static struct smp_chan *smp_chan_create(struct l2cap_conn *conn)if (!smp)return NULL ; - smp->tfm_aes =crypto_alloc_skcipher ("ecb(aes)" ,0 ,CRYPTO_ALG_ASYNC ); + smp->tfm_aes =crypto_alloc_cipher ("aes" ,0 ,CRYPTO_ALG_ASYNC );if (IS_ERR (smp->tfm_aes)) { -BT_ERR ("Unable to create ECB crypto context" ); +BT_ERR ("Unable to create AES crypto context" );kzfree (smp);return NULL ; }
@@ -1369,7 +1358,7 @@ static struct smp_chan *smp_chan_create(struct l2cap_conn *conn) smp->tfm_cmac =crypto_alloc_shash ("cmac(aes)" ,0 ,0 );if (IS_ERR (smp->tfm_cmac)) {BT_ERR ("Unable to create CMAC crypto context" ); -crypto_free_skcipher (smp->tfm_aes); +crypto_free_cipher (smp->tfm_aes);kzfree (smp);return NULL ; }
@@ -3120,7 +3109,7 @@ static struct l2cap_chan *smp_add_cid(struct hci_dev *hdev, u16 cid) {struct l2cap_chan *chan;struct smp_dev *smp; -struct crypto_skcipher *tfm_aes; +struct crypto_cipher *tfm_aes;struct crypto_shash *tfm_cmac;if (cid ==L2CAP_CID_SMP_BREDR ) {
@@ -3132,9 +3121,9 @@ static struct l2cap_chan *smp_add_cid(struct hci_dev *hdev, u16 cid)if (!smp)return ERR_PTR (-ENOMEM ); - tfm_aes =crypto_alloc_skcipher ("ecb(aes)" ,0 ,CRYPTO_ALG_ASYNC ); + tfm_aes =crypto_alloc_cipher ("aes" ,0 ,CRYPTO_ALG_ASYNC );if (IS_ERR (tfm_aes)) { -BT_ERR ("Unable to create ECB crypto context" ); +BT_ERR ("Unable to create AES crypto context" );kzfree (smp);return ERR_CAST (tfm_aes); }
@@ -3142,7 +3131,7 @@ static struct l2cap_chan *smp_add_cid(struct hci_dev *hdev, u16 cid) tfm_cmac =crypto_alloc_shash ("cmac(aes)" ,0 ,0 );if (IS_ERR (tfm_cmac)) {BT_ERR ("Unable to create CMAC crypto context" ); -crypto_free_skcipher (tfm_aes); +crypto_free_cipher (tfm_aes);kzfree (smp);return ERR_CAST (tfm_cmac); }
@@ -3156,7 +3145,7 @@ create_chan: chan =l2cap_chan_create ();if (!chan) {if (smp) { -crypto_free_skcipher (smp->tfm_aes); +crypto_free_cipher (smp->tfm_aes);crypto_free_shash (smp->tfm_cmac);kzfree (smp); }
@@ -3203,7 +3192,7 @@ static void smp_del_chan(struct l2cap_chan *chan) smp = chan->data;if (smp) { chan->data =NULL ; -crypto_free_skcipher (smp->tfm_aes); +crypto_free_cipher (smp->tfm_aes);crypto_free_shash (smp->tfm_cmac);kzfree (smp); }
@@ -3440,7 +3429,7 @@ void smp_unregister(struct hci_dev *hdev) #if IS_ENABLED(CONFIG_BT_SELFTEST_SMP) -static int __init test_ah(struct crypto_skcipher *tfm_aes) +static int __init test_ah(struct crypto_cipher *tfm_aes) { const u8 irk[16] = { 0x9b, 0x7d, 0x39, 0x0a, 0xa6, 0x10, 0x10, 0x34,
@@ -3460,7 +3449,7 @@ static int __init test_ah(struct crypto_skcipher *tfm_aes)return 0 ; } -static int __init test_c1 (struct crypto_skcipher *tfm_aes) +static int __init test_c1 (struct crypto_cipher *tfm_aes) {const u8 k[16 ] = {0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,
@@ -3490,7 +3479,7 @@ static int __init test_c1(struct crypto_skcipher *tfm_aes)return 0 ; } -static int __init test_s1 (struct crypto_skcipher *tfm_aes) +static int __init test_s1 (struct crypto_cipher *tfm_aes) {const u8 k[16 ] = {0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,
@@ -3686,7 +3675,7 @@ static const struct file_operations test_smp_fops = { .llseek = default_llseek, }; -static int __init run_selftests (struct crypto_skcipher *tfm_aes, +static int __init run_selftests (struct crypto_cipher *tfm_aes,struct crypto_shash *tfm_cmac) {ktime_t calltime, delta, rettime;
@@ -3764,27 +3753,27 @@ done:int __init bt_selftest_smp (void ) { -struct crypto_skcipher *tfm_aes; +struct crypto_cipher *tfm_aes;struct crypto_shash *tfm_cmac;int err; - tfm_aes =crypto_alloc_skcipher ("ecb(aes)" ,0 ,CRYPTO_ALG_ASYNC ); + tfm_aes =crypto_alloc_cipher ("aes" ,0 ,CRYPTO_ALG_ASYNC );if (IS_ERR (tfm_aes)) { -BT_ERR ("Unable to create ECB crypto context" ); +BT_ERR ("Unable to create AES crypto context" );return PTR_ERR (tfm_aes); } tfm_cmac =crypto_alloc_shash ("cmac(aes)" ,0 ,CRYPTO_ALG_ASYNC );if (IS_ERR (tfm_cmac)) {BT_ERR ("Unable to create CMAC crypto context" ); -crypto_free_skcipher (tfm_aes); +crypto_free_cipher (tfm_aes);return PTR_ERR (tfm_cmac); } err =run_selftests (tfm_aes, tfm_cmac);crypto_free_shash (tfm_cmac); -crypto_free_skcipher (tfm_aes); +crypto_free_cipher (tfm_aes);return err; }
Signature · 1 line
2.5.5
Re: [PATCH v4 01/16] bluetooth: Switch SMP to crypto_cipher_encrypt_one()
Quoted 5 lines · Andy Lutomirski
SMP does ECB crypto on stack buffers. This is complicated and fragile, and it will not work if the stack is virtually allocated.
Switch to the
crypto_cipherinterface, which is simpler and safer.
Quoted 8 lines
Cc Marcel Holtmann <[email protected]>Cc Gustavo Padovan <[email protected]>Cc Johan Hedberg <[email protected]>Cc "David S. Miller" <[email protected]>Cc Herbert Xu <[email protected]>Signed-off-by Andy Lutomirski <[email protected]>
Thanks,
Signature · 3 lines
Email: Herbert Xu <[email protected]> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH v4 01/16] bluetooth: Switch SMP to crypto_cipher_encrypt_one()
In reply to Andy LutomirskiQuoted 13 lines · Andy Lutomirski
SMP does ECB crypto on stack buffers. This is complicated and fragile, and it will not work if the stack is virtually allocated.
Switch to the
crypto_cipherinterface, which is simpler and safer.Cc Marcel Holtmann <[email protected]>Cc Gustavo Padovan <[email protected]>Cc Johan Hedberg <[email protected]>Cc "David S. Miller" <[email protected]>Cc Herbert Xu <[email protected]>Signed-off-by Andy Lutomirski <[email protected]>
Quoted 3 lines
--- net/bluetooth/smp.c | 67 ++++++++++++++++++++++------------------------------- 1 file changed, 28 insertions(+), 39 deletions(-)
Johan
Andy Lutomirski
maintainerPATCH v4 2/16 rxrpc: Avoid using stack memory in SG lists in rxkad
In reply to Andy LutomirskiFrom: Herbert Xu <[email protected]>
rxkad uses stack memory in SG lists which would not work if stacks were allocated from vmalloc memory. In fact, in most cases this isn't even necessary as the stack memory ends up getting copied over to kmalloc memory.
This patch eliminates all the unnecessary stack memory uses by supplying the final destination directly to the crypto API. In two instances where a temporary buffer is actually needed we also switch use the skb->cb area instead of the stack.
Finally there is no need to split a split-page buffer into two SG entries so code dealing with that has been removed.
Message-Id: <[email protected]>
--- net/rxrpc/ar-internal.h | 1 + net/rxrpc/rxkad.c | 103 ++++++++++++++++++++---------------------------- 2 files changed, 44 insertions(+), 60 deletions(-)
net/rxrpc/ar-internal.h +1 -0
@@ -277,6 +277,7 @@ struct rxrpc_connection {struct key *key;/* security for this connection (client) */ struct key *server_key;/* security for this service */ struct crypto_skcipher *cipher;/* encryption handle */ +struct rxrpc_crypt csum_iv_head;/* leading block for csum_iv */ struct rxrpc_crypt csum_iv;/* packet checksum base */ unsigned long events;#define RXRPC_CONN_CHALLENGE 0/* send challenge packet */
net/rxrpc/rxkad.c +43 -60
231 lines not on this page · raw message
Signature · 1 line
2.5.5
Andy Lutomirski
maintainerPATCH v4 3/16 x86/mm/hotplug: Don't remove PGD entries in remove_pagetable()
In reply to Andy LutomirskiFrom: Ingo Molnar <[email protected]>
So when memory hotplug removes a piece of physical memory from pagetable mappings, it also frees the underlying PGD entry.
This complicates PGD management, so don't do this. We can keep the PGD mapped and the PUD table all clear - it's only a single 4K page per 512 GB of memory hotplugged.
Message-Id: <[email protected]>
--- arch/x86/mm/init_64.c | 27 --------------------------- 1 file changed, 27 deletions(-)
arch/x86/mm/init_64.c +0 -27
@@ -702,27 +702,6 @@ static void __meminit free_pmd_table(pmd_t *pmd_start, pud_t *pud)spin_unlock (&init_mm.page_table_lock); } -/* Return true if pgd is changed, otherwise return false. */ -static bool __meminit free_pud_table (pud_t *pud_start,pgd_t *pgd) -{ -pud_t *pud; -int i; - -for (i =0 ; i <PTRS_PER_PUD ; i++) { - pud = pud_start + i; -if (pud_val (*pud)) -return false; - } - -/* free a pud table */ -free_pagetable (pgd_page (*pgd),0 ); -spin_lock (&init_mm.page_table_lock); -pgd_clear (pgd); -spin_unlock (&init_mm.page_table_lock); - -return true; -} -static void __meminit remove_pte_table (pte_t *pte_start,unsigned long addr,unsigned long end,bool direct)
@@ -913,7 +892,6 @@ remove_pagetable(unsigned long start, unsigned long end, bool direct)unsigned long addr;pgd_t *pgd;pud_t *pud; -bool pgd_changed = false;for (addr = start; addr < end; addr = next) { next =pgd_addr_end (addr, end);
@@ -924,13 +902,8 @@ remove_pagetable(unsigned long start, unsigned long end, bool direct) pud = (pud_t *)pgd_page_vaddr (*pgd);remove_pud_table (pud, addr, next, direct); -if (free_pud_table (pud, pgd)) - pgd_changed = true; } -if (pgd_changed) -sync_global_pgds (start, end -1 ,1 ); -flush_tlb_all (); }
Signature · 1 line
2.5.5
Andy Lutomirski
maintainerPATCH v4 4/16 x86/cpa: In populate_pgd, don't set the pgd entry until it's populated
In reply to Andy LutomirskiThis avoids pointless races in which another CPU or task might see a partially populated global pgd entry. These races should normally be harmless, but, if another CPU propagates the entry via vmalloc_fault and then populate_pgd fails (due to memory allocation failure, for example), this prevents a use-after-free of the pgd entry.
--- arch/x86/mm/pageattr.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
arch/x86/mm/pageattr.c +3 -1
@@ -1113,7 +1113,9 @@ static int populate_pgd(struct cpa_data *cpa, unsigned long addr) ret =populate_pud (cpa, addr, pgd_entry, pgprot);if (ret <0 ) { -unmap_pgd_range (cpa->pgd, addr, +if (pud) +free_page ((unsigned long )pud); +unmap_pud_range (pgd_entry, addr, addr + (cpa->numpages <<PAGE_SHIFT ));return ret; }
Signature · 1 line
2.5.5
Andy Lutomirski
maintainerPATCH v4 5/16 x86/mm: Remove kernel_unmap_pages_in_pgd() and efi_cleanup_page_tables()
In reply to Andy Lutomirskikernel_unmap_pages_in_pgd() is dangerous: if a pgd entry in init_mm.pgd were to be cleared, callers would need to ensure that the pgd entry hadn't been propagated to any other pgd.
Its only caller was efi_cleanup_page_tables(), and that, in turn, was unused, so just delete both functions. This leaves a couple of other helpers unused, so delete them, too.
--- arch/x86/include/asm/efi.h | 1 - arch/x86/include/asm/pgtable_types.h | 2 -- arch/x86/mm/pageattr.c | 28 ---------------------------- arch/x86/platform/efi/efi.c | 2 -- arch/x86/platform/efi/efi_32.c | 3 --- arch/x86/platform/efi/efi_64.c | 5 ----- 6 files changed, 41 deletions(-)
arch/x86/include/asm/efi.h +0 -1
@@ -125,7 +125,6 @@ extern void __init efi_map_region_fixed(efi_memory_desc_t *md);extern void efi_sync_low_kernel_mappings (void );extern int __init efi_alloc_page_tables (void );extern int __init efi_setup_page_tables (unsigned long pa_memmap,unsigned num_pages); -extern void __init efi_cleanup_page_tables (unsigned long pa_memmap,unsigned num_pages);extern void __init old_map_region (efi_memory_desc_t *md);extern void __init runtime_code_page_mkexec (void );extern void __init efi_runtime_update_mappings (void );
arch/x86/include/asm/pgtable_types.h +0 -2
@@ -475,8 +475,6 @@ extern pmd_t *lookup_pmd_address(unsigned long address);extern phys_addr_t slow_virt_to_phys (void *__address);extern int kernel_map_pages_in_pgd (pgd_t *pgd,u64 pfn,unsigned long address,unsigned numpages,unsigned long page_flags); -void kernel_unmap_pages_in_pgd (pgd_t *root,unsigned long address, -unsigned numpages);#endif /* !__ASSEMBLY__ */ #endif /* _ASM_X86_PGTABLE_DEFS_H */
arch/x86/mm/pageattr.c +0 -28
@@ -746,18 +746,6 @@ static bool try_to_free_pmd_page(pmd_t *pmd)return true; } -static bool try_to_free_pud_page (pud_t *pud) -{ -int i; - -for (i =0 ; i <PTRS_PER_PUD ; i++) -if (!pud_none (pud[i])) -return false; - -free_page ((unsigned long )pud); -return true; -} -static bool unmap_pte_range (pmd_t *pmd,unsigned long start,unsigned long end) {pte_t *pte =pte_offset_kernel (pmd, start);
@@ -871,16 +859,6 @@ static void unmap_pud_range(pgd_t *pgd, unsigned long start, unsigned long end)*/ } -static void unmap_pgd_range (pgd_t *root,unsigned long addr,unsigned long end) -{ -pgd_t *pgd_entry = root +pgd_index (addr); - -unmap_pud_range (pgd_entry, addr, end); - -if (try_to_free_pud_page ((pud_t *)pgd_page_vaddr (*pgd_entry))) -pgd_clear (pgd_entry); -} -static int alloc_pte_page (pmd_t *pmd) {pte_t *pte = (pte_t *)get_zeroed_page (GFP_KERNEL | __GFP_NOTRACK);
@@ -1993,12 +1971,6 @@ out:return retval; } -void kernel_unmap_pages_in_pgd (pgd_t *root,unsigned long address, -unsigned numpages) -{ -unmap_pgd_range (root, address, address + (numpages <<PAGE_SHIFT )); -} -/* * The testcases use internal knowledge of the implementation that shouldn't * be exposed to the rest of the kernel. Include these directly here.
arch/x86/platform/efi/efi.c +0 -2
@@ -978,8 +978,6 @@ static void __init __efi_enter_virtual_mode(void)* EFI mixed mode we need all of memory to be accessible when * we pass parameters to the EFI runtime services in the * thunking code. -* -* efi_cleanup_page_tables(__pa(new_memmap), 1 << pg_shift); */ free_pages ((unsigned long )new_memmap, pg_shift);
arch/x86/platform/efi/efi_32.c +0 -3
@@ -49,9 +49,6 @@ int __init efi_setup_page_tables(unsigned long pa_memmap, unsigned num_pages) {return 0 ; } -void __init efi_cleanup_page_tables (unsigned long pa_memmap,unsigned num_pages) -{ -}void __init efi_map_region (efi_memory_desc_t *md) {
arch/x86/platform/efi/efi_64.c +0 -5
@@ -285,11 +285,6 @@ int __init efi_setup_page_tables(unsigned long pa_memmap, unsigned num_pages)return 0 ; } -void __init efi_cleanup_page_tables (unsigned long pa_memmap,unsigned num_pages) -{ -kernel_unmap_pages_in_pgd (efi_pgd, pa_memmap, num_pages); -} -static void __init __map_region (efi_memory_desc_t *md,u64 va) {unsigned long flags = _PAGE_RW;
Signature · 1 line
2.5.5
Andy Lutomirski
maintainerPATCH v4 6/16 mm: Track NR_KERNEL_STACK in KiB instead of number of stacks
In reply to Andy LutomirskiCurrently, NR_KERNEL_STACK tracks the number of kernel stacks in a zone. This only makes sense if each kernel stack exists entirely in one zone, and allowing vmapped stacks could break this assumption.
Since frv has THREAD_SIZE < PAGE_SIZE, we need to track kernel stack allocations in a unit that divides both THREAD_SIZE and PAGE_SIZE on all architectures. Keep it simple and use KiB.
--- drivers/base/node.c | 3 +-- fs/proc/meminfo.c | 2 +- include/linux/mmzone.h | 2 +- kernel/fork.c | 3 ++- mm/page_alloc.c | 3 +-- 5 files changed, 6 insertions(+), 7 deletions(-)
drivers/base/node.c +1 -2
@@ -121,8 +121,7 @@ static ssize_t node_read_meminfo(struct device *dev, nid,K (node_page_state (nid,NR_FILE_MAPPED )), nid,K (node_page_state (nid,NR_ANON_PAGES )), nid,K (i.sharedram), - nid,node_page_state (nid,NR_KERNEL_STACK ) * -THREAD_SIZE /1024 , + nid,node_page_state (nid,NR_KERNEL_STACK_KB ), nid,K (node_page_state (nid,NR_PAGETABLE )), nid,K (node_page_state (nid,NR_UNSTABLE_NFS )), nid, K(node_page_state(nid, NR_BOUNCE)),
fs/proc/meminfo.c +1 -1
@@ -145,7 +145,7 @@ static int meminfo_proc_show(struct seq_file *m, void *v) global_page_state(NR_SLAB_UNRECLAIMABLE)),K (global_page_state (NR_SLAB_RECLAIMABLE )),K (global_page_state (NR_SLAB_UNRECLAIMABLE )), -global_page_state (NR_KERNEL_STACK ) *THREAD_SIZE /1024 , +global_page_state (NR_KERNEL_STACK_KB ),K (global_page_state (NR_PAGETABLE )), #ifdef CONFIG_QUICKLIST K(quicklist_total_size()),
include/linux/mmzone.h +1 -1
@@ -127,7 +127,7 @@ enum zone_stat_item {NR_SLAB_RECLAIMABLE ,NR_SLAB_UNRECLAIMABLE ,NR_PAGETABLE ,/* used for pagetables */ -NR_KERNEL_STACK , +NR_KERNEL_STACK_KB ,/* measured in KiB */ /* Second 128 byte cacheline */ NR_UNSTABLE_NFS ,/* NFS unstable pages */ NR_BOUNCE,
kernel/fork.c +2 -1
@@ -225,7 +225,8 @@ static void account_kernel_stack(struct thread_info *ti, int account) {struct zone *zone =page_zone (virt_to_page (ti)); -mod_zone_page_state (zone,NR_KERNEL_STACK , account); +mod_zone_page_state (zone,NR_KERNEL_STACK_KB , +THREAD_SIZE /1024 * account); }void free_task (struct task_struct *tsk)
mm/page_alloc.c +1 -2
@@ -4457,8 +4457,7 @@ void show_free_areas(unsigned int filter)K (zone_page_state (zone,NR_SHMEM )),K (zone_page_state (zone,NR_SLAB_RECLAIMABLE )),K (zone_page_state (zone,NR_SLAB_UNRECLAIMABLE )), -zone_page_state (zone,NR_KERNEL_STACK ) * -THREAD_SIZE /1024 , +zone_page_state (zone,NR_KERNEL_STACK_KB ),K (zone_page_state (zone,NR_PAGETABLE )),K (zone_page_state (zone,NR_UNSTABLE_NFS )), K(zone_page_state(zone, NR_BOUNCE)),
Signature · 1 line
2.5.5
Re: [PATCH v4 06/16] mm: Track NR_KERNEL_STACK in KiB instead of number of stacks
Quoted 8 lines · Andy Lutomirski
Currently,
NR_KERNEL_STACKtracks the number of kernel stacks in a zone. This only makes sense if each kernel stack exists entirely in one zone, and allowing vmapped stacks could break this assumption.Since frv has
THREAD_SIZE<PAGE_SIZE, we need to track kernel stack allocations in a unit that divides bothTHREAD_SIZEandPAGE_SIZEon all architectures. Keep it simple and use KiB.
Quoted 7 lines
Cc Vladimir Davydov <[email protected]>Cc Johannes Weiner <[email protected]>Cc Michal Hocko <[email protected]>Reviewed-by Vladimir Davydov <[email protected]>Acked-by Michal Hocko <[email protected]>Signed-off-by Andy Lutomirski <[email protected]>
Signature · 1 line
Josh
Andy Lutomirski
maintainerPATCH v4 7/16 mm: Fix memcg stack accounting for sub-page stacks
In reply to Andy LutomirskiWe should account for stacks regardless of stack size, and we need to account in sub-page units if THREAD_SIZE < PAGE_SIZE. Change the units to kilobytes and Move it into account_kernel_stack().
--- include/linux/memcontrol.h | 2 +- kernel/fork.c | 15 ++++++--------- mm/memcontrol.c | 2 +- 3 files changed, 8 insertions(+), 11 deletions(-)
include/linux/memcontrol.h +1 -1
@@ -52,7 +52,7 @@ enum mem_cgroup_stat_index {MEM_CGROUP_STAT_SWAP ,/* # of pages, swapped out */ MEM_CGROUP_STAT_NSTATS ,/* default hierarchy stats */ -MEMCG_KERNEL_STACK =MEM_CGROUP_STAT_NSTATS , +MEMCG_KERNEL_STACK_KB =MEM_CGROUP_STAT_NSTATS ,MEMCG_SLAB_RECLAIMABLE ,MEMCG_SLAB_UNRECLAIMABLE , MEMCG_SOCK,
kernel/fork.c +6 -9
@@ -165,20 +165,12 @@ static struct thread_info *alloc_thread_info_node(struct task_struct *tsk,struct page *page =alloc_kmem_pages_node (node,THREADINFO_GFP ,THREAD_SIZE_ORDER ); -if (page) -memcg_kmem_update_page_stat (page,MEMCG_KERNEL_STACK , -1 <<THREAD_SIZE_ORDER ); - return page ? page_address(page) : NULL; }static inline void free_thread_info (struct thread_info *ti) { -struct page *page =virt_to_page (ti); - -memcg_kmem_update_page_stat (page,MEMCG_KERNEL_STACK , - -(1 <<THREAD_SIZE_ORDER )); -__free_kmem_pages (page,THREAD_SIZE_ORDER ); +free_kmem_pages ((unsigned long )ti,THREAD_SIZE_ORDER ); }# else static struct kmem_cache *thread_info_cache;
@@ -227,6 +219,11 @@ static void account_kernel_stack(struct thread_info *ti, int account)mod_zone_page_state (zone,NR_KERNEL_STACK_KB ,THREAD_SIZE /1024 * account); + +/* All stack pages belong to the same memcg. */ +memcg_kmem_update_page_stat ( +virt_to_page (ti),MEMCG_KERNEL_STACK_KB , + account * (THREAD_SIZE /1024 )); }void free_task (struct task_struct *tsk)
mm/memcontrol.c +1 -1
@@ -5133,7 +5133,7 @@ static int memory_stat_show(struct seq_file *m, void *v)seq_printf (m,"file %llu\n" , (u64 )stat[MEM_CGROUP_STAT_CACHE ] *PAGE_SIZE );seq_printf (m,"kernel_stack %llu\n" , - (u64 )stat[MEMCG_KERNEL_STACK ] *PAGE_SIZE ); + (u64 )stat[MEMCG_KERNEL_STACK_KB ] *1024 );seq_printf (m,"slab %llu\n" , (u64)(stat[MEMCG_SLAB_RECLAIMABLE ] + stat[MEMCG_SLAB_UNRECLAIMABLE ]) *PAGE_SIZE );
Signature · 1 line
2.5.5
Re: [PATCH v4 07/16] mm: Fix memcg stack accounting for sub-page stacks
Quoted 4 lines · Andy Lutomirski
We should account for stacks regardless of stack size, and we need to account in sub-page units if
THREAD_SIZE<PAGE_SIZE. Change the units to kilobytes and Move it intoaccount_kernel_stack().
Quoted 8 lines
Fixes 12580e4b54ba8 ("mm: memcontrol: report kernel stack usage in cgroup2 memory.stat")Cc Vladimir Davydov <[email protected]>Cc Johannes Weiner <[email protected]>Cc Michal Hocko <[email protected]>Reviewed-by Vladimir Davydov <[email protected]>Acked-by Michal Hocko <[email protected]>Signed-off-by Andy Lutomirski <[email protected]>
Signature · 1 line
Josh
Andy Lutomirski
maintainerPATCH v4 8/16 dma-api: Teach the "DMA-from-stack" check about vmapped stacks
In reply to Andy LutomirskiIf we're using CONFIG_VMAP_STACK and we manage to point an sg entry at the stack, then either the sg page will be in highmem or sg_virt will return the direct-map alias. In neither case will the existing check_for_stack() implementation realize that it's a stack page.
Fix it by explicitly checking for stack pages.
This has no effect by itself. It's broken out for ease of review.
--- lib/dma-debug.c | 39 +++++++++++++++++++++++++++++++++------ 1 file changed, 33 insertions(+), 6 deletions(-)
lib/dma-debug.c +33 -6
@@ -22,6 +22,7 @@#include <linux/stacktrace.h> #include <linux/dma-debug.h> #include <linux/spinlock.h> +#include <linux/vmalloc.h> #include <linux/debugfs.h> #include <linux/uaccess.h> #include <linux/export.h>
@@ -1162,11 +1163,35 @@ static void check_unmap(struct dma_debug_entry *ref)put_hash_bucket (bucket, &flags); } -static void check_for_stack (struct device *dev,void *addr) +static void check_for_stack (struct device *dev, +struct page *page,size_t offset) { -if (object_is_on_stack (addr)) -err_printk (dev,NULL ,"DMA-API: device driver maps memory from " -"stack [addr=%p]\n" , addr); +void *addr; +struct vm_struct *stack_vm_area =task_stack_vm_area (current); + +if (!stack_vm_area) { +/* Stack is direct-mapped. */ +if (PageHighMem (page)) +return ; + addr =page_address (page) + offset; +if (object_is_on_stack (addr)) +err_printk (dev,NULL ,"DMA-API: device driver maps memory from stack [addr=%p]\n" , + addr); + }else { +/* Stack is vmalloced. */ +int i; + +for (i =0 ; i < stack_vm_area->nr_pages; i++) { +if (page != stack_vm_area->pages[i]) +continue ; + + addr = (u8 *)current->stack + i *PAGE_SIZE + + offset; +err_printk (dev,NULL ,"DMA-API: device driver maps memory from stack [probable addr=%p]\n" , + addr); +break ; + } + } }static inline bool overlap (void *addr,unsigned long len,void *start,void *end)
@@ -1289,10 +1314,11 @@ void debug_dma_map_page(struct device *dev, struct page *page, size_t offset,if (map_single) entry->type = dma_debug_single; +check_for_stack (dev, page, offset); +if (!PageHighMem (page)) {void *addr =page_address (page) + offset; -check_for_stack (dev, addr);check_for_illegal_area (dev, addr, size); }
@@ -1384,8 +1410,9 @@ void debug_dma_map_sg(struct device *dev, struct scatterlist *sg, entry->sg_call_ents = nents; entry->sg_mapped_ents = mapped_ents; +check_for_stack (dev,sg_page (s), s->offset); +if (!PageHighMem (sg_page (s))) { -check_for_stack (dev,sg_virt (s));check_for_illegal_area (dev,sg_virt (s),sg_dma_len (s)); }
Signature · 1 line
2.5.5
Andy Lutomirski
maintainerPATCH v4 9/16 fork: Add generic vmalloced stack support
In reply to Andy LutomirskiIf CONFIG_VMAP_STACK is selected, kernel stacks are allocated with vmalloc_node.
--- arch/Kconfig | 29 +++++++++++++ arch/ia64/include/asm/thread_info.h | 2 +- include/linux/sched.h | 15 +++++++ kernel/fork.c | 82 +++++++++++++++++++++++++++++-------- 4 files changed, 110 insertions(+), 18 deletions(-)
arch/Kconfig +29 -0
@@ -661,4 +661,33 @@ config ARCH_NO_COHERENT_DMA_MMAP config CPU_NO_EFFICIENT_FFS def_bool n +config HAVE_ARCH_VMAP_STACK + def_bool n + help + An arch should select this symbol if it can support kernel stacks + in vmalloc space. This means: + + - vmalloc space must be large enough to hold many kernel stacks. + This may rule out many 32-bit architectures. + + - Stacks in vmalloc space need to work reliably. For example, if + vmap page tables are created on demand, either this mechanism + needs to work while the stack points to a virtual address with + unpopulated page tables or arch code (switch_to and switch_mm, + most likely) needs to ensure that the stack's page table entries + are populated before running on a possibly unpopulated stack. + + - If the stack overflows into a guard page, something reasonable + should happen. The definition of "reasonable" is flexible, but + instantly rebooting without logging anything would be unfriendly. + +config VMAP_STACK + bool "Use a virtually-mapped stack" + depends on HAVE_ARCH_VMAP_STACK + ---help--- + Enable this if you want the use virtually-mapped kernel stacks + with guard pages. This causes kernel stack overflows to be + caught immediately rather than causing difficult-to-diagnose + corruption. + source "kernel/gcov/Kconfig"
arch/ia64/include/asm/thread_info.h +1 -1
@@ -56,7 +56,7 @@ struct thread_info {#define alloc_thread_info_node (tsk, node) ((struct thread_info *) 0)#define task_thread_info (tsk) ((struct thread_info *) 0)#endif -#define free_thread_info (ti)/* nothing */ +#define free_thread_info (tsk)/* nothing */ #define task_stack_page (tsk) ((void *)(tsk))#define __HAVE_THREAD_FUNCTIONS
include/linux/sched.h +15 -0
@@ -1918,6 +1918,9 @@ struct task_struct {#ifdef CONFIG_MMU struct task_struct *oom_reaper_list;#endif +#ifdef CONFIG_VMAP_STACK +struct vm_struct *stack_vm_area; +#endif /* CPU-specific state of this task */ struct thread_struct thread;/*
@@ -1934,6 +1937,18 @@ extern int arch_task_struct_size __read_mostly;# define arch_task_struct_size (sizeof(struct task_struct))#endif +#ifdef CONFIG_VMAP_STACK +static inline struct vm_struct *task_stack_vm_area (const struct task_struct *t) +{ +return t->stack_vm_area; +} +#else +static inline struct vm_struct *task_stack_vm_area (const struct task_struct *t) +{ +return NULL ; +} +#endif +/* Future-safe accessor for struct task_struct's cpus_allowed. */ #define tsk_cpus_allowed (tsk) (&(tsk)->cpus_allowed)
kernel/fork.c +65 -17
148 lines not on this page · raw message
Signature · 1 line
2.5.5
Andy Lutomirski
maintainerPATCH v4 10/16 x86/die: Don't try to recover from an OOPS on a non-default stack
In reply to Andy LutomirskiIt's not going to work, because the scheduler will explode if we try to schedule when running on an IST stack or similar.
This will matter when we let kernel stack overflows (which are #DF) call die().
--- arch/x86/kernel/dumpstack.c | 3 +++ 1 file changed, 3 insertions(+)
arch/x86/kernel/dumpstack.c +3 -0
@@ -245,6 +245,9 @@ void oops_end(unsigned long flags, struct pt_regs *regs, int signr)return ;if (in_interrupt ())panic ("Fatal exception in interrupt" ); +if (((current_stack_pointer () ^ (current_top_of_stack () -1 )) + & ~(THREAD_SIZE -1 )) !=0 ) +panic ("Fatal exception on special stack" );if (panic_on_oops)panic ("Fatal exception" );do_exit (signr);
Signature · 1 line
2.5.5
Andy Lutomirski
maintainerPATCH v4 11/16 x86/dumpstack: When OOPSing, rewind the stack before do_exit
In reply to Andy LutomirskiIf we call do_exit with a clean stack, we greatly reduce the risk of recursive oopses due to stack overflow in do_exit, and we allow do_exit to work even if we OOPS from an IST stack. The latter gives us a much better chance of surviving long enough after we detect a stack overflow to write out our logs.
I intentionally separated this from the preceding patch that disables do_exit-on-OOPS on IST stacks. This way, if we need to revert this patch, we still end up in an acceptable state wrt stack overflow handling.
--- arch/x86/entry/entry_32.S | 11 +++++++++++ arch/x86/entry/entry_64.S | 11 +++++++++++ arch/x86/kernel/dumpstack.c | 13 +++++++++---- 3 files changed, 31 insertions(+), 4 deletions(-)
arch/x86/entry/entry_32.S +11 -0
@@ -1153,3 +1153,14 @@ ENTRY(async_page_fault) jmp error_code END(async_page_fault) #endif + +ENTRY(rewind_stack_do_exit) + /* Prevent any naive code from trying to unwind to our caller. */ + xorl %ebp, %ebp + + movl PER_CPU_VAR(cpu_current_top_of_stack), %esi + leal -TOP_OF_KERNEL_STACK_PADDING-PTREGS_SIZE(%esi), %esp + + call do_exit +1: jmp 1b +END(rewind_stack_do_exit)
arch/x86/entry/entry_64.S +11 -0
@@ -1423,3 +1423,14 @@ ENTRY(ignore_sysret) mov $-ENOSYS, %eax sysret END(ignore_sysret) + +ENTRY(rewind_stack_do_exit) + /* Prevent any naive code from trying to unwind to our caller. */ + xorl %ebp, %ebp + + movq PER_CPU_VAR(cpu_current_top_of_stack), %rax + leaq -TOP_OF_KERNEL_STACK_PADDING-PTREGS_SIZE(%rax), %rsp + + call do_exit +1: jmp 1b +END(rewind_stack_do_exit)
arch/x86/kernel/dumpstack.c +9 -4
@@ -226,6 +226,8 @@ unsigned long oops_begin(void)EXPORT_SYMBOL_GPL (oops_begin);NOKPROBE_SYMBOL (oops_begin); +extern void __noreturn rewind_stack_do_exit (int signr); +void oops_end (unsigned long flags,struct pt_regs *regs,int signr) {if (regs &&kexec_should_crash (current))
@@ -245,12 +247,15 @@ void oops_end(unsigned long flags, struct pt_regs *regs, int signr)return ;if (in_interrupt ())panic ("Fatal exception in interrupt" ); -if (((current_stack_pointer () ^ (current_top_of_stack () -1 )) - & ~(THREAD_SIZE -1 )) !=0 ) -panic ("Fatal exception on special stack" );if (panic_on_oops)panic ("Fatal exception" ); -do_exit (signr); + +/* +* We're not going to return, but we might be on an IST stack or +* have very little stack space left. Rewind the stack and kill +* the task. +*/ +rewind_stack_do_exit (signr); }NOKPROBE_SYMBOL (oops_end);
Signature · 1 line
2.5.5
Re: [PATCH v4 11/16] x86/dumpstack: When OOPSing, rewind the stack before do_exit
Quoted 42 lines · Andy Lutomirski
If we call
do_exitwith a clean stack, we greatly reduce the risk of recursive oopses due to stack overflow indo_exit, and we allowdo_exitto work even if we OOPS from an IST stack. The latter gives us a much better chance of surviving long enough after we detect a stack overflow to write out our logs.I intentionally separated this from the preceding patch that disables do_exit-on-OOPS on IST stacks. This way, if we need to revert this patch, we still end up in an acceptable state wrt stack overflow handling.
Signed-off-by Andy Lutomirski <[email protected]>--- arch/x86/entry/entry_32.S | 11 +++++++++++ arch/x86/entry/entry_64.S | 11 +++++++++++ arch/x86/kernel/dumpstack.c | 13 +++++++++---- 3 files changed, 31 insertions(+), 4 deletions(-)arch/x86/entry/entry_32.S +11
-0@@ -1153,3 +1153,14 @@ ENTRY(async_page_fault) jmp error_code END(async_page_fault) #endif + +ENTRY(rewind_stack_do_exit) + /* Prevent any naive code from trying to unwind to our caller. */ + xorl %ebp, %ebp + + movl PER_CPU_VAR(cpu_current_top_of_stack), %esi + leal -TOP_OF_KERNEL_STACK_PADDING-PTREGS_SIZE(%esi), %esp + + call do_exit +1: jmp 1b +END(rewind_stack_do_exit)arch/x86/entry/entry_64.S +0
-0
arch/x86/entry/entry_64.S ENTRY(ignore_sysret) · line 1429
Quoted 7 lines
arch/x86/entry/entry_64.S +4
-0@@ -1423,3 +1423,14 @@ ENTRY(ignore_sysret) mov $-ENOSYS, %eax sysret END(ignore_sysret) + +ENTRY(rewind_stack_do_exit) + /* Prevent any naive code from trying to unwind to our caller. */ + xorl %ebp, %ebp
s/ebp/rbp/g/ ?
Signature · 1 line
Josh
Re: [PATCH v4 11/16] x86/dumpstack: When OOPSing, rewind the stack before do_exit
Quoted 44 lines · Josh Poimboeuf
Quoted 43 lines · Andy Lutomirski
If we call
do_exitwith a clean stack, we greatly reduce the risk of recursive oopses due to stack overflow indo_exit, and we allowdo_exitto work even if we OOPS from an IST stack. The latter gives us a much better chance of surviving long enough after we detect a stack overflow to write out our logs.I intentionally separated this from the preceding patch that disables do_exit-on-OOPS on IST stacks. This way, if we need to revert this patch, we still end up in an acceptable state wrt stack overflow handling.
Signed-off-by Andy Lutomirski <[email protected]>--- arch/x86/entry/entry_32.S | 11 +++++++++++ arch/x86/entry/entry_64.S | 11 +++++++++++ arch/x86/kernel/dumpstack.c | 13 +++++++++---- 3 files changed, 31 insertions(+), 4 deletions(-)arch/x86/entry/entry_32.S +11
-0@@ -1153,3 +1153,14 @@ ENTRY(async_page_fault) jmp error_code END(async_page_fault) #endif + +ENTRY(rewind_stack_do_exit) + /* Prevent any naive code from trying to unwind to our caller. */ + xorl %ebp, %ebp + + movl PER_CPU_VAR(cpu_current_top_of_stack), %esi + leal -TOP_OF_KERNEL_STACK_PADDING-PTREGS_SIZE(%esi), %esp + + call do_exit +1: jmp 1b +END(rewind_stack_do_exit)arch/x86/entry/entry_64.S +0
-0@@ -1423,3 +1423,14 @@ ENTRY(ignore_sysret) mov $-ENOSYS, %eax
Quoted 8 lines
arch/x86/entry/entry_64.S ENTRY(ignore_sysret) · line 1429
Quoted 6 lines
arch/x86/entry/entry_64.S +4
-0sysret END(ignore_sysret) + +ENTRY(rewind_stack_do_exit) + /* Prevent any naive code from trying to unwind to our caller. */ + xorl %ebp, %ebps/ebp/rbp/g/ ?
No, this quirk of the x86-64 instruction set will zero-extend to 64-bits without needing a REX prefix.
--
Brian Gerst
Josh Poimboeuf
Reviewed-byRe: [PATCH v4 11/16] x86/dumpstack: When OOPSing, rewind the stack before do_exit
Quoted 54 lines · Brian Gerst
Quoted 44 lines · Josh Poimboeuf
Quoted 43 lines · Andy Lutomirski
If we call
do_exitwith a clean stack, we greatly reduce the risk of recursive oopses due to stack overflow indo_exit, and we allowdo_exitto work even if we OOPS from an IST stack. The latter gives us a much better chance of surviving long enough after we detect a stack overflow to write out our logs.I intentionally separated this from the preceding patch that disables do_exit-on-OOPS on IST stacks. This way, if we need to revert this patch, we still end up in an acceptable state wrt stack overflow handling.
Signed-off-by Andy Lutomirski <[email protected]>--- arch/x86/entry/entry_32.S | 11 +++++++++++ arch/x86/entry/entry_64.S | 11 +++++++++++ arch/x86/kernel/dumpstack.c | 13 +++++++++---- 3 files changed, 31 insertions(+), 4 deletions(-)arch/x86/entry/entry_32.S +11
-0@@ -1153,3 +1153,14 @@ ENTRY(async_page_fault) jmp error_code END(async_page_fault) #endif + +ENTRY(rewind_stack_do_exit) + /* Prevent any naive code from trying to unwind to our caller. */ + xorl %ebp, %ebp + + movl PER_CPU_VAR(cpu_current_top_of_stack), %esi + leal -TOP_OF_KERNEL_STACK_PADDING-PTREGS_SIZE(%esi), %esp + + call do_exit +1: jmp 1b +END(rewind_stack_do_exit)arch/x86/entry/entry_64.S +0
-0@@ -1423,3 +1423,14 @@ ENTRY(ignore_sysret) mov $-ENOSYS, %eaxQuoted 8 lines
arch/x86/entry/entry_64.S ENTRY(ignore_sysret) · line 1429
Quoted 6 lines
arch/x86/entry/entry_64.S +4
-0sysret END(ignore_sysret) + +ENTRY(rewind_stack_do_exit) + /* Prevent any naive code from trying to unwind to our caller. */ + xorl %ebp, %ebps/ebp/rbp/g/ ?
Quoted 2 lines
No, this quirk of the x86-64 instruction set will zero-extend to 64-bits without needing a REX prefix.
Ah, so it makes the instruction smaller. And I see that gcc also does the same. In that case:
Signature · 1 line
Josh
Andy Lutomirski
maintainerPATCH v4 12/16 x86/dumpstack: When dumping stack bytes due to OOPS, start with regs->sp
In reply to Andy LutomirskiThe comment suggests that show_stack(NULL, NULL) should backtrace the current context, but the code doesn't match the comment. If regs are given, start the "Stack:" hexdump at regs->sp.
--- arch/x86/kernel/dumpstack_32.c | 4 +++- arch/x86/kernel/dumpstack_64.c | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-)
arch/x86/kernel/dumpstack_32.c +3 -1
@@ -96,7 +96,9 @@ show_stack_log_lvl(struct task_struct *task, struct pt_regs *regs,int i;if (sp ==NULL ) { -if (task) +if (regs) + sp = (unsigned long *)regs->sp; +else if (task) sp = (unsigned long *)task->thread.sp;else sp = (unsigned long *)&sp;
arch/x86/kernel/dumpstack_64.c +3 -1
@@ -264,7 +264,9 @@ show_stack_log_lvl(struct task_struct *task, struct pt_regs *regs,* back trace for this cpu: */ if (sp ==NULL ) { -if (task) +if (regs) + sp = (unsigned long *)regs->sp; +else if (task) sp = (unsigned long *)task->thread.sp;else sp = (unsigned long *)&sp;
Signature · 1 line
2.5.5
Re: [PATCH v4 12/16] x86/dumpstack: When dumping stack bytes due to OOPS, start with regs->sp
Quoted 5 lines · Andy Lutomirski
The comment suggests that
show_stack(NULL, NULL)should backtrace the current context, but the code doesn't match the comment. If regs are given, start the "Stack:" hexdump atregs->sp.Signed-off-by Andy Lutomirski <[email protected]>
Signature · 1 line
Josh
Andy Lutomirski
maintainerPATCH v4 13/16 x86/dumpstack: Try harder to get a call trace on stack overflow
In reply to Andy LutomirskiIf we overflow the stack, print_context_stack will abort. Detect this case and rewind back into the valid part of the stack so that we can trace it.
--- arch/x86/kernel/dumpstack.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)
arch/x86/kernel/dumpstack.c +8 -1
@@ -87,7 +87,7 @@ static inline int valid_stack_ptr(struct task_struct *task,else return 0 ; } -return p > t && p < t +THREAD_SIZE - size; +return p >= t && p < t +THREAD_SIZE - size; }unsigned long
@@ -98,6 +98,13 @@ print_context_stack(struct task_struct *task, {struct stack_frame *frame = (struct stack_frame *)bp; +/* +* If we overflowed the stack into a guard page, jump back to the +* bottom of the usable stack. +*/ +if ((unsigned long )task->stack - (unsigned long )stack <PAGE_SIZE ) + stack = (unsigned long *)task->stack; +while (valid_stack_ptr (task, stack,sizeof (*stack), end)) {unsigned long addr;
Signature · 1 line
2.5.5
Re: [PATCH v4 13/16] x86/dumpstack: Try harder to get a call trace on stack overflow
Quoted 5 lines · Andy Lutomirski
If we overflow the stack,
print_context_stackwill abort. Detect this case and rewind back into the valid part of the stack so that we can trace it.Signed-off-by Andy Lutomirski <[email protected]>
Signature · 1 line
Josh
Re: [PATCH v4 13/16] x86/dumpstack: Try harder to get a call trace on stack overflow
Quoted 9 lines · Josh Poimboeuf
Quoted 5 lines · Andy Lutomirski
If we overflow the stack,
print_context_stackwill abort. Detect this case and rewind back into the valid part of the stack so that we can trace it.Signed-off-by Andy Lutomirski <[email protected]>Reviewed-by Josh Poimboeuf <[email protected]>
FWIW, I'm making a trivial change here for v4: task->stack -> task_stack_page(task). Since it seems inconsequential, I'm keeping your reviewed-by.
Quoted 2 lines
--
Josh
Signature · 2 lines
Andy Lutomirski AMA Capital Management, LLC
Andy Lutomirski
maintainerPATCH v4 14/16 x86/dumpstack/64: Handle faults when printing the "Stack:" part of an OOPS
In reply to Andy LutomirskiIf we overflow the stack into a guard page, we'll recursively fault when trying to dump the contents of the guard page. Use probe_kernel_address so we can recover if this happens.
--- arch/x86/kernel/dumpstack_64.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)
arch/x86/kernel/dumpstack_64.c +10 -2
@@ -274,6 +274,8 @@ show_stack_log_lvl(struct task_struct *task, struct pt_regs *regs, stack = sp;for (i =0 ; i < kstack_depth_to_print; i++) { +unsigned long word; +if (stack >= irq_stack && stack <= irq_stack_end) {if (stack == irq_stack_end) { stack = (unsigned long *) (irq_stack_end[-1 ]);
@@ -283,12 +285,18 @@ show_stack_log_lvl(struct task_struct *task, struct pt_regs *regs,if (kstack_end (stack))break ; } + +if (probe_kernel_address (stack, word)) +break ; +if ((i %STACKSLOTS_PER_LINE ) ==0 ) {if (i !=0 )pr_cont ("\n" ); -printk ("%s %016lx" , log_lvl, *stack++); +printk ("%s %016lx" , log_lvl, word); }else -pr_cont (" %016lx" , *stack++); +pr_cont (" %016lx" , word); + + stack++;touch_nmi_watchdog (); }preempt_enable ();
Signature · 1 line
2.5.5
Re: [PATCH v4 14/16] x86/dumpstack/64: Handle faults when printing the "Stack:" part of an OOPS
Quoted 5 lines · Andy Lutomirski
If we overflow the stack into a guard page, we'll recursively fault when trying to dump the contents of the guard page. Use
probe_kernel_addressso we can recover if this happens.Signed-off-by Andy Lutomirski <[email protected]>
Signature · 1 line
Josh
Andy Lutomirski
maintainerPATCH v4 15/16 x86/mm/64: Enable vmapped stacks
In reply to Andy LutomirskiThis allows x86_64 kernels to enable vmapped stacks. There are a couple of interesting bits.
First, x86 lazily faults in top-level paging entries for the vmalloc area. This won't work if we get a page fault while trying to access the stack: the CPU will promote it to a double-fault and we'll die. To avoid this problem, probe the new stack when switching stacks and forcibly populate the pgd entry for the stack when switching mms.
Second, once we have guard pages around the stack, we'll want to detect and handle stack overflow.
I didn't enable it on x86_32. We'd need to rework the double-fault code a bit and I'm concerned about running out of vmalloc virtual addresses under some workloads.
This patch, by itself, will behave somewhat erratically when the stack overflows while RSP is still more than a few tens of bytes above the bottom of the stack. Specifically, we'll get #PF and make it to no_context and an oops without triggering a double-fault, and no_context doesn't know about stack overflows. The next patch will improve that case.
--- arch/x86/Kconfig | 1 + arch/x86/include/asm/switch_to.h | 28 +++++++++++++++++++++++++++- arch/x86/kernel/traps.c | 32 ++++++++++++++++++++++++++++++++ arch/x86/mm/tlb.c | 15 +++++++++++++++ 4 files changed, 75 insertions(+), 1 deletion(-)
arch/x86/Kconfig +1 -0
@@ -92,6 +92,7 @@ config X86 select HAVE_ARCH_TRACEHOOK select HAVE_ARCH_TRANSPARENT_HUGEPAGE select HAVE_EBPF_JIT if X86_64 + select HAVE_ARCH_VMAP_STACK if X86_64 select HAVE_CC_STACKPROTECTOR select HAVE_CMPXCHG_DOUBLE select HAVE_CMPXCHG_LOCAL
arch/x86/include/asm/switch_to.h +27 -1
@@ -8,6 +8,28 @@ struct tss_struct;void __switch_to_xtra (struct task_struct *prev_p,struct task_struct *next_p,struct tss_struct *tss); +/* This runs runs on the previous thread's stack. */ +static inline void prepare_switch_to (struct task_struct *prev, +struct task_struct *next) +{ +#ifdef CONFIG_VMAP_STACK +/* +* If we switch to a stack that has a top-level paging entry +* that is not present in the current mm, the resulting #PF will +* will be promoted to a double-fault and we'll panic. Probe +* the new stack now so that vmalloc_fault can fix up the page +* tables if needed. This can only happen if we use a stack +* in vmap space. +* +* We assume that the stack is aligned so that it never spans +* more than one top-level paging entry. +* +* To minimize cache pollution, just follow the stack pointer. +*/ +READ_ONCE (*(unsigned char *)next->thread.sp); +#endif +} +#ifdef CONFIG_X86_32 #ifdef CONFIG_CC_STACKPROTECTOR
@@ -39,6 +61,8 @@ do { \*/ \unsigned long ebx, ecx, edx, esi, edi; \ \ +prepare_switch_to (prev, next); \ + \ asmvolatile ("pushl %%ebp\n\t" /* save EBP */ \"movl %%esp,%[prev_sp]\n\t" /* save ESP */ \ "movl %[next_sp],%%esp\n\t" /* restore ESP */ \
@@ -103,7 +127,9 @@ do { \* clean in kernel mode, with the possible exception of IOPL. Kernel IOPL * has no effect. */ -#define switch_to (prev, next, last) \ +#define switch_to (prev, next, last) \ + prepare_switch_to(prev, next); \ + \ asm volatile(SAVE_CONTEXT \ "movq %%rsp,%P[threadrsp](%[prev])\n\t"/* save RSP */ \"movq %P[threadrsp](%[next]),%%rsp\n\t" /* restore RSP */ \
arch/x86/kernel/traps.c +32 -0
@@ -292,12 +292,30 @@ DO_ERROR(X86_TRAP_NP, SIGBUS, "segment not present", segment_not_present) DO_ERROR(X86_TRAP_SS, SIGBUS, "stack segment", stack_segment) DO_ERROR(X86_TRAP_AC, SIGBUS, "alignment check", alignment_check) +#ifdef CONFIG_VMAP_STACK +static void __noreturn handle_stack_overflow (const char *message, +struct pt_regs *regs, +unsigned long fault_address) +{ +printk (KERN_EMERG "BUG: stack guard page was hit at %p (stack is %p..%p)\n" , + (void *)fault_address, current->stack, + (char *)current->stack +THREAD_SIZE -1 ); +die (message, regs,0 ); + +/* Be absolutely certain we don't return. */ +panic (message); +} +#endif +#ifdef CONFIG_X86_64 /* Runs on IST stack */ dotraplinkage void do_double_fault(struct pt_regs *regs, long error_code) {static const char str[] ="double fault" ;struct task_struct *tsk = current; +#ifdef CONFIG_VMAP_STACK +unsigned long cr2; +#endif #ifdef CONFIG_X86_ESPFIX64 extern unsigned char native_irq_return_iret[];
@@ -332,6 +350,20 @@ dotraplinkage void do_double_fault(struct pt_regs *regs, long error_code) tsk->thread.error_code = error_code; tsk->thread.trap_nr =X86_TRAP_DF ; +#ifdef CONFIG_VMAP_STACK +/* +* If we overflow the stack into a guard page, the CPU will fail +* to deliver #PF and will send #DF instead. CR2 will contain +* the linear address of the second fault, which will be in the +* guard page below the bottom of the stack. +*/ + cr2 =read_cr2 (); +if ((unsigned long )tsk->stack -1 - cr2 <PAGE_SIZE ) +handle_stack_overflow ( +"kernel stack overflow (double-fault)" , + regs, cr2); +#endif +#ifdef CONFIG_DOUBLEFAULT df_debug (regs, error_code);#endif
arch/x86/mm/tlb.c +15 -0
@@ -77,10 +77,25 @@ void switch_mm_irqs_off(struct mm_struct *prev, struct mm_struct *next,unsigned cpu =smp_processor_id ();if (likely (prev != next)) { +if (IS_ENABLED (CONFIG_VMAP_STACK )) { +/* +* If our current stack is in vmalloc space and isn't +* mapped in the new pgd, we'll double-fault. Forcibly +* map it. +*/ +unsigned int stack_pgd_index = +pgd_index (current_stack_pointer ()); +pgd_t *pgd = next->pgd + stack_pgd_index; + +if (unlikely (pgd_none (*pgd))) +set_pgd (pgd, init_mm.pgd[stack_pgd_index]); + } +#ifdef CONFIG_SMP this_cpu_write (cpu_tlbstate.state,TLBSTATE_OK );this_cpu_write (cpu_tlbstate.active_mm, next);#endif +cpumask_set_cpu (cpu,mm_cpumask (next));/*
Signature · 1 line
2.5.5
Andy Lutomirski
maintainerPATCH v4 16/16 x86/mm: Improve stack-overflow #PF handling
In reply to Andy LutomirskiIf we get a page fault indicating kernel stack overflow, invoke handle_stack_overflow(). To prevent us from overflowing the stack again while handling the overflow (because we are likely to have very little stack space left), call handle_stack_overflow() on the double-fault stack
--- arch/x86/include/asm/traps.h | 6 ++++++ arch/x86/kernel/traps.c | 6 +++--- arch/x86/mm/fault.c | 39 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 48 insertions(+), 3 deletions(-)
arch/x86/include/asm/traps.h +6 -0
@@ -117,6 +117,12 @@ extern void ist_exit(struct pt_regs *regs);extern void ist_begin_non_atomic (struct pt_regs *regs);extern void ist_end_non_atomic (void ); +#ifdef CONFIG_VMAP_STACK +void __noreturn handle_stack_overflow (const char *message, +struct pt_regs *regs, +unsigned long fault_address); +#endif +/* Interrupts/Exceptions */ enum {X86_TRAP_DE =0 ,/* 0, Divide-by-zero */
arch/x86/kernel/traps.c +3 -3
@@ -293,9 +293,9 @@ DO_ERROR(X86_TRAP_SS, SIGBUS, "stack segment", stack_segment) DO_ERROR(X86_TRAP_AC, SIGBUS, "alignment check", alignment_check)#ifdef CONFIG_VMAP_STACK -static void __noreturn handle_stack_overflow (const char *message, -struct pt_regs *regs, -unsigned long fault_address) +__visible void __noreturn handle_stack_overflow(const char *message, +struct pt_regs *regs, +unsigned long fault_address) { printk(KERN_EMERG "BUG: stack guard page was hit at %p (stack is %p..%p)\n", (void *)fault_address, current->stack,
arch/x86/mm/fault.c +39 -0
@@ -753,6 +753,45 @@ no_context(struct pt_regs *regs, unsigned long error_code,return ; } +#ifdef CONFIG_VMAP_STACK +/* +* Stack overflow? During boot, we can fault near the initial +* stack in the direct map, but that's not an overflow -- check +* that we're in vmalloc space to avoid this. +* +* Check this after trying fixup_exception, since there are handful +* of kernel code paths that wander off the top of the stack but +* handle any faults that occur. Once those are fixed, we can +* move this above fixup_exception. +*/ +if (is_vmalloc_addr ((void *)address) && + (((unsigned long )tsk->stack -1 - address <PAGE_SIZE ) || + address - ((unsigned long )tsk->stack +THREAD_SIZE ) <PAGE_SIZE )) { + registervoid *__sp asm("rsp" ); +unsigned long stack = +this_cpu_read (orig_ist.ist[DOUBLEFAULT_STACK ]) - +sizeof (void *); +/* +* We're likely to be running with very little stack space +* left. It's plausible that we'd hit this condition but +* double-fault even before we get this far, in which case +* we're fine: the double-fault handler will deal with it. +* +* We don't want to make it all the way into the oops code +* and then double-fault, though, because we're likely to +* break the console driver and lose most of the stack dump. +*/ + asmvolatile ("movq %[stack], %%rsp\n\t" +"call handle_stack_overflow\n\t" +"1: jmp 1b" + :"+r" (__sp) + :"D" ("kernel stack overflow (page fault)" ), +"S" (regs),"d" (address), + [stack]"rm" (stack)); +unreachable (); + } +#endif +/* * 32-bit: *
Signature · 1 line
2.5.5