Message ID | 20221215182140.129559-3-helgaas@kernel.org |
---|---|
State | New |
Headers | show |
Series | PCI: switchtec: Trivial cleanups | expand |
On 2022-12-15 11:21, Bjorn Helgaas wrote: > From: Bjorn Helgaas <bhelgaas@google.com> > > Some switchtec_dev_read() error cases assign to "rc", then branch to "out". > But the code at "out" never uses "rc". Drop the useless assignments. No > functional change intended. Ah, hmm, yes. I think if copy_to_user() fails, the function should probably return -EFAULT. So perhaps an unlock and specific return as is done in previous conditions in the same function? Thanks, Logan
On Thu, Dec 15, 2022 at 11:34:06AM -0700, Logan Gunthorpe wrote: > On 2022-12-15 11:21, Bjorn Helgaas wrote: > > From: Bjorn Helgaas <bhelgaas@google.com> > > > > Some switchtec_dev_read() error cases assign to "rc", then branch to "out". > > But the code at "out" never uses "rc". Drop the useless assignments. No > > functional change intended. > > Ah, hmm, yes. I think if copy_to_user() fails, the function should > probably return -EFAULT. So perhaps an unlock and specific return as is > done in previous conditions in the same function? Sure, I'll post a v2 that does that.
diff --git a/drivers/pci/switch/switchtec.c b/drivers/pci/switch/switchtec.c index d7ae84070e29..c04c7ee35184 100644 --- a/drivers/pci/switch/switchtec.c +++ b/drivers/pci/switch/switchtec.c @@ -605,18 +605,14 @@ static ssize_t switchtec_dev_read(struct file *filp, char __user *data, rc = copy_to_user(data, &stuser->return_code, sizeof(stuser->return_code)); - if (rc) { - rc = -EFAULT; + if (rc) goto out; - } data += sizeof(stuser->return_code); rc = copy_to_user(data, &stuser->data, size - sizeof(stuser->return_code)); - if (rc) { - rc = -EFAULT; + if (rc) goto out; - } stuser_set_state(stuser, MRPC_IDLE);