On Fri, Mar 28, 2008 at 12:30:29PM -0500, Scott Wood wrote:
> York Sun wrote:
> >+static int fsl_diu_ioctl(struct fb_info *info, unsigned int cmd,
> >+ unsigned long arg)
> >+{
> >+ struct mfb_info *mfbi = info->par;
> >+ struct diu_ad *ad = mfbi->ad;
> >+ struct mfb_chroma_key ck;
> >+ unsigned char global_alpha;
> >+ struct aoi_display_offset aoi_d;
> >+ __u32 pix_fmt;
> >+
> >+ switch (cmd) {
> >+ case MFB_SET_PIXFMT:
> >+ if (!arg)
> >+ return -EINVAL;
> >+ if (copy_from_user(&pix_fmt, (void __user *)arg,
> >+ sizeof(pix_fmt)))
>
> OK, you fixed the cast here...
Maybe better would be to define "buf" at the top of this function as
void __user *buf = (void __user *)arg;, i.e. just once?
>
> >+ return -EFAULT;
> >+ ad->pix_fmt = pix_fmt;
> >+ pr_debug("Set pixel format to 0x%08x\n", ad->pix_fmt);
> >+ break;
> >+ case MFB_GET_PIXFMT:
> >+ if (!arg)
> >+ return -EINVAL;
This if (!arg) appears to be everywhere except default: label, maybe
this could be placed on top too?
> >+ pix_fmt = ad->pix_fmt;
> >+ if (copy_to_user((void *)arg, &pix_fmt, sizeof(pix_fmt)))
> >+ return -EFAULT;
> >+ pr_debug("get pixel format 0x%08x\n", ad->pix_fmt);
> >+ break;
> >+ case MFB_SET_AOID:
> >+ if (!arg)
> >+ return -EINVAL;
> >+ if (copy_from_user(&aoi_d, (void *)arg, sizeof(aoi_d)))
> >+ return -EFAULT;
>
> ...but not anywhere else. All user pointers should have a __user
> annotation.
--
Anton Vorontsov
email: [EMAIL PROTECTED]
irc://irc.freenode.net/bd2
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
