[RFC v1 0/6] pseries/papr-hvpipe: Fix and simplify papr-hvpipe

[RFC v1 0/6] pseries/papr-hvpipe: Fix and simplify papr-hvpipe

[Ritesh Harjani (IBM)]

Haren reported a UAF / null ptr deref issue here [1]. While reviewing that and
going over papr-hvpipe code, I found couple of more issues around the usage of
copy_to_user() and few refactoring which simplifies the code.

This patch series is an attempt to that. Note that this is only compile tested
on pseries for now.

Haren, I will kindly need your help in verifying this please. Let me know if we
have a selftests or any other test framework for this, which I can utilize too.

[1]: https://lore.kernel.org/linuxppc-dev/20260317040444.2785741-1-haren@linux.ibm.com/


Ritesh Harjani (IBM) (6):
  pseries/papr-hvpipe: Fix null ptr deref in papr_hvpipe_dev_create_handle
  pseries/papr-hvpipe: Fix the usage of copy_to_user()
  pseries/papr-hvpipe: Simplify spin_unlock() usage in papr_hvpipe_handle_release
  pseries/papr-hvpipe: Kill task_struct pointer from struct hvpipe_source_info
  pseries/papr-hvpipe: Refactor and simplify hvpipe_rtas_recv_msg()
  pseries/papr-hvpipe: Simplify error handling in papr_hvpipe_init()

 arch/powerpc/platforms/pseries/papr-hvpipe.c | 135 +++++++++----------
 arch/powerpc/platforms/pseries/papr-hvpipe.h |   1 -
 2 files changed, 66 insertions(+), 70 deletions(-)

--
2.39.5

[RFC v1 1/6] pseries/papr-hvpipe: Fix null ptr deref in papr_hvpipe_dev_create_handle

[Ritesh Harjani (IBM)]

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

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.

Cc: Christian Brauner <brauner@kernel.org>
Fixes: 6d3789d347a7 ("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>
---
 arch/powerpc/platforms/pseries/papr-hvpipe.c | 50 +++++++++-----------
 1 file changed, 22 insertions(+), 28 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/papr-hvpipe.c b/arch/powerpc/platforms/pseries/papr-hvpipe.c
index 14ae480d060a..ef10f5a5a4fa 100644
--- a/arch/powerpc/platforms/pseries/papr-hvpipe.c
+++ b/arch/powerpc/platforms/pseries/papr-hvpipe.c
@@ -479,21 +479,8 @@ static const struct file_operations papr_hvpipe_handle_ops = {

 static int papr_hvpipe_dev_create_handle(u32 srcID)
 {
-	struct hvpipe_source_info *src_info __free(kfree) = NULL;
-
-	spin_lock(&hvpipe_src_list_lock);
-	/*
-	 * Do not allow more than one process communicates with
-	 * each source.
-	 */
-	src_info = hvpipe_find_source(srcID);
-	if (src_info) {
-		spin_unlock(&hvpipe_src_list_lock);
-		pr_err("pid(%d) is already using the source(%d)\n",
-				src_info->tsk->pid, srcID);
-		return -EALREADY;
-	}
-	spin_unlock(&hvpipe_src_list_lock);
+	struct hvpipe_source_info *src_info;
+	int fd;

 	src_info = kzalloc_obj(*src_info, GFP_KERNEL_ACCOUNT);
 	if (!src_info)
@@ -503,26 +490,33 @@ static int papr_hvpipe_dev_create_handle(u32 srcID)
 	src_info->tsk = current;
 	init_waitqueue_head(&src_info->recv_wqh);

-	FD_PREPARE(fdf, O_RDONLY | O_CLOEXEC,
-		   anon_inode_getfile("[papr-hvpipe]", &papr_hvpipe_handle_ops,
-				      (void *)src_info, O_RDWR));
-	if (fdf.err)
-		return fdf.err;
-
-	retain_and_null_ptr(src_info);
-	spin_lock(&hvpipe_src_list_lock);
 	/*
-	 * If two processes are executing ioctl() for the same
-	 * source ID concurrently, prevent the second process to
-	 * acquire FD.
+	 * Do not allow more than one process communicates with
+	 * each source.
 	 */
-	if (hvpipe_find_source(srcID)) {
+	spin_lock(&hvpipe_src_list_lock);
+	if(hvpipe_find_source(srcID)) {
 		spin_unlock(&hvpipe_src_list_lock);
+		pr_err("pid(%d) could not get the source(%d)\n",
+				src_info->tsk->pid, srcID);
+		kfree(src_info);
 		return -EALREADY;
 	}
 	list_add(&src_info->list, &hvpipe_src_list);
 	spin_unlock(&hvpipe_src_list_lock);
-	return fd_publish(fdf);
+
+	fd = FD_ADD(O_RDONLY | O_CLOEXEC,
+		   anon_inode_getfile("[papr-hvpipe]", &papr_hvpipe_handle_ops,
+				      (void *)src_info, O_RDWR));
+	if (fd < 0) {
+		spin_lock(&hvpipe_src_list_lock);
+		list_del(&src_info->list);
+		spin_unlock(&hvpipe_src_list_lock);
+		kfree(src_info);
+		return fd;
+	}
+
+	return fd;
 }

 /*
--
2.39.5

[RFC v1 2/6] pseries/papr-hvpipe: Fix the usage of copy_to_user()

[Ritesh Harjani (IBM)]

copy_to_user() return bytes_not_copied to the user buffer. If there was
an error writing bytes into the user buffer, i.e. if copy_to_user
returns a non-zero value, then we simply return -EFAULT from the
->read() call.

Signed-off-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
---
 arch/powerpc/platforms/pseries/papr-hvpipe.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/papr-hvpipe.c b/arch/powerpc/platforms/pseries/papr-hvpipe.c
index ef10f5a5a4fa..fffebbae113c 100644
--- a/arch/powerpc/platforms/pseries/papr-hvpipe.c
+++ b/arch/powerpc/platforms/pseries/papr-hvpipe.c
@@ -206,10 +206,11 @@ static int hvpipe_rtas_recv_msg(char __user *buf, int size)
 					bytes_written, size);
 				bytes_written = size;
 			}
-			ret = copy_to_user(buf,
+			if (copy_to_user(buf,
 					rtas_work_area_raw_buf(work_area),
-					bytes_written);
-			if (!ret)
+					bytes_written))
+				ret = -EFAULT;
+			else
 				ret = bytes_written;
 		}
 	} else {
@@ -376,7 +377,7 @@ static ssize_t papr_hvpipe_handle_read(struct file *file,
 
 	ret = copy_to_user(buf, &hdr, HVPIPE_HDR_LEN);
 	if (ret)
-		return ret;
+		return -EFAULT;
 
 	/*
 	 * Message event has payload, so get the payload with
-- 
2.39.5

[RFC v1 3/6] pseries/papr-hvpipe: Simplify spin_unlock() usage in papr_hvpipe_handle_release

[Ritesh Harjani (IBM)]

Once the src_info is removed from the global list, no one can access it.
This simplies the usage of spin_unlock() in papr_hvpipe_handle_release()

Signed-off-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
---
 arch/powerpc/platforms/pseries/papr-hvpipe.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/papr-hvpipe.c b/arch/powerpc/platforms/pseries/papr-hvpipe.c
index fffebbae113c..57ec50e3e883 100644
--- a/arch/powerpc/platforms/pseries/papr-hvpipe.c
+++ b/arch/powerpc/platforms/pseries/papr-hvpipe.c
@@ -455,6 +455,7 @@ static int papr_hvpipe_handle_release(struct inode *inode,
 	src_info = file->private_data;
 	list_del(&src_info->list);
 	file->private_data = NULL;
+	spin_unlock(&hvpipe_src_list_lock);
 	/*
 	 * If the pipe for this specific source has any pending
 	 * payload, issue recv HVPIPE RTAS so that pipe will not
@@ -462,10 +463,8 @@ static int papr_hvpipe_handle_release(struct inode *inode,
 	 */
 	if (src_info->hvpipe_status & HVPIPE_MSG_AVAILABLE) {
 		src_info->hvpipe_status = 0;
-		spin_unlock(&hvpipe_src_list_lock);
 		hvpipe_rtas_recv_msg(NULL, 0);
-	} else
-		spin_unlock(&hvpipe_src_list_lock);
+	}
 
 	kfree(src_info);
 	return 0;
-- 
2.39.5

[RFC v1 4/6] pseries/papr-hvpipe: Kill task_struct pointer from struct hvpipe_source_info

[Ritesh Harjani (IBM)]

We don't really use task_struct pointer for anything meaningful. So just
kill it for now, and we can bring back later if we need this for any
future debug purposes.

Signed-off-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
---
 arch/powerpc/platforms/pseries/papr-hvpipe.c | 5 ++---
 arch/powerpc/platforms/pseries/papr-hvpipe.h | 1 -
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/papr-hvpipe.c b/arch/powerpc/platforms/pseries/papr-hvpipe.c
index 57ec50e3e883..7242a14eb5ec 100644
--- a/arch/powerpc/platforms/pseries/papr-hvpipe.c
+++ b/arch/powerpc/platforms/pseries/papr-hvpipe.c
@@ -487,7 +487,6 @@ static int papr_hvpipe_dev_create_handle(u32 srcID)
 		return -ENOMEM;
 
 	src_info->srcID = srcID;
-	src_info->tsk = current;
 	init_waitqueue_head(&src_info->recv_wqh);
 
 	/*
@@ -497,8 +496,8 @@ static int papr_hvpipe_dev_create_handle(u32 srcID)
 	spin_lock(&hvpipe_src_list_lock);
 	if(hvpipe_find_source(srcID)) {
 		spin_unlock(&hvpipe_src_list_lock);
-		pr_err("pid(%d) could not get the source(%d)\n",
-				src_info->tsk->pid, srcID);
+		pr_err("pid(%s:%d) could not get the source(%d)\n",
+				current->comm, task_pid_nr(current), srcID);
 		kfree(src_info);
 		return -EALREADY;
 	}
diff --git a/arch/powerpc/platforms/pseries/papr-hvpipe.h b/arch/powerpc/platforms/pseries/papr-hvpipe.h
index c343f4230865..4bdf7bb2fc4d 100644
--- a/arch/powerpc/platforms/pseries/papr-hvpipe.h
+++ b/arch/powerpc/platforms/pseries/papr-hvpipe.h
@@ -21,7 +21,6 @@ struct hvpipe_source_info {
 	u32 srcID;
 	u32 hvpipe_status;
 	wait_queue_head_t recv_wqh;	 /* wake up poll() waitq */
-	struct task_struct *tsk;
 };
 
 /*
-- 
2.39.5

[RFC v1 5/6] pseries/papr-hvpipe: Refactor and simplify hvpipe_rtas_recv_msg()

[Ritesh Harjani (IBM)]

Simplify hvpipe_rtas_recv_msg() by removing three levels of nesting...
if (!ret)
    if (buf)
	if (size < bytes_written)
... this refactoring of the funcion ensures bail out routines are
handled early, to simplify the function logic.

Signed-off-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
---
 arch/powerpc/platforms/pseries/papr-hvpipe.c | 52 ++++++++++----------
 1 file changed, 26 insertions(+), 26 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/papr-hvpipe.c b/arch/powerpc/platforms/pseries/papr-hvpipe.c
index 7242a14eb5ec..f41f5c0a8418 100644
--- a/arch/powerpc/platforms/pseries/papr-hvpipe.c
+++ b/arch/powerpc/platforms/pseries/papr-hvpipe.c
@@ -190,34 +190,34 @@ static int hvpipe_rtas_recv_msg(char __user *buf, int size)
 		return -ENOMEM;
 	}
 
-	ret = rtas_ibm_receive_hvpipe_msg(work_area, &srcID,
-					&bytes_written);
-	if (!ret) {
-		/*
-		 * Recv HVPIPE RTAS is successful.
-		 * When releasing FD or no one is waiting on the
-		 * specific source, issue recv HVPIPE RTAS call
-		 * so that pipe is not blocked - this func is called
-		 * with NULL buf.
-		 */
-		if (buf) {
-			if (size < bytes_written) {
-				pr_err("Received the payload size = %d, but the buffer size = %d\n",
-					bytes_written, size);
-				bytes_written = size;
-			}
-			if (copy_to_user(buf,
-					rtas_work_area_raw_buf(work_area),
-					bytes_written))
-				ret = -EFAULT;
-			else
-				ret = bytes_written;
-		}
-	} else {
-		pr_err("ibm,receive-hvpipe-msg failed with %d\n",
-				ret);
+	/*
+	 * Recv HVPIPE RTAS is successful.
+	 * When releasing FD or no one is waiting on the
+	 * specific source, issue recv HVPIPE RTAS call
+	 * so that pipe is not blocked - this func is called
+	 * with NULL buf.
+	 */
+	ret = rtas_ibm_receive_hvpipe_msg(work_area, &srcID, &bytes_written);
+	if (ret) {
+		pr_err("ibm,receive-hvpipe-msg failed with %d\n", ret);
+		goto out;
 	}
 
+	if (!buf)
+		goto out;
+
+	if (size < bytes_written) {
+		pr_err("Received the payload size = %d, but the buffer size = %d\n",
+				bytes_written, size);
+		bytes_written = size;
+	}
+
+	if (copy_to_user(buf, rtas_work_area_raw_buf(work_area), bytes_written))
+		ret = -EFAULT;
+	else
+		ret = bytes_written;
+
+out:
 	rtas_work_area_free(work_area);
 	return ret;
 }
-- 
2.39.5

[RFC v1 6/6] pseries/papr-hvpipe: Simplify error handling in papr_hvpipe_init()

[Ritesh Harjani (IBM)]

Remove such 3 levels of nesting patterns to check success return values
from function calls.

ret = enable_hvpipe_IRQ()
    if (!ret)
	    ret = set_hvpipe_sys_param(1)
	        if (!ret)
		    ret = misc_register()

Instead just bail out to "out*:" labels, in case of any error. This
simplifies the init flow.

Signed-off-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
---
 arch/powerpc/platforms/pseries/papr-hvpipe.c | 26 +++++++++++---------
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/papr-hvpipe.c b/arch/powerpc/platforms/pseries/papr-hvpipe.c
index f41f5c0a8418..03ec004a6701 100644
--- a/arch/powerpc/platforms/pseries/papr-hvpipe.c
+++ b/arch/powerpc/platforms/pseries/papr-hvpipe.c
@@ -768,23 +768,27 @@ static int __init papr_hvpipe_init(void)
 	}

 	ret = enable_hvpipe_IRQ();
-	if (!ret) {
-		ret = set_hvpipe_sys_param(1);
-		if (!ret)
-			ret = misc_register(&papr_hvpipe_dev);
-	}
+	if (ret)
+		goto out_wq;

-	if (!ret) {
-		pr_info("hvpipe feature is enabled\n");
-		hvpipe_feature = true;
-		return 0;
-	}
+	ret = set_hvpipe_sys_param(1);
+	if (ret)
+		goto out_wq;

-	pr_err("hvpipe feature is not enabled %d\n", ret);
+	ret = misc_register(&papr_hvpipe_dev);
+	if (ret)
+		goto out_wq;
+
+	pr_info("hvpipe feature is enabled\n");
+	hvpipe_feature = true;
+	return 0;
+
+out_wq:
 	destroy_workqueue(papr_hvpipe_wq);
 out:
 	kfree(papr_hvpipe_work);
 	papr_hvpipe_work = NULL;
+	pr_err("hvpipe feature is not enabled %d\n", ret);
 	return ret;
 }
 machine_device_initcall(pseries, papr_hvpipe_init);
--
2.39.5

Re: [RFC v1 0/6] pseries/papr-hvpipe: Fix and simplify papr-hvpipe

[Haren Myneni]

On Tue, 2026-04-07 at 20:01 +0530, Ritesh Harjani (IBM) wrote:
> Haren reported a UAF / null ptr deref issue here [1]. While reviewing
> that and
> going over papr-hvpipe code, I found couple of more issues around the
> usage of
> copy_to_user() and few refactoring which simplifies the code.
> 
> This patch series is an attempt to that. Note that this is only
> compile tested
> on pseries for now.
> 
> Haren, I will kindly need your help in verifying this please. Let me
> know if we
> have a selftests or any other test framework for this, which I can
> utilize too.

Thanks for fixes and cleanup the code. The testing should be part of
HMC/RCST framework. So we do not have selftests. I will work with HMC
to verify these patches. 

> 
> [1]:
> https://lore.kernel.org/linuxppc-dev/20260317040444.2785741-1-haren@linux.ibm.com/
> 
> 
> Ritesh Harjani (IBM) (6):
>   pseries/papr-hvpipe: Fix null ptr deref in
> papr_hvpipe_dev_create_handle
>   pseries/papr-hvpipe: Fix the usage of copy_to_user()
>   pseries/papr-hvpipe: Simplify spin_unlock() usage in
> papr_hvpipe_handle_release
>   pseries/papr-hvpipe: Kill task_struct pointer from struct
> hvpipe_source_info
>   pseries/papr-hvpipe: Refactor and simplify hvpipe_rtas_recv_msg()
>   pseries/papr-hvpipe: Simplify error handling in papr_hvpipe_init()
> 
>  arch/powerpc/platforms/pseries/papr-hvpipe.c | 135 +++++++++--------
> --
>  arch/powerpc/platforms/pseries/papr-hvpipe.h |   1 -
>  2 files changed, 66 insertions(+), 70 deletions(-)
> 
> --
> 2.39.5