From: Dan Carpenter <dan.carpenter@oracle.com>
To: Mike Marciniszyn <infinipath@intel.com>
Cc: devel@driverdev.osuosl.org, linux-rdma@vger.kernel.org,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
kernel-janitors@vger.kernel.org,
Doug Ledford <dledford@redhat.com>,
Sean Hefty <sean.hefty@intel.com>,
Hal Rosenstock <hal.rosenstock@gmail.com>
Subject: [patch] IB/hfi1: fix copy_to/from_user() error handling
Date: Tue, 15 Sep 2015 10:35:25 +0000 [thread overview]
Message-ID: <20150915103525.GA20813@mwanda> (raw)
copy_to/from_user() returns the number of bytes which we were not able
to copy. It doesn't return an error code.
Also a couple places had a printk() on error and I removed that because
people can take advantage of it to fill /var/log/messages with spam.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
This patch has some checkpatch warnings because the alignment is off.
That was a problem in the original code and needs to be fixed in a separate patch.
Also there is another bug:
drivers/staging/rdma/hfi1/diag.c:1160 hfi1_ioctl() error: scheduling with locks held: 'spin_lock:snoop_lock'
That function needs to be re-worked and simplified and technically I'm
still on vacation so I didn't address it.
diff --git a/drivers/staging/rdma/hfi1/diag.c b/drivers/staging/rdma/hfi1/diag.c
index 6777d6b..ce01dee 100644
--- a/drivers/staging/rdma/hfi1/diag.c
+++ b/drivers/staging/rdma/hfi1/diag.c
@@ -1012,11 +1012,10 @@ static long hfi1_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
case HFI1_SNOOP_IOCSETLINKSTATE_EXTRA:
memset(&link_info, 0, sizeof(link_info));
- ret = copy_from_user(&link_info,
+ if (copy_from_user(&link_info,
(struct hfi1_link_info __user *)arg,
- sizeof(link_info));
- if (ret)
- break;
+ sizeof(link_info)))
+ ret = -EFAULT;
value = link_info.port_state;
index = link_info.port_number;
@@ -1080,9 +1079,10 @@ static long hfi1_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
case HFI1_SNOOP_IOCGETLINKSTATE_EXTRA:
if (cmd = HFI1_SNOOP_IOCGETLINKSTATE_EXTRA) {
memset(&link_info, 0, sizeof(link_info));
- ret = copy_from_user(&link_info,
+ if (copy_from_user(&link_info,
(struct hfi1_link_info __user *)arg,
- sizeof(link_info));
+ sizeof(link_info)))
+ ret = -EFAULT;
index = link_info.port_number;
} else {
ret = __get_user(index, (int __user *) arg);
@@ -1114,9 +1114,10 @@ static long hfi1_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
ppd->link_speed_active;
link_info.link_width_active ppd->link_width_active;
- ret = copy_to_user(
+ if (copy_to_user(
(struct hfi1_link_info __user *)arg,
- &link_info, sizeof(link_info));
+ &link_info, sizeof(link_info)))
+ ret = -EFAULT;
} else {
ret = __put_user(value, (int __user *)arg);
}
@@ -1142,10 +1143,9 @@ static long hfi1_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
snoop_dbg("Setting filter");
/* just copy command structure */
argp = (unsigned long *)arg;
- ret = copy_from_user(&filter_cmd, (void __user *)argp,
- sizeof(filter_cmd));
- if (ret < 0) {
- pr_alert("Error copying filter command\n");
+ if (copy_from_user(&filter_cmd, (void __user *)argp,
+ sizeof(filter_cmd))) {
+ ret = -EFAULT;
break;
}
if (filter_cmd.opcode >= HFI1_MAX_FILTERS) {
@@ -1167,12 +1167,11 @@ static long hfi1_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
break;
}
/* copy remaining data from userspace */
- ret = copy_from_user((u8 *)filter_value,
+ if (copy_from_user((u8 *)filter_value,
(void __user *)filter_cmd.value_ptr,
- filter_cmd.length);
- if (ret < 0) {
+ filter_cmd.length)) {
kfree(filter_value);
- pr_alert("Error copying filter data\n");
+ ret = -EFAULT;
break;
}
/* Drain packets first */
WARNING: multiple messages have this Message-ID (diff)
From: Dan Carpenter <dan.carpenter@oracle.com>
To: Mike Marciniszyn <infinipath@intel.com>
Cc: devel@driverdev.osuosl.org, linux-rdma@vger.kernel.org,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
kernel-janitors@vger.kernel.org,
Doug Ledford <dledford@redhat.com>,
Sean Hefty <sean.hefty@intel.com>,
Hal Rosenstock <hal.rosenstock@gmail.com>
Subject: [patch] IB/hfi1: fix copy_to/from_user() error handling
Date: Tue, 15 Sep 2015 13:35:25 +0300 [thread overview]
Message-ID: <20150915103525.GA20813@mwanda> (raw)
copy_to/from_user() returns the number of bytes which we were not able
to copy. It doesn't return an error code.
Also a couple places had a printk() on error and I removed that because
people can take advantage of it to fill /var/log/messages with spam.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
This patch has some checkpatch warnings because the alignment is off.
That was a problem in the original code and needs to be fixed in a separate patch.
Also there is another bug:
drivers/staging/rdma/hfi1/diag.c:1160 hfi1_ioctl() error: scheduling with locks held: 'spin_lock:snoop_lock'
That function needs to be re-worked and simplified and technically I'm
still on vacation so I didn't address it.
diff --git a/drivers/staging/rdma/hfi1/diag.c b/drivers/staging/rdma/hfi1/diag.c
index 6777d6b..ce01dee 100644
--- a/drivers/staging/rdma/hfi1/diag.c
+++ b/drivers/staging/rdma/hfi1/diag.c
@@ -1012,11 +1012,10 @@ static long hfi1_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
case HFI1_SNOOP_IOCSETLINKSTATE_EXTRA:
memset(&link_info, 0, sizeof(link_info));
- ret = copy_from_user(&link_info,
+ if (copy_from_user(&link_info,
(struct hfi1_link_info __user *)arg,
- sizeof(link_info));
- if (ret)
- break;
+ sizeof(link_info)))
+ ret = -EFAULT;
value = link_info.port_state;
index = link_info.port_number;
@@ -1080,9 +1079,10 @@ static long hfi1_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
case HFI1_SNOOP_IOCGETLINKSTATE_EXTRA:
if (cmd == HFI1_SNOOP_IOCGETLINKSTATE_EXTRA) {
memset(&link_info, 0, sizeof(link_info));
- ret = copy_from_user(&link_info,
+ if (copy_from_user(&link_info,
(struct hfi1_link_info __user *)arg,
- sizeof(link_info));
+ sizeof(link_info)))
+ ret = -EFAULT;
index = link_info.port_number;
} else {
ret = __get_user(index, (int __user *) arg);
@@ -1114,9 +1114,10 @@ static long hfi1_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
ppd->link_speed_active;
link_info.link_width_active =
ppd->link_width_active;
- ret = copy_to_user(
+ if (copy_to_user(
(struct hfi1_link_info __user *)arg,
- &link_info, sizeof(link_info));
+ &link_info, sizeof(link_info)))
+ ret = -EFAULT;
} else {
ret = __put_user(value, (int __user *)arg);
}
@@ -1142,10 +1143,9 @@ static long hfi1_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
snoop_dbg("Setting filter");
/* just copy command structure */
argp = (unsigned long *)arg;
- ret = copy_from_user(&filter_cmd, (void __user *)argp,
- sizeof(filter_cmd));
- if (ret < 0) {
- pr_alert("Error copying filter command\n");
+ if (copy_from_user(&filter_cmd, (void __user *)argp,
+ sizeof(filter_cmd))) {
+ ret = -EFAULT;
break;
}
if (filter_cmd.opcode >= HFI1_MAX_FILTERS) {
@@ -1167,12 +1167,11 @@ static long hfi1_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
break;
}
/* copy remaining data from userspace */
- ret = copy_from_user((u8 *)filter_value,
+ if (copy_from_user((u8 *)filter_value,
(void __user *)filter_cmd.value_ptr,
- filter_cmd.length);
- if (ret < 0) {
+ filter_cmd.length)) {
kfree(filter_value);
- pr_alert("Error copying filter data\n");
+ ret = -EFAULT;
break;
}
/* Drain packets first */
next reply other threads:[~2015-09-15 10:35 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-09-15 10:35 Dan Carpenter [this message]
2015-09-15 10:35 ` [patch] IB/hfi1: fix copy_to/from_user() error handling Dan Carpenter
2015-09-15 14:01 ` Marciniszyn, Mike
2015-09-18 15:50 ` Doug Ledford
2015-09-18 15:50 ` Doug Ledford
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20150915103525.GA20813@mwanda \
--to=dan.carpenter@oracle.com \
--cc=devel@driverdev.osuosl.org \
--cc=dledford@redhat.com \
--cc=gregkh@linuxfoundation.org \
--cc=hal.rosenstock@gmail.com \
--cc=infinipath@intel.com \
--cc=kernel-janitors@vger.kernel.org \
--cc=linux-rdma@vger.kernel.org \
--cc=sean.hefty@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.