Skip to contentSkip to conversation

Btrfs: do not overwrite scrub error with fault error in scrub ioctl

PATCH New [email protected] 2 participants 3 messages Dec 14, 2018 – Jan 2, 2019 Btrfs file system

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Btrfs: do not overwrite scrub error with fault error in scrub ioctl

From: Filipe Manana <[email protected]>

If scrub returned an error and then the copy_to_user() call did not succeed, we would overwrite the error returned by scrub with -EFAULT. Fix that by removing the call to copy_to_user() as there is no need to do it at all.

---
 fs/btrfs/ioctl.c | 3 ---
 1 file changed, 3 deletions(-)
fs/btrfs/ioctl.c +0 -3
index 3a27efa2b955..01d18e1a393e 100644
@@ -4301,9 +4301,6 @@ static long btrfs_ioctl_scrub(struct file *file, void __user *arg)
 			      &sa->progress, sa->flags & BTRFS_SCRUB_READONLY,
 			      0);
 
-	if (copy_to_user(arg, sa, sizeof(*sa)))
-		ret = -EFAULT;
-
 	if (!(sa->flags & BTRFS_SCRUB_READONLY))
 		mnt_drop_write_file(file);
 out:
Signature · 1 line
2.11.0
#1 lore raw

PATCH v2 1/1 Btrfs: do not overwrite scrub error with fault error in scrub ioctl

From: Filipe Manana <[email protected]>

If scrub returned an error and then the copy_to_user() call did not succeed, we would overwrite the error returned by scrub with -EFAULT. Fix that by removing the call to copy_to_user() as there is no need to do it at all.

---

V2: Leave to copy_to_user() call since progress needs to be reported back
to user space.

 fs/btrfs/ioctl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
fs/btrfs/ioctl.c +1 -1
index 3a27efa2b955..4c05d9c4fe3d 100644
@@ -4301,7 +4301,7 @@ static long btrfs_ioctl_scrub(struct file *file, void __user *arg)
 			      &sa->progress, sa->flags & BTRFS_SCRUB_READONLY,
 			      0);
 
-	if (copy_to_user(arg, sa, sizeof(*sa)))
+	if (ret == 0 && copy_to_user(arg, sa, sizeof(*sa)))
 		ret = -EFAULT;
 
 	if (!(sa->flags & BTRFS_SCRUB_READONLY))
Signature · 1 line
2.11.0
Reviewed-by on v2 1/1
#2 lore raw
Quoted 7 lines · On Fri, Dec 14, 2018 at 07:50:17PM +0000, [email protected] wrote:

From: Filipe Manana <[email protected]>

If scrub returned an error and then the copy_to_user() call did not succeed, we would overwrite the error returned by scrub with -EFAULT. Fix that by removing the call to copy_to_user() as there is no need to do it at all.

Quoted 5 lines

---

V2: Leave to copy_to_user() call since progress needs to be reported back
to user space.

The changelog does not reflect v2, I'll update the last sentence to

"Fix that by calling copy_to_user() only if btrfs_scrub_dev returned success."

Thanks.