]>
Commit | Line | Data |
---|---|---|
7eb50402 GKH |
1 | From 48fb6f4db940e92cfb16cd878cddd59ea6120d06 Mon Sep 17 00:00:00 2001 |
2 | From: Mel Gorman <mgorman@suse.de> | |
3 | Date: Wed, 9 Aug 2017 08:27:11 +0100 | |
4 | Subject: futex: Remove unnecessary warning from get_futex_key | |
5 | ||
6 | From: Mel Gorman <mgorman@suse.de> | |
7 | ||
8 | commit 48fb6f4db940e92cfb16cd878cddd59ea6120d06 upstream. | |
9 | ||
10 | Commit 65d8fc777f6d ("futex: Remove requirement for lock_page() in | |
11 | get_futex_key()") removed an unnecessary lock_page() with the | |
12 | side-effect that page->mapping needed to be treated very carefully. | |
13 | ||
14 | Two defensive warnings were added in case any assumption was missed and | |
15 | the first warning assumed a correct application would not alter a | |
16 | mapping backing a futex key. Since merging, it has not triggered for | |
17 | any unexpected case but Mark Rutland reported the following bug | |
18 | triggering due to the first warning. | |
19 | ||
20 | kernel BUG at kernel/futex.c:679! | |
21 | Internal error: Oops - BUG: 0 [#1] PREEMPT SMP | |
22 | Modules linked in: | |
23 | CPU: 0 PID: 3695 Comm: syz-executor1 Not tainted 4.13.0-rc3-00020-g307fec773ba3 #3 | |
24 | Hardware name: linux,dummy-virt (DT) | |
25 | task: ffff80001e271780 task.stack: ffff000010908000 | |
26 | PC is at get_futex_key+0x6a4/0xcf0 kernel/futex.c:679 | |
27 | LR is at get_futex_key+0x6a4/0xcf0 kernel/futex.c:679 | |
28 | pc : [<ffff00000821ac14>] lr : [<ffff00000821ac14>] pstate: 80000145 | |
29 | ||
30 | The fact that it's a bug instead of a warning was due to an unrelated | |
31 | arm64 problem, but the warning itself triggered because the underlying | |
32 | mapping changed. | |
33 | ||
34 | This is an application issue but from a kernel perspective it's a | |
35 | recoverable situation and the warning is unnecessary so this patch | |
36 | removes the warning. The warning may potentially be triggered with the | |
37 | following test program from Mark although it may be necessary to adjust | |
38 | NR_FUTEX_THREADS to be a value smaller than the number of CPUs in the | |
39 | system. | |
40 | ||
41 | #include <linux/futex.h> | |
42 | #include <pthread.h> | |
43 | #include <stdio.h> | |
44 | #include <stdlib.h> | |
45 | #include <sys/mman.h> | |
46 | #include <sys/syscall.h> | |
47 | #include <sys/time.h> | |
48 | #include <unistd.h> | |
49 | ||
50 | #define NR_FUTEX_THREADS 16 | |
51 | pthread_t threads[NR_FUTEX_THREADS]; | |
52 | ||
53 | void *mem; | |
54 | ||
55 | #define MEM_PROT (PROT_READ | PROT_WRITE) | |
56 | #define MEM_SIZE 65536 | |
57 | ||
58 | static int futex_wrapper(int *uaddr, int op, int val, | |
59 | const struct timespec *timeout, | |
60 | int *uaddr2, int val3) | |
61 | { | |
62 | syscall(SYS_futex, uaddr, op, val, timeout, uaddr2, val3); | |
63 | } | |
64 | ||
65 | void *poll_futex(void *unused) | |
66 | { | |
67 | for (;;) { | |
68 | futex_wrapper(mem, FUTEX_CMP_REQUEUE_PI, 1, NULL, mem + 4, 1); | |
69 | } | |
70 | } | |
71 | ||
72 | int main(int argc, char *argv[]) | |
73 | { | |
74 | int i; | |
75 | ||
76 | mem = mmap(NULL, MEM_SIZE, MEM_PROT, | |
77 | MAP_SHARED | MAP_ANONYMOUS, -1, 0); | |
78 | ||
79 | printf("Mapping @ %p\n", mem); | |
80 | ||
81 | printf("Creating futex threads...\n"); | |
82 | ||
83 | for (i = 0; i < NR_FUTEX_THREADS; i++) | |
84 | pthread_create(&threads[i], NULL, poll_futex, NULL); | |
85 | ||
86 | printf("Flipping mapping...\n"); | |
87 | for (;;) { | |
88 | mmap(mem, MEM_SIZE, MEM_PROT, | |
89 | MAP_FIXED | MAP_SHARED | MAP_ANONYMOUS, -1, 0); | |
90 | } | |
91 | ||
92 | return 0; | |
93 | } | |
94 | ||
95 | Reported-and-tested-by: Mark Rutland <mark.rutland@arm.com> | |
96 | Signed-off-by: Mel Gorman <mgorman@suse.de> | |
97 | Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org> | |
98 | Cc: stable@vger.kernel.org # 4.7+ | |
99 | Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> | |
100 | Cc: Ben Hutchings <ben.hutchings@codethink.co.uk> | |
101 | Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> | |
102 | ||
103 | --- | |
104 | kernel/futex.c | 5 +++-- | |
105 | 1 file changed, 3 insertions(+), 2 deletions(-) | |
106 | ||
107 | --- a/kernel/futex.c | |
108 | +++ b/kernel/futex.c | |
109 | @@ -666,13 +666,14 @@ again: | |
110 | * this reference was taken by ihold under the page lock | |
111 | * pinning the inode in place so i_lock was unnecessary. The | |
112 | * only way for this check to fail is if the inode was | |
113 | - * truncated in parallel so warn for now if this happens. | |
114 | + * truncated in parallel which is almost certainly an | |
115 | + * application bug. In such a case, just retry. | |
116 | * | |
117 | * We are not calling into get_futex_key_refs() in file-backed | |
118 | * cases, therefore a successful atomic_inc return below will | |
119 | * guarantee that get_futex_key() will still imply smp_mb(); (B). | |
120 | */ | |
121 | - if (WARN_ON_ONCE(!atomic_inc_not_zero(&inode->i_count))) { | |
122 | + if (!atomic_inc_not_zero(&inode->i_count)) { | |
123 | rcu_read_unlock(); | |
124 | put_page(page_head); | |
125 |