* [patch] V4L/DVB: dvb_ca_en50221: return -EFAULT on copy_to_user
@ 2010-06-04 10:36 Dan Carpenter
2010-06-04 12:26 ` walter harms
2010-06-04 17:14 ` [patch] " walter harms
0 siblings, 2 replies; 5+ messages in thread
From: Dan Carpenter @ 2010-06-04 10:36 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Frederic Weisbecker, Arnd Bergmann, linux-media, kernel-janitors
copy_to_user() returns the number of bytes remaining to be copied which
isn't the right thing to return here. The comments say that these
functions in dvb_ca_en50221.c should return the number of bytes copied or
an error return. I've changed it to return -EFAULT.
Signed-off-by: Dan Carpenter <error27@gmail.com>
diff --git a/drivers/media/dvb/dvb-core/dvb_ca_en50221.c b/drivers/media/dvb/dvb-core/dvb_ca_en50221.c
index ef259a0..aa7a298 100644
--- a/drivers/media/dvb/dvb-core/dvb_ca_en50221.c
+++ b/drivers/media/dvb/dvb-core/dvb_ca_en50221.c
@@ -1318,8 +1318,10 @@ static ssize_t dvb_ca_en50221_io_write(struct file *file,
fragbuf[0] = connection_id;
fragbuf[1] = ((fragpos + fraglen) < count) ? 0x80 : 0x00;
- if ((status = copy_from_user(fragbuf + 2, buf + fragpos, fraglen)) != 0)
+ if ((status = copy_from_user(fragbuf + 2, buf + fragpos, fraglen)) != 0) {
+ status = -EFAULT;
goto exit;
+ }
timeout = jiffies + HZ / 2;
written = 0;
@@ -1494,8 +1496,10 @@ static ssize_t dvb_ca_en50221_io_read(struct file *file, char __user * buf,
hdr[0] = slot;
hdr[1] = connection_id;
- if ((status = copy_to_user(buf, hdr, 2)) != 0)
+ if ((status = copy_to_user(buf, hdr, 2)) != 0) {
+ status = -EFAULT;
goto exit;
+ }
status = pktlen;
exit:
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [patch] V4L/DVB: dvb_ca_en50221: return -EFAULT on copy_to_user
2010-06-04 10:36 [patch] V4L/DVB: dvb_ca_en50221: return -EFAULT on copy_to_user Dan Carpenter
@ 2010-06-04 12:26 ` walter harms
2010-06-04 15:37 ` [patch] V4L/DVB: dvb_ca_en50221: return -EFAULT on Dan Carpenter
2010-06-04 15:39 ` [patch v2] V4L/DVB: dvb_ca_en50221: return -EFAULT on copy_to_user Dan Carpenter
2010-06-04 17:14 ` [patch] " walter harms
1 sibling, 2 replies; 5+ messages in thread
From: walter harms @ 2010-06-04 12:26 UTC (permalink / raw)
To: Dan Carpenter
Cc: Mauro Carvalho Chehab, Frederic Weisbecker, Arnd Bergmann,
linux-media, kernel-janitors
Dan Carpenter schrieb:
> copy_to_user() returns the number of bytes remaining to be copied which
> isn't the right thing to return here. The comments say that these
> functions in dvb_ca_en50221.c should return the number of bytes copied or
> an error return. I've changed it to return -EFAULT.
>
> Signed-off-by: Dan Carpenter <error27@gmail.com>
>
> diff --git a/drivers/media/dvb/dvb-core/dvb_ca_en50221.c b/drivers/media/dvb/dvb-core/dvb_ca_en50221.c
> index ef259a0..aa7a298 100644
> --- a/drivers/media/dvb/dvb-core/dvb_ca_en50221.c
> +++ b/drivers/media/dvb/dvb-core/dvb_ca_en50221.c
> @@ -1318,8 +1318,10 @@ static ssize_t dvb_ca_en50221_io_write(struct file *file,
>
> fragbuf[0] = connection_id;
> fragbuf[1] = ((fragpos + fraglen) < count) ? 0x80 : 0x00;
> - if ((status = copy_from_user(fragbuf + 2, buf + fragpos, fraglen)) != 0)
> + if ((status = copy_from_user(fragbuf + 2, buf + fragpos, fraglen)) != 0) {
> + status = -EFAULT;
> goto exit;
> + }
>
> timeout = jiffies + HZ / 2;
> written = 0;
> @@ -1494,8 +1496,10 @@ static ssize_t dvb_ca_en50221_io_read(struct file *file, char __user * buf,
>
> hdr[0] = slot;
> hdr[1] = connection_id;
> - if ((status = copy_to_user(buf, hdr, 2)) != 0)
> + if ((status = copy_to_user(buf, hdr, 2)) != 0) {
> + status = -EFAULT;
> goto exit;
> + }
> status = pktlen;
>
> exit:
> --
Doint to many things at once is bad. IMHO it is more readable to do so:
+status = copy_to_user(buf, hdr, 2);
+if ( status != 0) {
Maybe the maintainer has different ideas but especialy lines like will gain.
-if ((status = copy_from_user(fragbuf + 2, buf + fragpos, fraglen)) != 0)
+status = copy_from_user(fragbuf + 2, buf + fragpos, fraglen):
+if ( status != 0) {
just my 2 cents,
wh
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [patch] V4L/DVB: dvb_ca_en50221: return -EFAULT on
2010-06-04 12:26 ` walter harms
@ 2010-06-04 15:37 ` Dan Carpenter
2010-06-04 15:39 ` [patch v2] V4L/DVB: dvb_ca_en50221: return -EFAULT on copy_to_user Dan Carpenter
1 sibling, 0 replies; 5+ messages in thread
From: Dan Carpenter @ 2010-06-04 15:37 UTC (permalink / raw)
To: walter harms
Cc: Mauro Carvalho Chehab, Frederic Weisbecker, Arnd Bergmann,
linux-media, kernel-janitors
On Fri, Jun 04, 2010 at 02:26:05PM +0200, walter harms wrote:
>
> Doint to many things at once is bad. IMHO it is more readable to do so:
>
> +status = copy_to_user(buf, hdr, 2);
> +if ( status != 0) {
>
> Maybe the maintainer has different ideas but especialy lines like will gain.
>
> -if ((status = copy_from_user(fragbuf + 2, buf + fragpos, fraglen)) != 0)
> +status = copy_from_user(fragbuf + 2, buf + fragpos, fraglen):
> +if ( status != 0) {
>
> just my 2 cents,
You're right of course as always and checkpatch warns about these as
well.
I figured if it was in the original code, it was probably OK to leave it.
But I now recognize this as pure laziness on my part and I appologize.
Twenty lashes for me and all that. Fixed patch coming up. ;)
regards,
dan carpenter
^ permalink raw reply [flat|nested] 5+ messages in thread
* [patch v2] V4L/DVB: dvb_ca_en50221: return -EFAULT on copy_to_user
2010-06-04 12:26 ` walter harms
2010-06-04 15:37 ` [patch] V4L/DVB: dvb_ca_en50221: return -EFAULT on Dan Carpenter
@ 2010-06-04 15:39 ` Dan Carpenter
1 sibling, 0 replies; 5+ messages in thread
From: Dan Carpenter @ 2010-06-04 15:39 UTC (permalink / raw)
To: walter harms
Cc: Mauro Carvalho Chehab, Frederic Weisbecker, Arnd Bergmann,
linux-media, kernel-janitors
copy_to_user() returns the number of bytes remaining to be copied which
isn't the right thing to return here. The comments say that these
functions in dvb_ca_en50221.c should return the number of bytes copied or
an error return. I've changed it to return -EFAULT.
Signed-off-by: Dan Carpenter <error27@gmail.com>
---
V2: Some style fixes suggested by Walter Harms.
diff --git a/drivers/media/dvb/dvb-core/dvb_ca_en50221.c b/drivers/media/dvb/dvb-core/dvb_ca_en50221.c
index ef259a0..cb97e6b 100644
--- a/drivers/media/dvb/dvb-core/dvb_ca_en50221.c
+++ b/drivers/media/dvb/dvb-core/dvb_ca_en50221.c
@@ -1318,8 +1318,11 @@ static ssize_t dvb_ca_en50221_io_write(struct file *file,
fragbuf[0] = connection_id;
fragbuf[1] = ((fragpos + fraglen) < count) ? 0x80 : 0x00;
- if ((status = copy_from_user(fragbuf + 2, buf + fragpos, fraglen)) != 0)
+ status = copy_from_user(fragbuf + 2, buf + fragpos, fraglen);
+ if (status) {
+ status = -EFAULT;
goto exit;
+ }
timeout = jiffies + HZ / 2;
written = 0;
@@ -1494,8 +1497,11 @@ static ssize_t dvb_ca_en50221_io_read(struct file *file, char __user * buf,
hdr[0] = slot;
hdr[1] = connection_id;
- if ((status = copy_to_user(buf, hdr, 2)) != 0)
+ status = copy_to_user(buf, hdr, 2);
+ if (status) {
+ status = -EFAULT;
goto exit;
+ }
status = pktlen;
exit:
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [patch] V4L/DVB: dvb_ca_en50221: return -EFAULT on copy_to_user
2010-06-04 10:36 [patch] V4L/DVB: dvb_ca_en50221: return -EFAULT on copy_to_user Dan Carpenter
2010-06-04 12:26 ` walter harms
@ 2010-06-04 17:14 ` walter harms
1 sibling, 0 replies; 5+ messages in thread
From: walter harms @ 2010-06-04 17:14 UTC (permalink / raw)
To: kernel-janitors
Dan Carpenter schrieb:
> On Fri, Jun 04, 2010 at 02:26:05PM +0200, walter harms wrote:
>> Doint to many things at once is bad. IMHO it is more readable to do so:
>>
>> +status = copy_to_user(buf, hdr, 2);
>> +if ( status != 0) {
>>
>> Maybe the maintainer has different ideas but especialy lines like will gain.
>>
>> -if ((status = copy_from_user(fragbuf + 2, buf + fragpos, fraglen)) != 0)
>> +status = copy_from_user(fragbuf + 2, buf + fragpos, fraglen):
>> +if ( status != 0) {
>>
>> just my 2 cents,
>
> You're right of course as always and checkpatch warns about these as
> well.
>
> I figured if it was in the original code, it was probably OK to leave it.
> But I now recognize this as pure laziness on my part and I appologize.
> Twenty lashes for me and all that. Fixed patch coming up. ;)
>
> regards,
> dan carpenter
I hope my guidance will ease your way to enlightenment. ;)
re,
wh
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2010-06-04 17:14 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-06-04 10:36 [patch] V4L/DVB: dvb_ca_en50221: return -EFAULT on copy_to_user Dan Carpenter
2010-06-04 12:26 ` walter harms
2010-06-04 15:37 ` [patch] V4L/DVB: dvb_ca_en50221: return -EFAULT on Dan Carpenter
2010-06-04 15:39 ` [patch v2] V4L/DVB: dvb_ca_en50221: return -EFAULT on copy_to_user Dan Carpenter
2010-06-04 17:14 ` [patch] " walter harms
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).