Skip to content

Commit cf51bec

Browse files
riteshharjanigregkh
authored andcommitted
pseries/papr-hvpipe: Fix null ptr deref in papr_hvpipe_dev_create_handle()
commit 1b9f7aa upstream. commit 6d3789d ("papr-hvpipe: convert papr_hvpipe_dev_create_handle() to FD_PREPARE()"), changed the create handle to FD_PREPARE(), but it caused kernel null-ptr-deref because after call to retain_and_null_ptr(src_info), src_info is re-used for adding it to the global list. Getting the following kernel panic in papr_hvpipe_dev_create_handle() when trying to add src_info to the list. Kernel attempted to write user page (0) - exploit attempt? (uid: 0) BUG: Kernel NULL pointer dereference on write at 0x00000000 Faulting instruction address: 0xc0000000001b44a0 Oops: Kernel access of bad area, sig: 11 [#1] ... Call Trace: papr_hvpipe_dev_ioctl+0x1f4/0x48c (unreliable) sys_ioctl+0x528/0x1064 system_call_exception+0x128/0x360 system_call_vectored_common+0x15c/0x2ec Now, the error handling with FD_PREPARE's file cleanup and __free(kfree) auto cleanup is getting too convoluted. This is mainly because we need to ensure only 1 user get the srcID handle. To simplify this, we allocate prepare the src_info in the beginning and add it to the global list under a spinlock after checking that no duplicates exist. This simplify the error handling where if the FD_ADD fails, we can simply remove the src_info from the list and consume any pending msg in hvpipe to be cleared, after src_info became visible in the global list. Cc: stable@vger.kernel.org Fixes: 6d3789d ("papr-hvpipe: convert papr_hvpipe_dev_create_handle() to FD_PREPARE()") Reported-by: Haren Myneni <haren@linux.ibm.com> Signed-off-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com> Signed-off-by: Madhavan Srinivasan <maddy@linux.ibm.com> Link: https://patch.msgid.link/31ad94bc89d44156ee700c5bd006cb47a748e3cb.1777606826.git.ritesh.list@gmail.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent f88f8e4 commit cf51bec

1 file changed

Lines changed: 30 additions & 27 deletions

File tree

arch/powerpc/platforms/pseries/papr-hvpipe.c

Lines changed: 30 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -480,23 +480,10 @@ static const struct file_operations papr_hvpipe_handle_ops = {
480480

481481
static int papr_hvpipe_dev_create_handle(u32 srcID)
482482
{
483-
struct hvpipe_source_info *src_info __free(kfree) = NULL;
483+
struct hvpipe_source_info *src_info;
484+
int fd;
484485
unsigned long flags;
485486

486-
spin_lock_irqsave(&hvpipe_src_list_lock, flags);
487-
/*
488-
* Do not allow more than one process communicates with
489-
* each source.
490-
*/
491-
src_info = hvpipe_find_source(srcID);
492-
if (src_info) {
493-
spin_unlock_irqrestore(&hvpipe_src_list_lock, flags);
494-
pr_err("pid(%d) is already using the source(%d)\n",
495-
src_info->tsk->pid, srcID);
496-
return -EALREADY;
497-
}
498-
spin_unlock_irqrestore(&hvpipe_src_list_lock, flags);
499-
500487
src_info = kzalloc_obj(*src_info, GFP_KERNEL_ACCOUNT);
501488
if (!src_info)
502489
return -ENOMEM;
@@ -505,26 +492,42 @@ static int papr_hvpipe_dev_create_handle(u32 srcID)
505492
src_info->tsk = current;
506493
init_waitqueue_head(&src_info->recv_wqh);
507494

508-
FD_PREPARE(fdf, O_RDONLY | O_CLOEXEC,
509-
anon_inode_getfile("[papr-hvpipe]", &papr_hvpipe_handle_ops,
510-
(void *)src_info, O_RDWR));
511-
if (fdf.err)
512-
return fdf.err;
513-
514-
retain_and_null_ptr(src_info);
515-
spin_lock_irqsave(&hvpipe_src_list_lock, flags);
516495
/*
517-
* If two processes are executing ioctl() for the same
518-
* source ID concurrently, prevent the second process to
519-
* acquire FD.
496+
* Do not allow more than one process communicates with
497+
* each source.
520498
*/
499+
spin_lock_irqsave(&hvpipe_src_list_lock, flags);
521500
if (hvpipe_find_source(srcID)) {
522501
spin_unlock_irqrestore(&hvpipe_src_list_lock, flags);
502+
pr_err("pid(%d) could not get the source(%d)\n",
503+
src_info->tsk->pid, srcID);
504+
kfree(src_info);
523505
return -EALREADY;
524506
}
525507
list_add(&src_info->list, &hvpipe_src_list);
526508
spin_unlock_irqrestore(&hvpipe_src_list_lock, flags);
527-
return fd_publish(fdf);
509+
510+
fd = FD_ADD(O_RDONLY | O_CLOEXEC,
511+
anon_inode_getfile("[papr-hvpipe]", &papr_hvpipe_handle_ops,
512+
(void *)src_info, O_RDWR));
513+
if (fd < 0) {
514+
spin_lock_irqsave(&hvpipe_src_list_lock, flags);
515+
list_del(&src_info->list);
516+
spin_unlock_irqrestore(&hvpipe_src_list_lock, flags);
517+
/*
518+
* if we fail to add FD, that means no userspace program is
519+
* polling. In that case if there is a msg pending because the
520+
* interrupt was fired after the src_info was added to the
521+
* global list, then let's consume it here, to unblock the
522+
* hvpipe
523+
*/
524+
if (src_info->hvpipe_status & HVPIPE_MSG_AVAILABLE)
525+
hvpipe_rtas_recv_msg(NULL, 0);
526+
kfree(src_info);
527+
return fd;
528+
}
529+
530+
return fd;
528531
}
529532

530533
/*

0 commit comments

Comments
 (0)