ACPI / debugger: copy_to_user doesn't return errors

The copy_to/from_user() functions don't return error codes, they return the number of bytes remaining. We had intended to return -EFUALT here. We actually have already checked access_ok() in an earlier function so I don't think these functions will fail but let's fix it anyway.

Signed-off-by Dan Carpenter <[email protected]>

drivers/acpi/acpi_dbg.c +0 -0

index f2c92ab..2a1777b 100644

drivers/acpi/acpi_dbg.c +6 -3

+	if (copy_to_user(buf, p, n)) {
+		ret = -EFAULT;
 		goto out;
+	}
 	/* sync tail after removing logs */
 	smp_mb();
 	crc->tail = (crc->tail + n) & (ACPI_AML_BUF_SIZE - 1);

@@ -661,9 +662,10 @@ static int acpi_aml_write_user(const char __user *buf, int len)
 	smp_mb();
 	p = &crc->buf[crc->head];
 	n = min(len, circ_space_to_end(crc));
-	ret = copy_from_user(p, buf, n);
-	if (IS_ERR_VALUE(ret))
+	if (copy_from_user(p, buf, n)) {
+		ret = -EFAULT;
 		goto out;
+	}
 	/* sync head after inserting cmds */
 	smp_wmb();
 	crc->head = (crc->head + n) & (ACPI_AML_BUF_SIZE - 1);
--

To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 7 lines · Dan Carpenter

The copy_to/from_user() functions don't return error codes, they return the number of bytes remaining. We had intended to return -EFUALT here. We actually have already checked access_ok() in an earlier function so I don't think these functions will fail but let's fix it anyway.

Signed-off-by Dan Carpenter <[email protected]>

drivers/acpi/acpi_dbg.c static int acpi_aml_read_user(char __user *buf, int len) · line 596

Quoted 10 lines

drivers/acpi/acpi_dbg.c +0 -2

index f2c92ab..2a1777b 100644
@@ -592,9 +592,10 @@ static int acpi_aml_read_user(char __user *buf, int len)
 	smp_rmb();
 	p = &crc->buf[crc->tail];
 	n = min(len, circ_count_to_end(crc));
-	ret = copy_to_user(buf, p, n);
-	if (IS_ERR_VALUE(ret))

I'm not familiar with IS_ERR_VALUE. Is it to allow functions that return unsigned values to also return negative error codes?

From: Dan Carpenter [mailto:[email protected]]
Sent: Friday, December 25, 2015 3:16 AM
Subject: Re: [patch 2/2] ACPI / debugger: copy_to_user doesn't return errors

Quoted 7 lines · Julia Lawall

Quoted 5 lines · Dan Carpenter

The copy_to/from_user() functions don't return error codes, they return the number of bytes remaining. We had intended to return -EFUALT here. We actually have already checked access_ok() in an earlier function so I don't think these functions will fail but let's fix it anyway.

Quoted 7 lines

drivers/acpi/acpi_dbg.c static int acpi_aml_read_user(char __user *buf,

Quoted 7 lines

Signed-off-by Dan Carpenter <[email protected]>

drivers/acpi/acpi_dbg.c +0 -0

index f2c92ab..2a1777b 100644
@@ -592,9 +592,10 @@ static int acpi_aml_read_user(char __user *buf,

int len)

Quoted 8 lines

drivers/acpi/acpi_dbg.c static int acpi_aml_read_user(char __user *buf, int len) · line 596

Quoted 5 lines

drivers/acpi/acpi_dbg.c +0 -2

 	smp_rmb();
 	p = &crc->buf[crc->tail];
 	n = min(len, circ_count_to_end(crc));
-	ret = copy_to_user(buf, p, n);
-	if (IS_ERR_VALUE(ret))

I'm not familiar with IS_ERR_VALUE. Is it to allow functions that return unsigned values to also return negative error codes?

Yes, but here it is used as a substitue for checking if (ret < 0) or if (ret >= 0). I would have prefered to open code it. Also I prefer if (ret) for when the value is either zero or negative error codes but here we have standardized on IS_ERR_VALUE(ret) for everything.

regards,
dan carpenter