diff options
author | Seiji Aguchi <seiji.aguchi@hds.com> | 2013-10-30 15:27:26 -0400 |
---|---|---|
committer | Matt Fleming <matt.fleming@intel.com> | 2013-11-28 15:16:55 -0500 |
commit | e0d59733f6b1796b8d6692642c87d7dd862c3e3a (patch) | |
tree | 4014bb6e375cfa973415e56252a9711dbaada317 /drivers/firmware | |
parent | 6ce4eac1f600b34f2f7f58f9cd8f0503d79e42ae (diff) |
efivars, efi-pstore: Hold off deletion of sysfs entry until the scan is completed
Currently, when mounting pstore file system, a read callback of
efi_pstore driver runs mutiple times as below.
- In the first read callback, scan efivar_sysfs_list from head and pass
a kmsg buffer of a entry to an upper pstore layer.
- In the second read callback, rescan efivar_sysfs_list from the entry
and pass another kmsg buffer to it.
- Repeat the scan and pass until the end of efivar_sysfs_list.
In this process, an entry is read across the multiple read function
calls. To avoid race between the read and erasion, the whole process
above is protected by a spinlock, holding in open() and releasing in
close().
At the same time, kmemdup() is called to pass the buffer to pstore
filesystem during it. And then, it causes a following lockdep warning.
To make the dynamic memory allocation runnable without taking spinlock,
holding off a deletion of sysfs entry if it happens while scanning it
via efi_pstore, and deleting it after the scan is completed.
To implement it, this patch introduces two flags, scanning and deleting,
to efivar_entry.
On the code basis, it seems that all the scanning and deleting logic is
not needed because __efivars->lock are not dropped when reading from the
EFI variable store.
But, the scanning and deleting logic is still needed because an
efi-pstore and a pstore filesystem works as follows.
In case an entry(A) is found, the pointer is saved to psi->data. And
efi_pstore_read() passes the entry(A) to a pstore filesystem by
releasing __efivars->lock.
And then, the pstore filesystem calls efi_pstore_read() again and the
same entry(A), which is saved to psi->data, is used for resuming to scan
a sysfs-list.
So, to protect the entry(A), the logic is needed.
[ 1.143710] ------------[ cut here ]------------
[ 1.144058] WARNING: CPU: 1 PID: 1 at kernel/lockdep.c:2740 lockdep_trace_alloc+0x104/0x110()
[ 1.144058] DEBUG_LOCKS_WARN_ON(irqs_disabled_flags(flags))
[ 1.144058] Modules linked in:
[ 1.144058] CPU: 1 PID: 1 Comm: systemd Not tainted 3.11.0-rc5 #2
[ 1.144058] 0000000000000009 ffff8800797e9ae0 ffffffff816614a5 ffff8800797e9b28
[ 1.144058] ffff8800797e9b18 ffffffff8105510d 0000000000000080 0000000000000046
[ 1.144058] 00000000000000d0 00000000000003af ffffffff81ccd0c0 ffff8800797e9b78
[ 1.144058] Call Trace:
[ 1.144058] [<ffffffff816614a5>] dump_stack+0x54/0x74
[ 1.144058] [<ffffffff8105510d>] warn_slowpath_common+0x7d/0xa0
[ 1.144058] [<ffffffff8105517c>] warn_slowpath_fmt+0x4c/0x50
[ 1.144058] [<ffffffff8131290f>] ? vsscanf+0x57f/0x7b0
[ 1.144058] [<ffffffff810bbd74>] lockdep_trace_alloc+0x104/0x110
[ 1.144058] [<ffffffff81192da0>] __kmalloc_track_caller+0x50/0x280
[ 1.144058] [<ffffffff815147bb>] ? efi_pstore_read_func.part.1+0x12b/0x170
[ 1.144058] [<ffffffff8115b260>] kmemdup+0x20/0x50
[ 1.144058] [<ffffffff815147bb>] efi_pstore_read_func.part.1+0x12b/0x170
[ 1.144058] [<ffffffff81514800>] ? efi_pstore_read_func.part.1+0x170/0x170
[ 1.144058] [<ffffffff815148b4>] efi_pstore_read_func+0xb4/0xe0
[ 1.144058] [<ffffffff81512b7b>] __efivar_entry_iter+0xfb/0x120
[ 1.144058] [<ffffffff8151428f>] efi_pstore_read+0x3f/0x50
[ 1.144058] [<ffffffff8128d7ba>] pstore_get_records+0x9a/0x150
[ 1.158207] [<ffffffff812af25c>] ? selinux_d_instantiate+0x1c/0x20
[ 1.158207] [<ffffffff8128ce30>] ? parse_options+0x80/0x80
[ 1.158207] [<ffffffff8128ced5>] pstore_fill_super+0xa5/0xc0
[ 1.158207] [<ffffffff811ae7d2>] mount_single+0xa2/0xd0
[ 1.158207] [<ffffffff8128ccf8>] pstore_mount+0x18/0x20
[ 1.158207] [<ffffffff811ae8b9>] mount_fs+0x39/0x1b0
[ 1.158207] [<ffffffff81160550>] ? __alloc_percpu+0x10/0x20
[ 1.158207] [<ffffffff811c9493>] vfs_kern_mount+0x63/0xf0
[ 1.158207] [<ffffffff811cbb0e>] do_mount+0x23e/0xa20
[ 1.158207] [<ffffffff8115b51b>] ? strndup_user+0x4b/0xf0
[ 1.158207] [<ffffffff811cc373>] SyS_mount+0x83/0xc0
[ 1.158207] [<ffffffff81673cc2>] system_call_fastpath+0x16/0x1b
[ 1.158207] ---[ end trace 61981bc62de9f6f4 ]---
Signed-off-by: Seiji Aguchi <seiji.aguchi@hds.com>
Tested-by: Madper Xie <cxie@redhat.com>
Cc: stable@kernel.org
Signed-off-by: Matt Fleming <matt.fleming@intel.com>
Diffstat (limited to 'drivers/firmware')
-rw-r--r-- | drivers/firmware/efi/efi-pstore.c | 143 | ||||
-rw-r--r-- | drivers/firmware/efi/efivars.c | 12 | ||||
-rw-r--r-- | drivers/firmware/efi/vars.c | 12 |
3 files changed, 151 insertions, 16 deletions
diff --git a/drivers/firmware/efi/efi-pstore.c b/drivers/firmware/efi/efi-pstore.c index 5002d50e3781..6ce31e93da55 100644 --- a/drivers/firmware/efi/efi-pstore.c +++ b/drivers/firmware/efi/efi-pstore.c | |||
@@ -18,14 +18,12 @@ module_param_named(pstore_disable, efivars_pstore_disable, bool, 0644); | |||
18 | 18 | ||
19 | static int efi_pstore_open(struct pstore_info *psi) | 19 | static int efi_pstore_open(struct pstore_info *psi) |
20 | { | 20 | { |
21 | efivar_entry_iter_begin(); | ||
22 | psi->data = NULL; | 21 | psi->data = NULL; |
23 | return 0; | 22 | return 0; |
24 | } | 23 | } |
25 | 24 | ||
26 | static int efi_pstore_close(struct pstore_info *psi) | 25 | static int efi_pstore_close(struct pstore_info *psi) |
27 | { | 26 | { |
28 | efivar_entry_iter_end(); | ||
29 | psi->data = NULL; | 27 | psi->data = NULL; |
30 | return 0; | 28 | return 0; |
31 | } | 29 | } |
@@ -91,19 +89,125 @@ static int efi_pstore_read_func(struct efivar_entry *entry, void *data) | |||
91 | __efivar_entry_get(entry, &entry->var.Attributes, | 89 | __efivar_entry_get(entry, &entry->var.Attributes, |
92 | &entry->var.DataSize, entry->var.Data); | 90 | &entry->var.DataSize, entry->var.Data); |
93 | size = entry->var.DataSize; | 91 | size = entry->var.DataSize; |
92 | memcpy(*cb_data->buf, entry->var.Data, | ||
93 | (size_t)min_t(unsigned long, EFIVARS_DATA_SIZE_MAX, size)); | ||
94 | 94 | ||
95 | *cb_data->buf = kmemdup(entry->var.Data, size, GFP_KERNEL); | ||
96 | if (*cb_data->buf == NULL) | ||
97 | return -ENOMEM; | ||
98 | return size; | 95 | return size; |
99 | } | 96 | } |
100 | 97 | ||
98 | /** | ||
99 | * efi_pstore_scan_sysfs_enter | ||
100 | * @entry: scanning entry | ||
101 | * @next: next entry | ||
102 | * @head: list head | ||
103 | */ | ||
104 | static void efi_pstore_scan_sysfs_enter(struct efivar_entry *pos, | ||
105 | struct efivar_entry *next, | ||
106 | struct list_head *head) | ||
107 | { | ||
108 | pos->scanning = true; | ||
109 | if (&next->list != head) | ||
110 | next->scanning = true; | ||
111 | } | ||
112 | |||
113 | /** | ||
114 | * __efi_pstore_scan_sysfs_exit | ||
115 | * @entry: deleting entry | ||
116 | * @turn_off_scanning: Check if a scanning flag should be turned off | ||
117 | */ | ||
118 | static inline void __efi_pstore_scan_sysfs_exit(struct efivar_entry *entry, | ||
119 | bool turn_off_scanning) | ||
120 | { | ||
121 | if (entry->deleting) { | ||
122 | list_del(&entry->list); | ||
123 | efivar_entry_iter_end(); | ||
124 | efivar_unregister(entry); | ||
125 | efivar_entry_iter_begin(); | ||
126 | } else if (turn_off_scanning) | ||
127 | entry->scanning = false; | ||
128 | } | ||
129 | |||
130 | /** | ||
131 | * efi_pstore_scan_sysfs_exit | ||
132 | * @pos: scanning entry | ||
133 | * @next: next entry | ||
134 | * @head: list head | ||
135 | * @stop: a flag checking if scanning will stop | ||
136 | */ | ||
137 | static void efi_pstore_scan_sysfs_exit(struct efivar_entry *pos, | ||
138 | struct efivar_entry *next, | ||
139 | struct list_head *head, bool stop) | ||
140 | { | ||
141 | __efi_pstore_scan_sysfs_exit(pos, true); | ||
142 | if (stop) | ||
143 | __efi_pstore_scan_sysfs_exit(next, &next->list != head); | ||
144 | } | ||
145 | |||
146 | /** | ||
147 | * efi_pstore_sysfs_entry_iter | ||
148 | * | ||
149 | * @data: function-specific data to pass to callback | ||
150 | * @pos: entry to begin iterating from | ||
151 | * | ||
152 | * You MUST call efivar_enter_iter_begin() before this function, and | ||
153 | * efivar_entry_iter_end() afterwards. | ||
154 | * | ||
155 | * It is possible to begin iteration from an arbitrary entry within | ||
156 | * the list by passing @pos. @pos is updated on return to point to | ||
157 | * the next entry of the last one passed to efi_pstore_read_func(). | ||
158 | * To begin iterating from the beginning of the list @pos must be %NULL. | ||
159 | */ | ||
160 | static int efi_pstore_sysfs_entry_iter(void *data, struct efivar_entry **pos) | ||
161 | { | ||
162 | struct efivar_entry *entry, *n; | ||
163 | struct list_head *head = &efivar_sysfs_list; | ||
164 | int size = 0; | ||
165 | |||
166 | if (!*pos) { | ||
167 | list_for_each_entry_safe(entry, n, head, list) { | ||
168 | efi_pstore_scan_sysfs_enter(entry, n, head); | ||
169 | |||
170 | size = efi_pstore_read_func(entry, data); | ||
171 | efi_pstore_scan_sysfs_exit(entry, n, head, size < 0); | ||
172 | if (size) | ||
173 | break; | ||
174 | } | ||
175 | *pos = n; | ||
176 | return size; | ||
177 | } | ||
178 | |||
179 | list_for_each_entry_safe_from((*pos), n, head, list) { | ||
180 | efi_pstore_scan_sysfs_enter((*pos), n, head); | ||
181 | |||
182 | size = efi_pstore_read_func((*pos), data); | ||
183 | efi_pstore_scan_sysfs_exit((*pos), n, head, size < 0); | ||
184 | if (size) | ||
185 | break; | ||
186 | } | ||
187 | *pos = n; | ||
188 | return size; | ||
189 | } | ||
190 | |||
191 | /** | ||
192 | * efi_pstore_read | ||
193 | * | ||
194 | * This function returns a size of NVRAM entry logged via efi_pstore_write(). | ||
195 | * The meaning and behavior of efi_pstore/pstore are as below. | ||
196 | * | ||
197 | * size > 0: Got data of an entry logged via efi_pstore_write() successfully, | ||
198 | * and pstore filesystem will continue reading subsequent entries. | ||
199 | * size == 0: Entry was not logged via efi_pstore_write(), | ||
200 | * and efi_pstore driver will continue reading subsequent entries. | ||
201 | * size < 0: Failed to get data of entry logging via efi_pstore_write(), | ||
202 | * and pstore will stop reading entry. | ||
203 | */ | ||
101 | static ssize_t efi_pstore_read(u64 *id, enum pstore_type_id *type, | 204 | static ssize_t efi_pstore_read(u64 *id, enum pstore_type_id *type, |
102 | int *count, struct timespec *timespec, | 205 | int *count, struct timespec *timespec, |
103 | char **buf, bool *compressed, | 206 | char **buf, bool *compressed, |
104 | struct pstore_info *psi) | 207 | struct pstore_info *psi) |
105 | { | 208 | { |
106 | struct pstore_read_data data; | 209 | struct pstore_read_data data; |
210 | ssize_t size; | ||
107 | 211 | ||
108 | data.id = id; | 212 | data.id = id; |
109 | data.type = type; | 213 | data.type = type; |
@@ -112,8 +216,17 @@ static ssize_t efi_pstore_read(u64 *id, enum pstore_type_id *type, | |||
112 | data.compressed = compressed; | 216 | data.compressed = compressed; |
113 | data.buf = buf; | 217 | data.buf = buf; |
114 | 218 | ||
115 | return __efivar_entry_iter(efi_pstore_read_func, &efivar_sysfs_list, &data, | 219 | *data.buf = kzalloc(EFIVARS_DATA_SIZE_MAX, GFP_KERNEL); |
116 | (struct efivar_entry **)&psi->data); | 220 | if (!*data.buf) |
221 | return -ENOMEM; | ||
222 | |||
223 | efivar_entry_iter_begin(); | ||
224 | size = efi_pstore_sysfs_entry_iter(&data, | ||
225 | (struct efivar_entry **)&psi->data); | ||
226 | efivar_entry_iter_end(); | ||
227 | if (size <= 0) | ||
228 | kfree(*data.buf); | ||
229 | return size; | ||
117 | } | 230 | } |
118 | 231 | ||
119 | static int efi_pstore_write(enum pstore_type_id type, | 232 | static int efi_pstore_write(enum pstore_type_id type, |
@@ -184,9 +297,17 @@ static int efi_pstore_erase_func(struct efivar_entry *entry, void *data) | |||
184 | return 0; | 297 | return 0; |
185 | } | 298 | } |
186 | 299 | ||
300 | if (entry->scanning) { | ||
301 | /* | ||
302 | * Skip deletion because this entry will be deleted | ||
303 | * after scanning is completed. | ||
304 | */ | ||
305 | entry->deleting = true; | ||
306 | } else | ||
307 | list_del(&entry->list); | ||
308 | |||
187 | /* found */ | 309 | /* found */ |
188 | __efivar_entry_delete(entry); | 310 | __efivar_entry_delete(entry); |
189 | list_del(&entry->list); | ||
190 | 311 | ||
191 | return 1; | 312 | return 1; |
192 | } | 313 | } |
@@ -214,10 +335,12 @@ static int efi_pstore_erase(enum pstore_type_id type, u64 id, int count, | |||
214 | 335 | ||
215 | efivar_entry_iter_begin(); | 336 | efivar_entry_iter_begin(); |
216 | found = __efivar_entry_iter(efi_pstore_erase_func, &efivar_sysfs_list, &edata, &entry); | 337 | found = __efivar_entry_iter(efi_pstore_erase_func, &efivar_sysfs_list, &edata, &entry); |
217 | efivar_entry_iter_end(); | ||
218 | 338 | ||
219 | if (found) | 339 | if (found && !entry->scanning) { |
340 | efivar_entry_iter_end(); | ||
220 | efivar_unregister(entry); | 341 | efivar_unregister(entry); |
342 | } else | ||
343 | efivar_entry_iter_end(); | ||
221 | 344 | ||
222 | return 0; | 345 | return 0; |
223 | } | 346 | } |
diff --git a/drivers/firmware/efi/efivars.c b/drivers/firmware/efi/efivars.c index 933eb027d527..3dc248239197 100644 --- a/drivers/firmware/efi/efivars.c +++ b/drivers/firmware/efi/efivars.c | |||
@@ -383,12 +383,16 @@ static ssize_t efivar_delete(struct file *filp, struct kobject *kobj, | |||
383 | else if (__efivar_entry_delete(entry)) | 383 | else if (__efivar_entry_delete(entry)) |
384 | err = -EIO; | 384 | err = -EIO; |
385 | 385 | ||
386 | efivar_entry_iter_end(); | 386 | if (err) { |
387 | 387 | efivar_entry_iter_end(); | |
388 | if (err) | ||
389 | return err; | 388 | return err; |
389 | } | ||
390 | 390 | ||
391 | efivar_unregister(entry); | 391 | if (!entry->scanning) { |
392 | efivar_entry_iter_end(); | ||
393 | efivar_unregister(entry); | ||
394 | } else | ||
395 | efivar_entry_iter_end(); | ||
392 | 396 | ||
393 | /* It's dead Jim.... */ | 397 | /* It's dead Jim.... */ |
394 | return count; | 398 | return count; |
diff --git a/drivers/firmware/efi/vars.c b/drivers/firmware/efi/vars.c index 391c67b182d9..b22659cccca4 100644 --- a/drivers/firmware/efi/vars.c +++ b/drivers/firmware/efi/vars.c | |||
@@ -683,8 +683,16 @@ struct efivar_entry *efivar_entry_find(efi_char16_t *name, efi_guid_t guid, | |||
683 | if (!found) | 683 | if (!found) |
684 | return NULL; | 684 | return NULL; |
685 | 685 | ||
686 | if (remove) | 686 | if (remove) { |
687 | list_del(&entry->list); | 687 | if (entry->scanning) { |
688 | /* | ||
689 | * The entry will be deleted | ||
690 | * after scanning is completed. | ||
691 | */ | ||
692 | entry->deleting = true; | ||
693 | } else | ||
694 | list_del(&entry->list); | ||
695 | } | ||
688 | 696 | ||
689 | return entry; | 697 | return entry; |
690 | } | 698 | } |