Re: [PATCHv2] evdev: fix evdev_write return value on partial writes
From: Henrik Rydberg <hidden>
Date: 2011-01-27 11:47:03
>> 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 [off-list ref] 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