Re: [patch 03/11] input: new force feedback interface

Andrew Morton
2006-05-25T14:00:58Z

Anssi Hannula <anssi@mandriva.org> wrote:
>
> >>+int input_ff_erase(struct input_dev *dev, int id)
> >>+{
> >>+ struct ff_device *ff;
> >>+ unsigned long flags = 0;
> >>+ int ret;
> >>+ if (!test_bit(EV_FF, dev->evbit))
> >>+ return -EINVAL;
> >>+ mutex_lock(&dev->ff_lock);
> >>+ ff = dev->ff;
> >>+ if (!ff) {
> >>+ mutex_unlock(&dev->ff_lock);
> >>+ return -ENODEV;
> >>+ }
> >>+ spin_ff_cond_lock(ff, flags);
> >>+ ret = _input_ff_erase(dev, id, current->pid == 0);
> >>+ spin_ff_cond_unlock(ff, flags);
> >>+
> >>+ mutex_unlock(&dev->ff_lock);
> >>+ return ret;
> >>+}
> >
> >
> > Perhaps you meant `current->uid == 0' here. There's no way in which pid
> > 0 will call this code.
>
> Right, a silly mistake.
>
> > What's happening here anyway? Why does this code need to know about pids?
> >
> > Checking for uid==0 woud be a fishy thing to do as well.
>
> User ID 0 is allowed to delete effects of other users. Pids are used to
> keep a track of what process owns what effects. This is the same
> behaviour as before.

Oh dear.

Whatever we do here should remain 100%-compatible with "before". Which
rather limits our options.

> There is a problem with this, though:
> When a process closes any fd to this device, all pid-matching effects
> are deleted whether the process has another fd using the device or not.
>
> One solution would probably be to add some handle parameter to
> input_ff_upload() and input_ff_erase(), and then in
> evdev_ioctl_handler() pass an id unique to this fd. Then effects would
> be fd-specific, not pid-specific. I think the uid == 0 thing can also be
> dropped... I don't think the root user needs ability to override user
> effects (it can delete them anyway, just kill the user process owning
> the effects).
>

Generally we use file descriptors (and driver-specific state at
file.f_private) to manage things like that. But I'd imagine that we
couldn't retain the existing semantics with any such scheme.

A pragmatic approach would be to put a big fat comment in there explaining
how it all works and leave it at that.