0244-x86-dumpstack-Fix-partial-register-dumps.patch 6.5 KB

123456789101112131415161718192021222324252627282930313233343536373839404142434445464748495051525354555657585960616263646566676869707172737475767778798081828384858687888990919293949596979899100101102103104105106107108109110111112113114115116117118119120121122123124125126127128129130131132133134135136137138139140141142143144145146147148149150151152153154155156157158159160161162163164165166167168169170171172
  1. From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
  2. From: Josh Poimboeuf <[email protected]>
  3. Date: Sun, 31 Dec 2017 10:18:06 -0600
  4. Subject: [PATCH] x86/dumpstack: Fix partial register dumps
  5. MIME-Version: 1.0
  6. Content-Type: text/plain; charset=UTF-8
  7. Content-Transfer-Encoding: 8bit
  8. CVE-2017-5754
  9. The show_regs_safe() logic is wrong. When there's an iret stack frame,
  10. it prints the entire pt_regs -- most of which is random stack data --
  11. instead of just the five registers at the end.
  12. show_regs_safe() is also poorly named: the on_stack() checks aren't for
  13. safety. Rename the function to show_regs_if_on_stack() and add a
  14. comment to explain why the checks are needed.
  15. These issues were introduced with the "partial register dump" feature of
  16. the following commit:
  17. b02fcf9ba121 ("x86/unwinder: Handle stack overflows more gracefully")
  18. That patch had gone through a few iterations of development, and the
  19. above issues were artifacts from a previous iteration of the patch where
  20. 'regs' pointed directly to the iret frame rather than to the (partially
  21. empty) pt_regs.
  22. Tested-by: Alexander Tsoy <[email protected]>
  23. Signed-off-by: Josh Poimboeuf <[email protected]>
  24. Cc: Andy Lutomirski <[email protected]>
  25. Cc: Linus Torvalds <[email protected]>
  26. Cc: Peter Zijlstra <[email protected]>
  27. Cc: Thomas Gleixner <[email protected]>
  28. Cc: Toralf Förster <[email protected]>
  29. Cc: [email protected]
  30. Fixes: b02fcf9ba121 ("x86/unwinder: Handle stack overflows more gracefully")
  31. Link: http://lkml.kernel.org/r/5b05b8b344f59db2d3d50dbdeba92d60f2304c54.1514736742.git.jpoimboe@redhat.com
  32. Signed-off-by: Ingo Molnar <[email protected]>
  33. (cherry picked from commit a9cdbe72c4e8bf3b38781c317a79326e2e1a230d)
  34. Signed-off-by: Andy Whitcroft <[email protected]>
  35. Signed-off-by: Kleber Sacilotto de Souza <[email protected]>
  36. (cherry picked from commit 3f159d02ecca1ffe81dc467767833dd6d0345147)
  37. Signed-off-by: Fabian Grünbichler <[email protected]>
  38. ---
  39. arch/x86/include/asm/unwind.h | 17 +++++++++++++----
  40. arch/x86/kernel/dumpstack.c | 28 ++++++++++++++++++++--------
  41. arch/x86/kernel/stacktrace.c | 2 +-
  42. 3 files changed, 34 insertions(+), 13 deletions(-)
  43. diff --git a/arch/x86/include/asm/unwind.h b/arch/x86/include/asm/unwind.h
  44. index 38fa6154e382..e1c1cb5019bc 100644
  45. --- a/arch/x86/include/asm/unwind.h
  46. +++ b/arch/x86/include/asm/unwind.h
  47. @@ -55,18 +55,27 @@ void unwind_start(struct unwind_state *state, struct task_struct *task,
  48. #if defined(CONFIG_UNWINDER_ORC) || defined(CONFIG_UNWINDER_FRAME_POINTER)
  49. /*
  50. - * WARNING: The entire pt_regs may not be safe to dereference. In some cases,
  51. - * only the iret frame registers are accessible. Use with caution!
  52. + * If 'partial' returns true, only the iret frame registers are valid.
  53. */
  54. -static inline struct pt_regs *unwind_get_entry_regs(struct unwind_state *state)
  55. +static inline struct pt_regs *unwind_get_entry_regs(struct unwind_state *state,
  56. + bool *partial)
  57. {
  58. if (unwind_done(state))
  59. return NULL;
  60. + if (partial) {
  61. +#ifdef CONFIG_UNWINDER_ORC
  62. + *partial = !state->full_regs;
  63. +#else
  64. + *partial = false;
  65. +#endif
  66. + }
  67. +
  68. return state->regs;
  69. }
  70. #else
  71. -static inline struct pt_regs *unwind_get_entry_regs(struct unwind_state *state)
  72. +static inline struct pt_regs *unwind_get_entry_regs(struct unwind_state *state,
  73. + bool *partial)
  74. {
  75. return NULL;
  76. }
  77. diff --git a/arch/x86/kernel/dumpstack.c b/arch/x86/kernel/dumpstack.c
  78. index 19a936e9b259..8da5b487919f 100644
  79. --- a/arch/x86/kernel/dumpstack.c
  80. +++ b/arch/x86/kernel/dumpstack.c
  81. @@ -76,12 +76,23 @@ void show_iret_regs(struct pt_regs *regs)
  82. regs->sp, regs->flags);
  83. }
  84. -static void show_regs_safe(struct stack_info *info, struct pt_regs *regs)
  85. +static void show_regs_if_on_stack(struct stack_info *info, struct pt_regs *regs,
  86. + bool partial)
  87. {
  88. - if (on_stack(info, regs, sizeof(*regs)))
  89. + /*
  90. + * These on_stack() checks aren't strictly necessary: the unwind code
  91. + * has already validated the 'regs' pointer. The checks are done for
  92. + * ordering reasons: if the registers are on the next stack, we don't
  93. + * want to print them out yet. Otherwise they'll be shown as part of
  94. + * the wrong stack. Later, when show_trace_log_lvl() switches to the
  95. + * next stack, this function will be called again with the same regs so
  96. + * they can be printed in the right context.
  97. + */
  98. + if (!partial && on_stack(info, regs, sizeof(*regs))) {
  99. __show_regs(regs, 0);
  100. - else if (on_stack(info, (void *)regs + IRET_FRAME_OFFSET,
  101. - IRET_FRAME_SIZE)) {
  102. +
  103. + } else if (partial && on_stack(info, (void *)regs + IRET_FRAME_OFFSET,
  104. + IRET_FRAME_SIZE)) {
  105. /*
  106. * When an interrupt or exception occurs in entry code, the
  107. * full pt_regs might not have been saved yet. In that case
  108. @@ -98,6 +109,7 @@ void show_trace_log_lvl(struct task_struct *task, struct pt_regs *regs,
  109. struct stack_info stack_info = {0};
  110. unsigned long visit_mask = 0;
  111. int graph_idx = 0;
  112. + bool partial;
  113. printk("%sCall Trace:\n", log_lvl);
  114. @@ -140,7 +152,7 @@ void show_trace_log_lvl(struct task_struct *task, struct pt_regs *regs,
  115. printk("%s <%s>\n", log_lvl, stack_name);
  116. if (regs)
  117. - show_regs_safe(&stack_info, regs);
  118. + show_regs_if_on_stack(&stack_info, regs, partial);
  119. /*
  120. * Scan the stack, printing any text addresses we find. At the
  121. @@ -164,7 +176,7 @@ void show_trace_log_lvl(struct task_struct *task, struct pt_regs *regs,
  122. /*
  123. * Don't print regs->ip again if it was already printed
  124. - * by show_regs_safe() below.
  125. + * by show_regs_if_on_stack().
  126. */
  127. if (regs && stack == &regs->ip) {
  128. unwind_next_frame(&state);
  129. @@ -200,9 +212,9 @@ void show_trace_log_lvl(struct task_struct *task, struct pt_regs *regs,
  130. unwind_next_frame(&state);
  131. /* if the frame has entry regs, print them */
  132. - regs = unwind_get_entry_regs(&state);
  133. + regs = unwind_get_entry_regs(&state, &partial);
  134. if (regs)
  135. - show_regs_safe(&stack_info, regs);
  136. + show_regs_if_on_stack(&stack_info, regs, partial);
  137. }
  138. if (stack_name)
  139. diff --git a/arch/x86/kernel/stacktrace.c b/arch/x86/kernel/stacktrace.c
  140. index 8dabd7bf1673..60244bfaf88f 100644
  141. --- a/arch/x86/kernel/stacktrace.c
  142. +++ b/arch/x86/kernel/stacktrace.c
  143. @@ -98,7 +98,7 @@ static int __save_stack_trace_reliable(struct stack_trace *trace,
  144. for (unwind_start(&state, task, NULL, NULL); !unwind_done(&state);
  145. unwind_next_frame(&state)) {
  146. - regs = unwind_get_entry_regs(&state);
  147. + regs = unwind_get_entry_regs(&state, NULL);
  148. if (regs) {
  149. /*
  150. * Kernel mode registers on the stack indicate an
  151. --
  152. 2.14.2