[PATCH 1/2] rsxx: Fix potential NULL dereference setting up debugfs

[PATCH 1/2] rsxx: Fix potential NULL dereference setting up debugfs

[Dan Carpenter]

The "card->gendisk" pointer is allocated in rsxx_setup_dev() but there
is a module option "enable_blkdev" which lets people disable the block
device.  In that situation the "card->gendisk" pointer is NULL and it
would lead to a NULL dereference here.

Fixes: 36f988e978f8 ("rsxx: Adding in debugfs entries.")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
This patch is from static analysis.  The patch is obviously harmless.
So far as I can tell, the bug is real.  But maybe a different solution
is prefered?

 drivers/block/rsxx/core.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/block/rsxx/core.c b/drivers/block/rsxx/core.c
index 10f6368117d81..6207449fa716f 100644
--- a/drivers/block/rsxx/core.c
+++ b/drivers/block/rsxx/core.c
@@ -228,6 +228,9 @@ static void rsxx_debugfs_dev_new(struct rsxx_cardinfo *card)
 	struct dentry *debugfs_pci_regs;
 	struct dentry *debugfs_cram;
 
+	if (!card->gendisk)
+		return;
+
 	card->debugfs_dir = debugfs_create_dir(card->gendisk->disk_name, NULL);
 	if (IS_ERR_OR_NULL(card->debugfs_dir))
 		goto failed_debugfs_dir;
-- 
2.26.2

[PATCH 2/2] rsxx: Return -EFAULT if copy_to_user() fails

[Dan Carpenter]

The copy_to_user() function returns the number of bytes remaining but
we want to return -EFAULT to the user if it can't complete the copy.
The "st" variable only holds zero on success or negative error codes on
failure so the type should be int.

Fixes: 36f988e978f8 ("rsxx: Adding in debugfs entries.")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
 drivers/block/rsxx/core.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/block/rsxx/core.c b/drivers/block/rsxx/core.c
index 6207449fa716f..558fa263acbc0 100644
--- a/drivers/block/rsxx/core.c
+++ b/drivers/block/rsxx/core.c
@@ -165,15 +165,17 @@ static ssize_t rsxx_cram_read(struct file *fp, char __user *ubuf,
 {
 	struct rsxx_cardinfo *card = file_inode(fp)->i_private;
 	char *buf;
-	ssize_t st;
+	int st;
 
 	buf = kzalloc(cnt, GFP_KERNEL);
 	if (!buf)
 		return -ENOMEM;
 
 	st = rsxx_creg_read(card, CREG_ADD_CRAM + (u32)*ppos, cnt, buf, 1);
-	if (!st)
-		st = copy_to_user(ubuf, buf, cnt);
+	if (!st) {
+		if (copy_to_user(ubuf, buf, cnt))
+			st = -EFAULT;
+	}
 	kfree(buf);
 	if (st)
 		return st;
-- 
2.26.2

[PATCH RESEND] rsxx: Return -EFAULT if copy_to_user() fails

[Dan Carpenter]

The copy_to_user() function returns the number of bytes remaining but
we want to return -EFAULT to the user if it can't complete the copy.
The "st" variable only holds zero on success or negative error codes on
failure so the type should be int.

Fixes: 36f988e978f8 ("rsxx: Adding in debugfs entries.")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
I sent this last June as part of a 2 patch series.  No one responded
to the patches.  The first patch was a NULL derefence fix but I now
think that the correct fix for that is to remove the "enable_blkdev"
module option...  Anyway, this patch is uncontroversial so I'm going to
resend it.

 drivers/block/rsxx/core.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/block/rsxx/core.c b/drivers/block/rsxx/core.c
index 6207449fa716f..558fa263acbc0 100644
--- a/drivers/block/rsxx/core.c
+++ b/drivers/block/rsxx/core.c
@@ -165,15 +165,17 @@ static ssize_t rsxx_cram_read(struct file *fp, char __user *ubuf,
 {
 	struct rsxx_cardinfo *card = file_inode(fp)->i_private;
 	char *buf;
-	ssize_t st;
+	int st;
 
 	buf = kzalloc(cnt, GFP_KERNEL);
 	if (!buf)
 		return -ENOMEM;
 
 	st = rsxx_creg_read(card, CREG_ADD_CRAM + (u32)*ppos, cnt, buf, 1);
-	if (!st)
-		st = copy_to_user(ubuf, buf, cnt);
+	if (!st) {
+		if (copy_to_user(ubuf, buf, cnt))
+			st = -EFAULT;
+	}
 	kfree(buf);
 	if (st)
 		return st;
-- 
2.26.2

Re: [PATCH RESEND] rsxx: Return -EFAULT if copy_to_user() fails

[Jens Axboe]

On 3/3/21 3:59 AM, Dan Carpenter wrote:
> The copy_to_user() function returns the number of bytes remaining but
> we want to return -EFAULT to the user if it can't complete the copy.
> The "st" variable only holds zero on success or negative error codes on
> failure so the type should be int.
> 
> Fixes: 36f988e978f8 ("rsxx: Adding in debugfs entries.")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> I sent this last June as part of a 2 patch series.  No one responded
> to the patches.  The first patch was a NULL derefence fix but I now
> think that the correct fix for that is to remove the "enable_blkdev"
> module option...  Anyway, this patch is uncontroversial so I'm going to
> resend it.

Thanks Dan, applied.

-- 
Jens Axboe