]>
Commit | Line | Data |
---|---|---|
7f30898e GKH |
1 | From ad7b4e8022b9864c075fe71e1328b1d25cad82f6 Mon Sep 17 00:00:00 2001 |
2 | From: Frederic Barrat <fbarrat@linux.vnet.ibm.com> | |
3 | Date: Tue, 3 Apr 2018 15:54:02 +0200 | |
4 | Subject: cxl: Fix possible deadlock when processing page faults from cxllib | |
5 | ||
6 | From: Frederic Barrat <fbarrat@linux.vnet.ibm.com> | |
7 | ||
8 | commit ad7b4e8022b9864c075fe71e1328b1d25cad82f6 upstream. | |
9 | ||
10 | cxllib_handle_fault() is called by an external driver when it needs to | |
11 | have the host resolve page faults for a buffer. The buffer can cover | |
12 | several pages and VMAs. The function iterates over all the pages used | |
13 | by the buffer, based on the page size of the VMA. | |
14 | ||
15 | To ensure some stability while processing the faults, the thread T1 | |
16 | grabs the mm->mmap_sem semaphore with read access (R1). However, when | |
17 | processing a page fault for a single page, one of the underlying | |
18 | functions, copro_handle_mm_fault(), also grabs the same semaphore with | |
19 | read access (R2). So the thread T1 takes the semaphore twice. | |
20 | ||
21 | If another thread T2 tries to access the semaphore in write mode W1 | |
22 | (say, because it wants to allocate memory and calls 'brk'), then that | |
23 | thread T2 will have to wait because there's a reader (R1). If the | |
24 | thread T1 is processing a new page at that time, it won't get an | |
25 | automatic grant at R2, because there's now a writer thread | |
26 | waiting (T2). And we have a deadlock. | |
27 | ||
28 | The timeline is: | |
29 | 1. thread T1 owns the semaphore with read access R1 | |
30 | 2. thread T2 requests write access W1 and waits | |
31 | 3. thread T1 requests read access R2 and waits | |
32 | ||
33 | The fix is for the thread T1 to release the semaphore R1 once it got | |
34 | the information it needs from the current VMA. The address space/VMAs | |
35 | could evolve while T1 iterates over the full buffer, but in the | |
36 | unlikely case where T1 misses a page, the external driver will raise a | |
37 | new page fault when retrying the memory access. | |
38 | ||
39 | Fixes: 3ced8d730063 ("cxl: Export library to support IBM XSL") | |
40 | Cc: stable@vger.kernel.org # 4.13+ | |
41 | Signed-off-by: Frederic Barrat <fbarrat@linux.vnet.ibm.com> | |
42 | Signed-off-by: Michael Ellerman <mpe@ellerman.id.au> | |
43 | Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> | |
44 | ||
45 | --- | |
46 | drivers/misc/cxl/cxllib.c | 83 +++++++++++++++++++++++++++++----------------- | |
47 | 1 file changed, 54 insertions(+), 29 deletions(-) | |
48 | ||
49 | --- a/drivers/misc/cxl/cxllib.c | |
50 | +++ b/drivers/misc/cxl/cxllib.c | |
51 | @@ -208,49 +208,74 @@ int cxllib_get_PE_attributes(struct task | |
52 | } | |
53 | EXPORT_SYMBOL_GPL(cxllib_get_PE_attributes); | |
54 | ||
55 | -int cxllib_handle_fault(struct mm_struct *mm, u64 addr, u64 size, u64 flags) | |
56 | +static int get_vma_info(struct mm_struct *mm, u64 addr, | |
57 | + u64 *vma_start, u64 *vma_end, | |
58 | + unsigned long *page_size) | |
59 | { | |
60 | - int rc; | |
61 | - u64 dar; | |
62 | struct vm_area_struct *vma = NULL; | |
63 | - unsigned long page_size; | |
64 | - | |
65 | - if (mm == NULL) | |
66 | - return -EFAULT; | |
67 | + int rc = 0; | |
68 | ||
69 | down_read(&mm->mmap_sem); | |
70 | ||
71 | vma = find_vma(mm, addr); | |
72 | if (!vma) { | |
73 | - pr_err("Can't find vma for addr %016llx\n", addr); | |
74 | rc = -EFAULT; | |
75 | goto out; | |
76 | } | |
77 | - /* get the size of the pages allocated */ | |
78 | - page_size = vma_kernel_pagesize(vma); | |
79 | + *page_size = vma_kernel_pagesize(vma); | |
80 | + *vma_start = vma->vm_start; | |
81 | + *vma_end = vma->vm_end; | |
82 | +out: | |
83 | + up_read(&mm->mmap_sem); | |
84 | + return rc; | |
85 | +} | |
86 | + | |
87 | +int cxllib_handle_fault(struct mm_struct *mm, u64 addr, u64 size, u64 flags) | |
88 | +{ | |
89 | + int rc; | |
90 | + u64 dar, vma_start, vma_end; | |
91 | + unsigned long page_size; | |
92 | ||
93 | - for (dar = (addr & ~(page_size - 1)); dar < (addr + size); dar += page_size) { | |
94 | - if (dar < vma->vm_start || dar >= vma->vm_end) { | |
95 | - vma = find_vma(mm, addr); | |
96 | - if (!vma) { | |
97 | - pr_err("Can't find vma for addr %016llx\n", addr); | |
98 | - rc = -EFAULT; | |
99 | - goto out; | |
100 | - } | |
101 | - /* get the size of the pages allocated */ | |
102 | - page_size = vma_kernel_pagesize(vma); | |
103 | + if (mm == NULL) | |
104 | + return -EFAULT; | |
105 | + | |
106 | + /* | |
107 | + * The buffer we have to process can extend over several pages | |
108 | + * and may also cover several VMAs. | |
109 | + * We iterate over all the pages. The page size could vary | |
110 | + * between VMAs. | |
111 | + */ | |
112 | + rc = get_vma_info(mm, addr, &vma_start, &vma_end, &page_size); | |
113 | + if (rc) | |
114 | + return rc; | |
115 | + | |
116 | + for (dar = (addr & ~(page_size - 1)); dar < (addr + size); | |
117 | + dar += page_size) { | |
118 | + if (dar < vma_start || dar >= vma_end) { | |
119 | + /* | |
120 | + * We don't hold the mm->mmap_sem semaphore | |
121 | + * while iterating, since the semaphore is | |
122 | + * required by one of the lower-level page | |
123 | + * fault processing functions and it could | |
124 | + * create a deadlock. | |
125 | + * | |
126 | + * It means the VMAs can be altered between 2 | |
127 | + * loop iterations and we could theoretically | |
128 | + * miss a page (however unlikely). But that's | |
129 | + * not really a problem, as the driver will | |
130 | + * retry access, get another page fault on the | |
131 | + * missing page and call us again. | |
132 | + */ | |
133 | + rc = get_vma_info(mm, dar, &vma_start, &vma_end, | |
134 | + &page_size); | |
135 | + if (rc) | |
136 | + return rc; | |
137 | } | |
138 | ||
139 | rc = cxl_handle_mm_fault(mm, flags, dar); | |
140 | - if (rc) { | |
141 | - pr_err("cxl_handle_mm_fault failed %d", rc); | |
142 | - rc = -EFAULT; | |
143 | - goto out; | |
144 | - } | |
145 | + if (rc) | |
146 | + return -EFAULT; | |
147 | } | |
148 | - rc = 0; | |
149 | -out: | |
150 | - up_read(&mm->mmap_sem); | |
151 | - return rc; | |
152 | + return 0; | |
153 | } | |
154 | EXPORT_SYMBOL_GPL(cxllib_handle_fault); |