]>
Commit | Line | Data |
---|---|---|
7a7567a2 GKH |
1 | From de9f869616dd95e95c00bdd6b0fcd3421e8a4323 Mon Sep 17 00:00:00 2001 |
2 | From: Jann Horn <jannh@google.com> | |
3 | Date: Sun, 2 Jun 2019 03:15:58 +0200 | |
4 | Subject: x86/insn-eval: Fix use-after-free access to LDT entry | |
5 | ||
6 | From: Jann Horn <jannh@google.com> | |
7 | ||
8 | commit de9f869616dd95e95c00bdd6b0fcd3421e8a4323 upstream. | |
9 | ||
10 | get_desc() computes a pointer into the LDT while holding a lock that | |
11 | protects the LDT from being freed, but then drops the lock and returns the | |
12 | (now potentially dangling) pointer to its caller. | |
13 | ||
14 | Fix it by giving the caller a copy of the LDT entry instead. | |
15 | ||
16 | Fixes: 670f928ba09b ("x86/insn-eval: Add utility function to get segment descriptor") | |
17 | Cc: stable@vger.kernel.org | |
18 | Signed-off-by: Jann Horn <jannh@google.com> | |
19 | Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> | |
20 | Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> | |
21 | ||
22 | --- | |
23 | arch/x86/lib/insn-eval.c | 47 ++++++++++++++++++++++++----------------------- | |
24 | 1 file changed, 24 insertions(+), 23 deletions(-) | |
25 | ||
26 | --- a/arch/x86/lib/insn-eval.c | |
27 | +++ b/arch/x86/lib/insn-eval.c | |
28 | @@ -557,7 +557,8 @@ static int get_reg_offset_16(struct insn | |
29 | } | |
30 | ||
31 | /** | |
32 | - * get_desc() - Obtain pointer to a segment descriptor | |
33 | + * get_desc() - Obtain contents of a segment descriptor | |
34 | + * @out: Segment descriptor contents on success | |
35 | * @sel: Segment selector | |
36 | * | |
37 | * Given a segment selector, obtain a pointer to the segment descriptor. | |
38 | @@ -565,18 +566,18 @@ static int get_reg_offset_16(struct insn | |
39 | * | |
40 | * Returns: | |
41 | * | |
42 | - * Pointer to segment descriptor on success. | |
43 | + * True on success, false on failure. | |
44 | * | |
45 | * NULL on error. | |
46 | */ | |
47 | -static struct desc_struct *get_desc(unsigned short sel) | |
48 | +static bool get_desc(struct desc_struct *out, unsigned short sel) | |
49 | { | |
50 | struct desc_ptr gdt_desc = {0, 0}; | |
51 | unsigned long desc_base; | |
52 | ||
53 | #ifdef CONFIG_MODIFY_LDT_SYSCALL | |
54 | if ((sel & SEGMENT_TI_MASK) == SEGMENT_LDT) { | |
55 | - struct desc_struct *desc = NULL; | |
56 | + bool success = false; | |
57 | struct ldt_struct *ldt; | |
58 | ||
59 | /* Bits [15:3] contain the index of the desired entry. */ | |
60 | @@ -584,12 +585,14 @@ static struct desc_struct *get_desc(unsi | |
61 | ||
62 | mutex_lock(¤t->active_mm->context.lock); | |
63 | ldt = current->active_mm->context.ldt; | |
64 | - if (ldt && sel < ldt->nr_entries) | |
65 | - desc = &ldt->entries[sel]; | |
66 | + if (ldt && sel < ldt->nr_entries) { | |
67 | + *out = ldt->entries[sel]; | |
68 | + success = true; | |
69 | + } | |
70 | ||
71 | mutex_unlock(¤t->active_mm->context.lock); | |
72 | ||
73 | - return desc; | |
74 | + return success; | |
75 | } | |
76 | #endif | |
77 | native_store_gdt(&gdt_desc); | |
78 | @@ -604,9 +607,10 @@ static struct desc_struct *get_desc(unsi | |
79 | desc_base = sel & ~(SEGMENT_RPL_MASK | SEGMENT_TI_MASK); | |
80 | ||
81 | if (desc_base > gdt_desc.size) | |
82 | - return NULL; | |
83 | + return false; | |
84 | ||
85 | - return (struct desc_struct *)(gdt_desc.address + desc_base); | |
86 | + *out = *(struct desc_struct *)(gdt_desc.address + desc_base); | |
87 | + return true; | |
88 | } | |
89 | ||
90 | /** | |
91 | @@ -628,7 +632,7 @@ static struct desc_struct *get_desc(unsi | |
92 | */ | |
93 | unsigned long insn_get_seg_base(struct pt_regs *regs, int seg_reg_idx) | |
94 | { | |
95 | - struct desc_struct *desc; | |
96 | + struct desc_struct desc; | |
97 | short sel; | |
98 | ||
99 | sel = get_segment_selector(regs, seg_reg_idx); | |
100 | @@ -666,11 +670,10 @@ unsigned long insn_get_seg_base(struct p | |
101 | if (!sel) | |
102 | return -1L; | |
103 | ||
104 | - desc = get_desc(sel); | |
105 | - if (!desc) | |
106 | + if (!get_desc(&desc, sel)) | |
107 | return -1L; | |
108 | ||
109 | - return get_desc_base(desc); | |
110 | + return get_desc_base(&desc); | |
111 | } | |
112 | ||
113 | /** | |
114 | @@ -692,7 +695,7 @@ unsigned long insn_get_seg_base(struct p | |
115 | */ | |
116 | static unsigned long get_seg_limit(struct pt_regs *regs, int seg_reg_idx) | |
117 | { | |
118 | - struct desc_struct *desc; | |
119 | + struct desc_struct desc; | |
120 | unsigned long limit; | |
121 | short sel; | |
122 | ||
123 | @@ -706,8 +709,7 @@ static unsigned long get_seg_limit(struc | |
124 | if (!sel) | |
125 | return 0; | |
126 | ||
127 | - desc = get_desc(sel); | |
128 | - if (!desc) | |
129 | + if (!get_desc(&desc, sel)) | |
130 | return 0; | |
131 | ||
132 | /* | |
133 | @@ -716,8 +718,8 @@ static unsigned long get_seg_limit(struc | |
134 | * not tested when checking the segment limits. In practice, | |
135 | * this means that the segment ends in (limit << 12) + 0xfff. | |
136 | */ | |
137 | - limit = get_desc_limit(desc); | |
138 | - if (desc->g) | |
139 | + limit = get_desc_limit(&desc); | |
140 | + if (desc.g) | |
141 | limit = (limit << 12) + 0xfff; | |
142 | ||
143 | return limit; | |
144 | @@ -741,7 +743,7 @@ static unsigned long get_seg_limit(struc | |
145 | */ | |
146 | int insn_get_code_seg_params(struct pt_regs *regs) | |
147 | { | |
148 | - struct desc_struct *desc; | |
149 | + struct desc_struct desc; | |
150 | short sel; | |
151 | ||
152 | if (v8086_mode(regs)) | |
153 | @@ -752,8 +754,7 @@ int insn_get_code_seg_params(struct pt_r | |
154 | if (sel < 0) | |
155 | return sel; | |
156 | ||
157 | - desc = get_desc(sel); | |
158 | - if (!desc) | |
159 | + if (!get_desc(&desc, sel)) | |
160 | return -EINVAL; | |
161 | ||
162 | /* | |
163 | @@ -761,10 +762,10 @@ int insn_get_code_seg_params(struct pt_r | |
164 | * determines whether a segment contains data or code. If this is a data | |
165 | * segment, return error. | |
166 | */ | |
167 | - if (!(desc->type & BIT(3))) | |
168 | + if (!(desc.type & BIT(3))) | |
169 | return -EINVAL; | |
170 | ||
171 | - switch ((desc->l << 1) | desc->d) { | |
172 | + switch ((desc.l << 1) | desc.d) { | |
173 | case 0: /* | |
174 | * Legacy mode. CS.L=0, CS.D=0. Address and operand size are | |
175 | * both 16-bit. |