Thread (3 messages) 3 messages, 3 authors, 2011-01-27

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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help