Re: [PATCHv2] evdev: fix evdev_write return value on partial writes

["Henrik Rydberg" <rydberg@euromail.se>]

>  >> Fix it by only handling each full input_event structure and return -EINVAL
>  >> if less than 1 struct was written, similar to how it is done in evdev_read.
>  >> 
>  >> Signed-off-by: Peter Korsgaard <jacmet@sunsite.dk>
> 
>  Henrik> Why not add the Reported-by here yourself?
> 
> Because I sent this mail before seing Baruch's reply.
> 
> I can send a v3 with it, but I wanted to wait a bit to see if there was
> any more feedback.

Great, no problem.

>  >> @@ -321,6 +321,9 @@ static ssize_t evdev_write(struct file *file, const char __user *buffer,
>  >> struct input_event event;
>  >> int retval;
>  >> 
>  >> +	if (count < input_event_size())
>  >> +		return -EINVAL;
>  >> +
> 
>  Henrik> This assumes that write will only ever be called with sufficient
>  Henrik> data. It is not an error to write (and report) less data than
>  Henrik> specified, so perhaps the above will yield unpleasant surprises.
> 
> Well, like this it's consistent with evdev_read(). We can only consume
> complete input_event structures, so we can either return 0 (no events
> consumed) or -EINVAL (invalid data).

But read and write are not completely symmetric.

> Before the patch we returned sizeof input_event (if data after write
> buffer is accessible) or -EINVAL otherwise.
> 
> The v1 patch returned 0, but that causes problems with userspace, as it
> often does:
> 
>     wlile (len) {
>         n = write(fd, buf, len);
>         if (n <= 0) break;
>         len -= n;
>         buf += n;
>     }
> 
> Which will then loop forever.

I won't argue against this case (with < 0) being frequent, but one
should really check "n < len" to be safe. Hopefully Dmitry has some
more input.

Either way, thanks for finding this problem.

Henrik