* [patch] isdn: return -EFAULT if copy_from_user() fails
@ 2010-12-10 12:40 Dan Carpenter
2010-12-11 0:16 ` David Miller
2010-12-11 0:41 ` Nicolas Kaiser
0 siblings, 2 replies; 7+ messages in thread
From: Dan Carpenter @ 2010-12-10 12:40 UTC (permalink / raw)
To: Karsten Keil; +Cc: David S. Miller, netdev, kernel-janitors
We should be returning -EFAULT here.
Mostly this patch is to silence a smatch warning. The upper levels
of this driver turn all non-zero return values from isar_load_firmware()
into 1.
Signed-off-by: Dan Carpenter <error27@gmail.com>
diff --git a/drivers/isdn/hisax/isar.c b/drivers/isdn/hisax/isar.c
index 2e72227..9cd4829 100644
--- a/drivers/isdn/hisax/isar.c
+++ b/drivers/isdn/hisax/isar.c
@@ -212,9 +212,9 @@ isar_load_firmware(struct IsdnCardState *cs, u_char __user *buf)
cs->debug &= ~(L1_DEB_HSCX | L1_DEB_HSCX_FIFO);
#endif
- if ((ret = copy_from_user(&size, p, sizeof(int)))) {
+ if (copy_from_user(&size, p, sizeof(int))) {
printk(KERN_ERR"isar_load_firmware copy_from_user ret %d\n", ret);
- return ret;
+ return -EFAULT;
}
p += sizeof(int);
printk(KERN_DEBUG"isar_load_firmware size: %d\n", size);
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [patch] isdn: return -EFAULT if copy_from_user() fails
2010-12-10 12:40 [patch] isdn: return -EFAULT if copy_from_user() fails Dan Carpenter
@ 2010-12-11 0:16 ` David Miller
2010-12-11 0:41 ` Nicolas Kaiser
1 sibling, 0 replies; 7+ messages in thread
From: David Miller @ 2010-12-11 0:16 UTC (permalink / raw)
To: error27; +Cc: isdn, netdev, kernel-janitors
From: Dan Carpenter <error27@gmail.com>
Date: Fri, 10 Dec 2010 15:40:09 +0300
> We should be returning -EFAULT here.
>
> Mostly this patch is to silence a smatch warning. The upper levels
> of this driver turn all non-zero return values from isar_load_firmware()
> into 1.
>
> Signed-off-by: Dan Carpenter <error27@gmail.com>
Applied.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [patch] isdn: return -EFAULT if copy_from_user() fails
2010-12-10 12:40 [patch] isdn: return -EFAULT if copy_from_user() fails Dan Carpenter
2010-12-11 0:16 ` David Miller
@ 2010-12-11 0:41 ` Nicolas Kaiser
2010-12-11 0:47 ` David Miller
1 sibling, 1 reply; 7+ messages in thread
From: Nicolas Kaiser @ 2010-12-11 0:41 UTC (permalink / raw)
To: Dan Carpenter; +Cc: David S. Miller, Karsten Keil, netdev, kernel-janitors
Ahem, we're printing this return value:
* Dan Carpenter <error27@gmail.com>:
> - if ((ret = copy_from_user(&size, p, sizeof(int)))) {
^^^
> + if (copy_from_user(&size, p, sizeof(int))) {
> printk(KERN_ERR"isar_load_firmware copy_from_user ret %d\n", ret);
^^^
> - return ret;
> + return -EFAULT;
Cheers,
Nicolas Kaiser
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [patch] isdn: return -EFAULT if copy_from_user() fails
2010-12-11 0:41 ` Nicolas Kaiser
@ 2010-12-11 0:47 ` David Miller
2010-12-11 0:50 ` David Miller
0 siblings, 1 reply; 7+ messages in thread
From: David Miller @ 2010-12-11 0:47 UTC (permalink / raw)
To: nikai; +Cc: error27, isdn, netdev, kernel-janitors
From: Nicolas Kaiser <nikai@nikai.net>
Date: Sat, 11 Dec 2010 01:41:54 +0100
> Ahem, we're printing this return value:
>
> * Dan Carpenter <error27@gmail.com>:
>> - if ((ret = copy_from_user(&size, p, sizeof(int)))) {
> ^^^
>> + if (copy_from_user(&size, p, sizeof(int))) {
>> printk(KERN_ERR"isar_load_firmware copy_from_user ret %d\n", ret);
> ^^^
>> - return ret;
>> + return -EFAULT;
I'll fix this, thanks.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [patch] isdn: return -EFAULT if copy_from_user() fails
2010-12-11 0:47 ` David Miller
@ 2010-12-11 0:50 ` David Miller
2010-12-11 0:52 ` David Miller
0 siblings, 1 reply; 7+ messages in thread
From: David Miller @ 2010-12-11 0:50 UTC (permalink / raw)
To: nikai; +Cc: error27, isdn, netdev, kernel-janitors
From: David Miller <davem@davemloft.net>
Date: Fri, 10 Dec 2010 16:47:21 -0800 (PST)
> From: Nicolas Kaiser <nikai@nikai.net>
> Date: Sat, 11 Dec 2010 01:41:54 +0100
>
>> Ahem, we're printing this return value:
>>
>> * Dan Carpenter <error27@gmail.com>:
>>> - if ((ret = copy_from_user(&size, p, sizeof(int)))) {
>> ^^^
>>> + if (copy_from_user(&size, p, sizeof(int))) {
>>> printk(KERN_ERR"isar_load_firmware copy_from_user ret %d\n", ret);
>> ^^^
>>> - return ret;
>>> + return -EFAULT;
>
> I'll fix this, thanks.
As follows:
>From cf108fdd482e80161128c2ed01e7f4fb5bc728b9 Mon Sep 17 00:00:00 2001
From: David S. Miller <davem@davemloft.net>
Date: Fri, 10 Dec 2010 16:49:24 -0800
Subject: [PATCH] isdn: Fix printed out copy_from_user() return value after previous change.
Signed-off-by: David S. Miller <davem@davemloft.net>
---
drivers/isdn/hisax/isar.c | 7 ++++---
1 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/isdn/hisax/isar.c b/drivers/isdn/hisax/isar.c
index c9cd4d2..313ce2d 100644
--- a/drivers/isdn/hisax/isar.c
+++ b/drivers/isdn/hisax/isar.c
@@ -189,7 +189,7 @@ ISARVersion(struct IsdnCardState *cs, char *s)
static int
isar_load_firmware(struct IsdnCardState *cs, u_char __user *buf)
{
- int ret, size, cnt, debug;
+ int cfu_ret, size, cnt, debug;
u_char len, nom, noc;
u_short sadr, left, *sp;
u_char __user *p = buf;
@@ -212,8 +212,9 @@ isar_load_firmware(struct IsdnCardState *cs, u_char __user *buf)
cs->debug &= ~(L1_DEB_HSCX | L1_DEB_HSCX_FIFO);
#endif
- if (copy_from_user(&size, p, sizeof(int))) {
- printk(KERN_ERR"isar_load_firmware copy_from_user ret %d\n", ret);
+ cfu_ret = copy_from_user(&size, p, sizeof(int));
+ if (cfu_ret) {
+ printk(KERN_ERR"isar_load_firmware copy_from_user ret %d\n", cfu_ret);
return -EFAULT;
}
p += sizeof(int);
--
1.7.3.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [patch] isdn: return -EFAULT if copy_from_user() fails
2010-12-11 0:50 ` David Miller
@ 2010-12-11 0:52 ` David Miller
2010-12-11 5:07 ` Dan Carpenter
0 siblings, 1 reply; 7+ messages in thread
From: David Miller @ 2010-12-11 0:52 UTC (permalink / raw)
To: nikai; +Cc: error27, isdn, netdev, kernel-janitors
From: David Miller <davem@davemloft.net>
Date: Fri, 10 Dec 2010 16:50:03 -0800 (PST)
> From: David Miller <davem@davemloft.net>
> Date: Fri, 10 Dec 2010 16:47:21 -0800 (PST)
>
>> From: Nicolas Kaiser <nikai@nikai.net>
>> Date: Sat, 11 Dec 2010 01:41:54 +0100
>>
>>> Ahem, we're printing this return value:
>>>
>>> * Dan Carpenter <error27@gmail.com>:
>>>> - if ((ret = copy_from_user(&size, p, sizeof(int)))) {
>>> ^^^
>>>> + if (copy_from_user(&size, p, sizeof(int))) {
>>>> printk(KERN_ERR"isar_load_firmware copy_from_user ret %d\n", ret);
>>> ^^^
>>>> - return ret;
>>>> + return -EFAULT;
>>
>> I'll fix this, thanks.
>
> As follows:
Ugh, that was buggy, let's try this :-)
--------------------
isdn: Fix printed out copy_from_user() return value after previous change.
Signed-off-by: David S. Miller <davem@davemloft.net>
---
drivers/isdn/hisax/isar.c | 7 ++++---
1 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/isdn/hisax/isar.c b/drivers/isdn/hisax/isar.c
index c9cd4d2..d4cce33 100644
--- a/drivers/isdn/hisax/isar.c
+++ b/drivers/isdn/hisax/isar.c
@@ -189,7 +189,7 @@ ISARVersion(struct IsdnCardState *cs, char *s)
static int
isar_load_firmware(struct IsdnCardState *cs, u_char __user *buf)
{
- int ret, size, cnt, debug;
+ int cfu_ret, ret, size, cnt, debug;
u_char len, nom, noc;
u_short sadr, left, *sp;
u_char __user *p = buf;
@@ -212,8 +212,9 @@ isar_load_firmware(struct IsdnCardState *cs, u_char __user *buf)
cs->debug &= ~(L1_DEB_HSCX | L1_DEB_HSCX_FIFO);
#endif
- if (copy_from_user(&size, p, sizeof(int))) {
- printk(KERN_ERR"isar_load_firmware copy_from_user ret %d\n", ret);
+ cfu_ret = copy_from_user(&size, p, sizeof(int));
+ if (cfu_ret) {
+ printk(KERN_ERR"isar_load_firmware copy_from_user ret %d\n", cfu_ret);
return -EFAULT;
}
p += sizeof(int);
--
1.7.3.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [patch] isdn: return -EFAULT if copy_from_user() fails
2010-12-11 0:52 ` David Miller
@ 2010-12-11 5:07 ` Dan Carpenter
0 siblings, 0 replies; 7+ messages in thread
From: Dan Carpenter @ 2010-12-11 5:07 UTC (permalink / raw)
To: David Miller; +Cc: nikai, isdn, netdev, kernel-janitors
> >>> Ahem, we're printing this return value:
> >>>
Doh! Thanks for that guys.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2010-12-11 5:07 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-12-10 12:40 [patch] isdn: return -EFAULT if copy_from_user() fails Dan Carpenter
2010-12-11 0:16 ` David Miller
2010-12-11 0:41 ` Nicolas Kaiser
2010-12-11 0:47 ` David Miller
2010-12-11 0:50 ` David Miller
2010-12-11 0:52 ` David Miller
2010-12-11 5:07 ` Dan Carpenter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).