user accesses in ivtv-fileops.c:ivtv_v4l2_write ?
A message that is not in the archive (sent off-list or not yet received): 20101128174022.GA4401@gallifrey
Quoted 5 lines · Dr. David Alan Gilbert
Hi,
Sparse pointed me at the following line in ivtv-fileops.c'sivtv_v4l2_write:ivtv_write_vbi(itv, (const struct v4l2_sliced_vbi_data *)user_buf, elems);
Hi David,
Let me know if this patch works for your sparse build and adequately addresses the problem.
It might be easiest to review this patch by starting at the bottom and working your way up.
Regards,
Andy
ivtv: ivtv_write_vbi() should use copy_from_user() for user data buffers
ivtv_write_vbi() is used to output VBI data to a TV screen using the CX23415's decoder. It was used for both VBI data that came from the driver internally and VBI data that came from the user, but did not use copy_from_user() for reading the VBI data from the user buffers.
This change adds a new version of the function, ivtv_write_vbi_from_user(), that uses copy_from_user() to read the VBI data provided via user buffers.
This should resolve an sparse build warning reported by Dave Gilbert.
drivers/media/video/ivtv/ivtv-fileops.c +2 -1
@@ -570,7 +570,8 @@ ssize_t ivtv_v4l2_write(struct file *filp, const char __user *user_buf, size_t cint elems = count /sizeof (struct v4l2_sliced_vbi_data );set_bit (IVTV_F_S_APPL_IO , &s->s_flags); -ivtv_write_vbi (itv, (const struct v4l2_sliced_vbi_data *)user_buf, elems); +ivtv_write_vbi_from_user (itv, + (const struct v4l2_sliced_vbi_data __user *)user_buf, elems);return elems *sizeof (struct v4l2_sliced_vbi_data ); }
drivers/media/video/ivtv/ivtv-vbi.c +73 -36
127 lines not on this page · raw message
drivers/media/video/ivtv/ivtv-vbi.h +3 -1
@@ -20,7 +20,9 @@#ifndef IVTV_VBI_H #define IVTV_VBI_H -void ivtv_write_vbi (struct ivtv *itv,const struct v4l2_sliced_vbi_data *sliced,size_t count); +void ivtv_write_vbi_from_user (struct ivtv *itv, +const struct v4l2_sliced_vbi_data __user *sliced, +size_t count);void ivtv_process_vbi_data (struct ivtv *itv,struct ivtv_buffer *buf,u64 pts_stamp,int streamtype);int ivtv_used_line (struct ivtv *itv,int line,int field);
Quoted 11 lines · * Andy Walls ([email protected]) wrote:
Quoted 5 lines · Dr. David Alan Gilbert
Hi,
Sparse pointed me at the following line in ivtv-fileops.c'sivtv_v4l2_write:ivtv_write_vbi(itv, (const struct v4l2_sliced_vbi_data *)user_buf, elems);Hi David,
Let me know if this patch works for your sparse build and adequately addresses the problem.
Hi Andy,
Yes that seems to fix it.
The only other comment I have is that it would probably be better if ivtv_write_vbi_from_user() were to return an error if the copy_from_user were to fail and then pass that all the way back up so that if an app passed a bad pointer in it would get an EFAULT or the like.
Thanks,
Dave
Signature · 4 lines
-----Open up your eyes, open up your mind, open up your code ------- / Dr. David Alan Gilbert | Running GNU/Linux | Happy \ \ gro.gilbert @ treblig.org | | In Hex / \ _________________________|_____ http://www.treblig.org |_______/
Hi Andy,
It looks like we missed something in that copy from user
patch from the end of last year:
+void ivtv_write_vbi_from_user(struct ivtv *itv,
+ const struct v4l2_sliced_vbi_data __user *sliced,
+ size_t cnt)
+{
+ struct vbi_cc cc = { .odd = { 0x80, 0x80 }, .even = { 0x80, 0x80 } };
+ int found_cc = 0;
+ size_t i;
+ struct v4l2_sliced_vbi_data d;+
+ for (i = 0; i < cnt; i++) {
+ if (copy_from_user(&d, sliced + i,
+ sizeof(struct v4l2_sliced_vbi_data)))
+ break;
+ ivtv_write_vbi_line(itv, sliced + i, &cc, &found_cc);
sparse is giving me:drivers/media/video/ivtv/ivtv-vbi.c:177:49: warning: incorrect type in argument 2 (different address spaces)drivers/media/video/ivtv/ivtv-vbi.c:177:49: expected struct v4l2_sliced_vbi_data const *ddrivers/media/video/ivtv/ivtv-vbi.c:177:49: got struct v4l2_sliced_vbi_data const [noderef] <asn:1>*
and I think the point is that while you've copied the data I think you're still passing the user pointer to ivtv_write_vbi_line and it should be:
ivtv_write_vbi_line(itv, &d, &cc, &found_cc);
What do you think?
Dave
Signature · 4 lines
-----Open up your eyes, open up your mind, open up your code ------- / Dr. David Alan Gilbert | Running GNU/Linux | Happy \ \ gro.gilbert @ treblig.org | | In Hex / \ _________________________|_____ http://www.treblig.org |_______/
Quoted 8 lines · Dr. David Alan Gilbert
Hi Andy,
It looks like we missed something in that copy from user
patch from the end of last year:drivers/media/video/ivtv/ivtv-vbi.c +4
-0+void ivtv_write_vbi_from_user (struct ivtv *itv, +const struct v4l2_sliced_vbi_data __user *sliced, +size_t cnt) +{
drivers/media/video/ivtv/ivtv-vbi.c static int odd_parity(u8 c) · line 173
Quoted 10 lines
drivers/media/video/ivtv/ivtv-vbi.c +10
-0+struct vbi_cc cc = { .odd = {0x80 ,0x80 }, .even = {0x80 ,0x80 } }; +int found_cc =0 ; +size_t i; +struct v4l2_sliced_vbi_data d; + +for (i =0 ; i < cnt; i++) { +if (copy_from_user (&d, sliced + i, +sizeof (struct v4l2_sliced_vbi_data ))) +break ; +ivtv_write_vbi_line (itv, sliced + i, &cc, &found_cc);
^^^^^^^^^^
What was I thinking? ---------------------+
Decent plan; failed execution. :(
Quoted 6 lines
sparse is giving me:
drivers/media/video/ivtv/ivtv-vbi.c:177:49: warning: incorrect type in argument 2 (different address spaces)drivers/media/video/ivtv/ivtv-vbi.c:177:49: expected structv4l2_sliced_vbi_dataconst *ddrivers/media/video/ivtv/ivtv-vbi.c:177:49: got structv4l2_sliced_vbi_dataconst [noderef] <asn:1>*
Quoted 8 lines
and I think the point is that while you've copied the data I think you're still passing the user pointer to
ivtv_write_vbi_lineand it should be:ivtv_write_vbi_line(itv, &d, &cc, &found_cc);What do you think?
Yes, it looks like I gooned that one up. :)
That's what I get for trying to fix things with the kids running around before bedtime.
I assume that you have made the replacement and tested that sparse is satisfied?
Regards,
Andy
Quoted 20 lines · * Andy Walls ([email protected]) wrote:
Quoted 8 lines · Dr. David Alan Gilbert
Hi Andy,
It looks like we missed something in that copy from user
patch from the end of last year:drivers/media/video/ivtv/ivtv-vbi.c +4
-0+void ivtv_write_vbi_from_user (struct ivtv *itv, +const struct v4l2_sliced_vbi_data __user *sliced, +size_t cnt) +{drivers/media/video/ivtv/ivtv-vbi.c static int odd_parity(u8 c) · line 173
Quoted 10 lines
drivers/media/video/ivtv/ivtv-vbi.c +10
-0+struct vbi_cc cc = { .odd = {0x80 ,0x80 }, .even = {0x80 ,0x80 } }; +int found_cc =0 ; +size_t i; +struct v4l2_sliced_vbi_data d; + +for (i =0 ; i < cnt; i++) { +if (copy_from_user (&d, sliced + i, +sizeof (struct v4l2_sliced_vbi_data ))) +break ; +ivtv_write_vbi_line (itv, sliced + i, &cc, &found_cc);^^^^^^^^^^
Quoted 3 lines
What was I thinking? ---------------------+
Decent plan; failed execution. :(
Sorry for not spotting it when you sent it out last time!
Quoted 14 lines
Quoted 5 lines
sparse is giving me:
drivers/media/video/ivtv/ivtv-vbi.c:177:49: warning: incorrect type in argument 2 (different address spaces)drivers/media/video/ivtv/ivtv-vbi.c:177:49: expected structv4l2_sliced_vbi_dataconst *ddrivers/media/video/ivtv/ivtv-vbi.c:177:49: got structv4l2_sliced_vbi_dataconst [noderef] <asn:1>*Quoted 8 lines
and I think the point is that while you've copied the data I think you're still passing the user pointer to
ivtv_write_vbi_lineand it should be:ivtv_write_vbi_line(itv, &d, &cc, &found_cc);What do you think?
Quoted 7 lines
Yes, it looks like I gooned that one up. :)
That's what I get for trying to fix things with the kids running around before bedtime.
I assume that you have made the replacement and tested that sparse is satisfied?
Yes, seems happy - of course I thought I did last time :-)
Dave
Signature · 4 lines
-----Open up your eyes, open up your mind, open up your code ------- / Dr. David Alan Gilbert | Running GNU/Linux | Happy \ \ gro.gilbert @ treblig.org | | In Hex / \ _________________________|_____ http://www.treblig.org |_______/