Re: [PATCH RESEND 2/2] Input: uinput: Avoid returning 0 on read()

Hi David,

On Thu, Mar 29, 2012 at 02:14:04PM +0200, David Herrmann wrote:

Consider the output-queue to be almost full. A thread inside read() will
pass the wait_event_*() call and reach the while() loop. Now assume a new
message was added to the output-queue and the queue overruns, i.e.,
we now have udev->head == udev->tail.
The thread now passes the while() loop without fetching any message and
returns 0. However, at least for blocking FDs there is really no reason to
wake up user-space and for non-blocking FDs we should return -EAGAIN now.
Therefore, simply retry the read() if we didn't fetch any message.

We also check whether the user-supplied buffer is actually big enough and
return -EINVAL if it is not. This differs from current behavior which
caused 0 to be returned which actually does not make any sense. This may
break ABI since user-space programs might be used to get 0 if the buffer
is to small. However, 0 means the FD was closed so returning -EINVAL
*must* be handled similar in user-space, otherwise the programs are
broken.
Anyway, we need this check, otherwise we would have a never-returning
loop here because retval would always be 0.

Also note that an queue-overrun is not the only situation where this bug
occurs. We might also have a race between multiple threads here so we
definitely need to handle it this way.

Signed-off-by: David Herrmann <redacted>
Acked-by: Aristeu Rozanski <redacted>
---
Hi Dmitry

Please note that this is based on my previous fix so you might get some
(trivial) conflicts if you didn't apply the previous one. They should be easy to
solve, though.
Also, the issue with returning -EINVAL if the buffer is too small and hence
breaking API can be resolved by moving the check down directly before running
"goto try_again;". However, I think returning -EINVAL is the better fix. Feel
free to change this, though.

I agree that we should return -EINVAL when buffer is too small. I
however do not like the whole "try_again" business; I think it is
perfectly fine to return 0 for blocking reads, we just want to return
-EAGAIN for nonblocking.

I changed around your patches a bit and will post them shortly.

Aristeu, since the patches changed somewhat I dropped your Acked-by so
please Ack the patches you are comfortable with again.

Thanks.

-- 
Dmitry