Skip to content

Commit aea948b

Browse files
maheshsalmpe
authored andcommitted
powerpc/powernv/elog: Fix race while processing OPAL error log event.
Every error log reported by OPAL is exported to userspace through a sysfs interface and notified using kobject_uevent(). The userspace daemon (opal_errd) then reads the error log and acknowledges the error log is saved safely to disk. Once acknowledged the kernel removes the respective sysfs file entry causing respective resources to be released including kobject. However it's possible the userspace daemon may already be scanning elog entries when a new sysfs elog entry is created by the kernel. User daemon may read this new entry and ack it even before kernel can notify userspace about it through kobject_uevent() call. If that happens then we have a potential race between elog_ack_store->kobject_put() and kobject_uevent which can lead to use-after-free of a kernfs object resulting in a kernel crash. eg: BUG: Unable to handle kernel data access on read at 0x6b6b6b6b6b6b6bfb Faulting instruction address: 0xc0000000008ff2a0 Oops: Kernel access of bad area, sig: 11 [#1] LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA PowerNV CPU: 27 PID: 805 Comm: irq/29-opal-elo Not tainted 5.9.0-rc2-gcc-8.2.0-00214-g6f56a67bcbb5-dirty #363 ... NIP kobject_uevent_env+0xa0/0x910 LR elog_event+0x1f4/0x2d0 Call Trace: 0x5deadbeef0000122 (unreliable) elog_event+0x1f4/0x2d0 irq_thread_fn+0x4c/0xc0 irq_thread+0x1c0/0x2b0 kthread+0x1c4/0x1d0 ret_from_kernel_thread+0x5c/0x6c This patch fixes this race by protecting the sysfs file creation/notification by holding a reference count on kobject until we safely send kobject_uevent(). The function create_elog_obj() returns the elog object which if used by caller function will end up in use-after-free problem again. However, the return value of create_elog_obj() function isn't being used today and there is no need as well. Hence change it to return void to make this fix complete. Fixes: 774fea1 ("powerpc/powernv: Read OPAL error log and export it through sysfs") Cc: stable@vger.kernel.org # v3.15+ Reported-by: Oliver O'Halloran <oohall@gmail.com> Signed-off-by: Mahesh Salgaonkar <mahesh@linux.ibm.com> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com> Reviewed-by: Oliver O'Halloran <oohall@gmail.com> Reviewed-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com> [mpe: Rework the logic to use a single return, reword comments, add oops] Signed-off-by: Michael Ellerman <mpe@ellerman.id.au> Link: https://lore.kernel.org/r/20201006122051.190176-1-mpe@ellerman.id.au
1 parent ebbfeef commit aea948b

File tree

1 file changed

+26
-7
lines changed

1 file changed

+26
-7
lines changed

Diff for: arch/powerpc/platforms/powernv/opal-elog.c

+26-7
Original file line numberDiff line numberDiff line change
@@ -179,14 +179,14 @@ static ssize_t raw_attr_read(struct file *filep, struct kobject *kobj,
179179
return count;
180180
}
181181

182-
static struct elog_obj *create_elog_obj(uint64_t id, size_t size, uint64_t type)
182+
static void create_elog_obj(uint64_t id, size_t size, uint64_t type)
183183
{
184184
struct elog_obj *elog;
185185
int rc;
186186

187187
elog = kzalloc(sizeof(*elog), GFP_KERNEL);
188188
if (!elog)
189-
return NULL;
189+
return;
190190

191191
elog->kobj.kset = elog_kset;
192192

@@ -219,18 +219,37 @@ static struct elog_obj *create_elog_obj(uint64_t id, size_t size, uint64_t type)
219219
rc = kobject_add(&elog->kobj, NULL, "0x%llx", id);
220220
if (rc) {
221221
kobject_put(&elog->kobj);
222-
return NULL;
222+
return;
223223
}
224224

225+
/*
226+
* As soon as the sysfs file for this elog is created/activated there is
227+
* a chance the opal_errd daemon (or any userspace) might read and
228+
* acknowledge the elog before kobject_uevent() is called. If that
229+
* happens then there is a potential race between
230+
* elog_ack_store->kobject_put() and kobject_uevent() which leads to a
231+
* use-after-free of a kernfs object resulting in a kernel crash.
232+
*
233+
* To avoid that, we need to take a reference on behalf of the bin file,
234+
* so that our reference remains valid while we call kobject_uevent().
235+
* We then drop our reference before exiting the function, leaving the
236+
* bin file to drop the last reference (if it hasn't already).
237+
*/
238+
239+
/* Take a reference for the bin file */
240+
kobject_get(&elog->kobj);
225241
rc = sysfs_create_bin_file(&elog->kobj, &elog->raw_attr);
226-
if (rc) {
242+
if (rc == 0) {
243+
kobject_uevent(&elog->kobj, KOBJ_ADD);
244+
} else {
245+
/* Drop the reference taken for the bin file */
227246
kobject_put(&elog->kobj);
228-
return NULL;
229247
}
230248

231-
kobject_uevent(&elog->kobj, KOBJ_ADD);
249+
/* Drop our reference */
250+
kobject_put(&elog->kobj);
232251

233-
return elog;
252+
return;
234253
}
235254

236255
static irqreturn_t elog_event(int irq, void *data)

0 commit comments

Comments
 (0)