Re: [PATCH 3/4] drivers/misc: Add ASpeed LPC control driver

On Thu, Jan 12, 2017 at 09:27:47AM -0600, Benjamin Herrenschmidt wrote:

On Thu, 2017-01-12 at 11:30 +0100, Greg KH wrote:

quoted

quoted

quoted

And don't call access_ok(), it's racy and no driver should ever do that.

Oh, duly noted. I'll be sure to check out how and why. Perhaps it
would be wise that no driver actually do that, I'm quite sure I used
other drivers as examples of best practice.

You did?  Please point me at that code so we can fix them up properly.
Cargo-cult coding is not a good thing, but it happens, so if we can at
least provide clean code to fixate on, it's good overall for everyone.

How so ? I mean, access_ok followed by __get/__put_user is still a
classic, what's wrong with it ?

No "normal" driver should do that, just call copy_to/from_user and be
done with it.  That way all of the proper locking and validation checks
like this are done correctly for you.  Why would a driver ever call the
"raw" __get/__put_user functions?

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html