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

[Peter Korsgaard <jacmet@sunsite.dk>]

>>>>> "Henrik" == Henrik Rydberg <rydberg@euromail.se> writes:

Hi Henrik,

 >> 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.

 >> @@ -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).

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.

Returning -EINVAL on something that can never work seems like the sanest
option.

-- 
Bye, Peter Korsgaard