Messages in this thread | ![/](/images/icornerl.gif) | | Date | Wed, 24 May 2023 13:20:27 +0800 | Subject | Re: [PATCH] media: dvb_ringbuffer: Return -EFAULT if copy fails | From | Su Hui <> |
| |
On 2023/5/24 12:47, Dan Carpenter wrote: > On Wed, May 24, 2023 at 07:23:45AM +0300, Dan Carpenter wrote: >> On Wed, May 24, 2023 at 09:27:33AM +0800, Su Hui wrote: >>> The copy_to/from_user() functions return the number of bytes remaining >>> to be copied, but we want to return -EFAULT to the user. >>> > So basically these bugs are caused because people are used to functions > returning negative error codes and they write some form of: > > ret = copy_from_user(); > if (ret) > return ret; > > If you look at the code and you think, "They author thinks 'ret' is > negative" then probably it is a bug. The common false positives are > in the core kernel where it does: > > return copy_from_user(); > > and the caller checks: > > if (function_one() || function_two() || function_three()) > return -EFAULT; > > Those are done because it's a fast path and adding a lot of if > statements would slow things down. Driver code tends not to do this > because normally drivers are not so performance sensitive and it's more > important to be readable. > > So you have to look at the context a bit.
Thanks very much for your suggestion.
It's confusing about the comment on function declaration.
/** * dvb_ringbuffer_write_user - Writes a buffer received via a user pointer
..........
* Return: number of bytes transferred or -EFAULT
But the function Only returns the number of bytes transferred.
Maybe the comment should be modified because it never returns -EFAULT.
I will look at the context more carefully next time , thanks again!
Su Hui
> regards, > dan carpenter >
| ![\](/images/icornerr.gif) |