Dr. David Alan Gilbert wrote:
> Hi,
> Sparse pointed me at the following line in ivtv-fileops.c's ivtv_v4l2_write:
>
> ivtv_write_vbi(itv, (const struct v4l2_sliced_vbi_data *)user_buf, elems);
>
> Now user_buf is a parameter:
> const char __user *user_buf,
>
> so that is losing the __user, and I don't see what else protects
> the accesses that ivtv_write_vbi performs.
Ummm, "__user" and similar attributes like "__iomem", don't protect
anything -- do they? I thought they were there to help tools like
sparse to detect type problems.
It looks like the patch that added "__user" attributes in the ivtv
driver missed or deliberately ignored this function.
> Is there something that makes this safe?
Not from what I can see in a few minutes worth of looking.
>From include/linux/compiler.h"
#ifdef __CHECKER__
# define __user __attribute__((noderef, address_space(1)))
# define __kernel __attribute__((address_space(0)))
...
# define __iomem __attribute__((noderef, address_space(2)))
It would appear that directly dereferencing the user pointer is not a
good thing to do. However, as you note, that's exactly what
ivtv_write_vbi() does.
v4l2_write() and ivtv_v4l2_write() are wrappers that don't do any
copy_from_user() calls before passing the data along.
Why does the call not induce faults for people? I'm not sure. Without
looking too hard through all the copy_from_user() checks, I'm guessing:
a. the user_buf for the VBI data is likely allocated properly aligned on
a writeable page.
b. 23 lines of VBI data for two fields only takes up 64*23*2 = 2944
bytes which likely does not cross a 4096 byte page boundary
c. Not many people have PVR-350's and even less use it to send out VBI
data to their TV.
Patches welcome. :)
Regards,
Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
