Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1547144 > unrolled thread

[RFC PATCH] input: Add disable sysfs entry for every input device

Back to article view | Back to linux.kernel

Contents

  [RFC PATCH] input: Add disable sysfs entry for every input device Pali Rohár <pali.rohar@gmail.com> - 2016-12-25 11:10 +0100
    Re: [RFC] input: Add disable sysfs entry for every input device Nikita Yushchenko <nikita.yoush@cogentembedded.com> - 2016-12-30 17:20 +0100
    Re: [RFC PATCH] input: Add disable sysfs entry for every input  device Bastien Nocera <hadess@hadess.net> - 2017-01-02 16:30 +0100
      Re: [RFC PATCH] input: Add disable sysfs entry for every input device Pali Rohár <pali.rohar@gmail.com> - 2017-01-02 18:20 +0100
        Re: [RFC PATCH] input: Add disable sysfs entry for every input  device Bastien Nocera <hadess@hadess.net> - 2017-01-03 12:40 +0100
    Re: [RFC PATCH] input: Add disable sysfs entry for every input device David Herrmann <dh.herrmann@gmail.com> - 2017-01-02 17:50 +0100
      Re: [RFC PATCH] input: Add disable sysfs entry for every input device Pali Rohár <pali.rohar@gmail.com> - 2017-01-02 18:40 +0100

#1547144 — [RFC PATCH] input: Add disable sysfs entry for every input device

This patch allows user to disable events from any input device so events
would not be delivered to userspace.

Currently there is no way to disable particular input device by kernel.
User for different reasons would need it for integrated PS/2 keyboard or
touchpad in notebook or touchscreen on mobile device to prevent sending
events. E.g. mobile phone in pocket or broken integrated PS/2 keyboard.

This is just a RFC patch, not tested yet. Original post about motivation
about this patch is there: https://lkml.org/lkml/2014/11/29/92

Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
---
 drivers/input/input.c |   35 +++++++++++++++++++++++++++++++++++
 include/linux/input.h |    4 ++++
 2 files changed, 39 insertions(+)

diff --git a/drivers/input/input.c b/drivers/input/input.c
index d95c34e..9f0da7e 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -430,6 +430,9 @@ void input_event(struct input_dev *dev,
 {
 	unsigned long flags;
 
+	if (unlikely(dev->disabled))
+		return;
+
 	if (is_event_supported(type, dev->evbit, EV_MAX)) {
 
 		spin_lock_irqsave(&dev->event_lock, flags);
@@ -457,6 +460,9 @@ void input_inject_event(struct input_handle *handle,
 	struct input_handle *grab;
 	unsigned long flags;
 
+	if (unlikely(dev->disabled))
+		return;
+
 	if (is_event_supported(type, dev->evbit, EV_MAX)) {
 		spin_lock_irqsave(&dev->event_lock, flags);
 
@@ -1389,12 +1395,41 @@ static ssize_t input_dev_show_properties(struct device *dev,
 }
 static DEVICE_ATTR(properties, S_IRUGO, input_dev_show_properties, NULL);
 
+static ssize_t input_dev_show_disable(struct device *dev,
+				      struct device_attribute *attr,
+				      char *buf)
+{
+	struct input_dev *input_dev = to_input_dev(dev);
+
+	return snprintf(buf, PAGE_SIZE, "%d\n", input_dev->disabled ? 1 : 0);
+}
+static ssize_t input_dev_store_disable(struct device *dev,
+				       struct device_attribute *attr,
+				       const char *buf, size_t count)
+{
+	struct input_dev *input_dev = to_input_dev(dev);
+	int disable;
+	int ret;
+
+	ret = kstrtoint(buf, 0, &disable);
+	if (ret)
+		return ret;
+
+	if (disable != 0 && disable != 1)
+		return -EINVAL;
+
+	input_dev->disabled = disable;
+	return count;
+}
+static DEVICE_ATTR(disable, S_IRUGO | S_IWUSR, input_dev_show_disable, input_dev_store_disable);
+
 static struct attribute *input_dev_attrs[] = {
 	&dev_attr_name.attr,
 	&dev_attr_phys.attr,
 	&dev_attr_uniq.attr,
 	&dev_attr_modalias.attr,
 	&dev_attr_properties.attr,
+	&dev_arrr_disable.attr,
 	NULL
 };
 
diff --git a/include/linux/input.h b/include/linux/input.h
index a65e3b2..e390b56 100644
--- a/include/linux/input.h
+++ b/include/linux/input.h
@@ -117,6 +117,8 @@ struct input_value {
  * @vals: array of values queued in the current frame
  * @devres_managed: indicates that devices is managed with devres framework
  *	and needs not be explicitly unregistered or freed.
+ * @disabled: indicates that device is in disabled state and kernel drop
+ *	all events from it
  */
 struct input_dev {
 	const char *name;
@@ -187,6 +189,8 @@ struct input_dev {
 	struct input_value *vals;
 
 	bool devres_managed;
+
+	bool disabled;
 };
 #define to_input_dev(d) container_of(d, struct input_dev, dev)
 
-- 
1.7.9.5

[toc] | [next] | [standalone]

#1548654 — Re: [RFC] input: Add disable sysfs entry for every input device

Hi

>  	&dev_attr_properties.attr,
> +	&dev_arrr_disable.attr,

Typo here.

After fixing that,

Tested-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com>

[toc] | [prev] | [next] | [standalone]

#1549247 — Re: [RFC PATCH] input: Add disable sysfs entry for every input device

On Sun, 2016-12-25 at 11:04 +0100, Pali Rohár wrote:
> This patch allows user to disable events from any input device so
> events
> would not be delivered to userspace.
> 
> Currently there is no way to disable particular input device by
> kernel.
> User for different reasons would need it for integrated PS/2 keyboard
> or
> touchpad in notebook or touchscreen on mobile device to prevent
> sending
> events. E.g. mobile phone in pocket or broken integrated PS/2
> keyboard.
> 
> This is just a RFC patch, not tested yet. Original post about
> motivation
> about this patch is there: https://lkml.org/lkml/2014/11/29/92

Having implemented something of that ilk in user-space (we
automatically disable touch devices when the associated screen is
turned off/suspended), I think this might need more thought.

What happens when a device is opened and the device disabled through
sysfs, are the users revoked?

Does this put the device in suspend in the same way that closing the
device's last user does?

Is this not better implemented in user-space at the session level,
where it knows about which output corresponds to which input device?

Is this useful enough to disable misbehaving devices on hardware, so
that the device is not effective on boot?

> 
> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> ---
>  drivers/input/input.c |   35 +++++++++++++++++++++++++++++++++++
>  include/linux/input.h |    4 ++++
>  2 files changed, 39 insertions(+)
> 
> diff --git a/drivers/input/input.c b/drivers/input/input.c
> index d95c34e..9f0da7e 100644
> --- a/drivers/input/input.c
> +++ b/drivers/input/input.c
> @@ -430,6 +430,9 @@ void input_event(struct input_dev *dev,
>  {
>  	unsigned long flags;
>  
> +	if (unlikely(dev->disabled))
> +		return;
> +
>  	if (is_event_supported(type, dev->evbit, EV_MAX)) {
>  
>  		spin_lock_irqsave(&dev->event_lock, flags);
> @@ -457,6 +460,9 @@ void input_inject_event(struct input_handle
> *handle,
>  	struct input_handle *grab;
>  	unsigned long flags;
>  
> +	if (unlikely(dev->disabled))
> +		return;
> +
>  	if (is_event_supported(type, dev->evbit, EV_MAX)) {
>  		spin_lock_irqsave(&dev->event_lock, flags);
>  
> @@ -1389,12 +1395,41 @@ static ssize_t
> input_dev_show_properties(struct device *dev,
>  }
>  static DEVICE_ATTR(properties, S_IRUGO, input_dev_show_properties,
> NULL);
>  
> +static ssize_t input_dev_show_disable(struct device *dev,
> +				      struct device_attribute *attr,
> +				      char *buf)
> +{
> +	struct input_dev *input_dev = to_input_dev(dev);
> +
> +	return snprintf(buf, PAGE_SIZE, "%d\n", input_dev->disabled
> ? 1 : 0);
> +}
> +static ssize_t input_dev_store_disable(struct device *dev,
> +				       struct device_attribute
> *attr,
> +				       const char *buf, size_t
> count)
> +{
> +	struct input_dev *input_dev = to_input_dev(dev);
> +	int disable;
> +	int ret;
> +
> +	ret = kstrtoint(buf, 0, &disable);
> +	if (ret)
> +		return ret;
> +
> +	if (disable != 0 && disable != 1)
> +		return -EINVAL;
> +
> +	input_dev->disabled = disable;
> +	return count;
> +}
> +static DEVICE_ATTR(disable, S_IRUGO | S_IWUSR,
> input_dev_show_disable, input_dev_store_disable);
> +
>  static struct attribute *input_dev_attrs[] = {
>  	&dev_attr_name.attr,
>  	&dev_attr_phys.attr,
>  	&dev_attr_uniq.attr,
>  	&dev_attr_modalias.attr,
>  	&dev_attr_properties.attr,
> +	&dev_arrr_disable.attr,
>  	NULL
>  };
>  
> diff --git a/include/linux/input.h b/include/linux/input.h
> index a65e3b2..e390b56 100644
> --- a/include/linux/input.h
> +++ b/include/linux/input.h
> @@ -117,6 +117,8 @@ struct input_value {
>   * @vals: array of values queued in the current frame
>   * @devres_managed: indicates that devices is managed with devres
> framework
>   *	and needs not be explicitly unregistered or freed.
> + * @disabled: indicates that device is in disabled state and kernel
> drop
> + *	all events from it
>   */
>  struct input_dev {
>  	const char *name;
> @@ -187,6 +189,8 @@ struct input_dev {
>  	struct input_value *vals;
>  
>  	bool devres_managed;
> +
> +	bool disabled;
>  };
>  #define to_input_dev(d) container_of(d, struct input_dev, dev)
>  

[toc] | [prev] | [next] | [standalone]

#1549307

[Multipart message — attachments visible in raw view] — view raw

On Monday 02 January 2017 16:27:05 Bastien Nocera wrote:
> On Sun, 2016-12-25 at 11:04 +0100, Pali Rohár wrote:
> > This patch allows user to disable events from any input device so
> > events
> > would not be delivered to userspace.
> > 
> > Currently there is no way to disable particular input device by
> > kernel.
> > User for different reasons would need it for integrated PS/2
> > keyboard or
> > touchpad in notebook or touchscreen on mobile device to prevent
> > sending
> > events. E.g. mobile phone in pocket or broken integrated PS/2
> > keyboard.
> > 
> > This is just a RFC patch, not tested yet. Original post about
> > motivation
> > about this patch is there: https://lkml.org/lkml/2014/11/29/92
> 
> Having implemented something of that ilk in user-space (we
> automatically disable touch devices when the associated screen is
> turned off/suspended), I think this might need more thought.

How to implement such thing in userspace? I think you cannot do that 
without rewriting every one userspace application which uses input.

> What happens when a device is opened and the device disabled through
> sysfs, are the users revoked?

Applications will not receive events. Same as if input device does not 
generates events.

> Does this put the device in suspend in the same way that closing the
> device's last user does?

Current code not (this is just RFC prototype), but it should be possible 
to implement.

> Is this not better implemented in user-space at the session level,
> where it knows about which output corresponds to which input device?

How to do that without rewriting existing applications?

> Is this useful enough to disable misbehaving devices on hardware, so
> that the device is not effective on boot?  

In case integrated device is absolutely unusable and generates always 
random events, it does not solve problem at boot time.

But more real case is laptop with closed LID press buttons and here it 
is useful.

-- 
Pali Rohár
pali.rohar@gmail.com

[toc] | [prev] | [next] | [standalone]

#1549670 — Re: [RFC PATCH] input: Add disable sysfs entry for every input device

On Mon, 2017-01-02 at 18:09 +0100, Pali Rohár wrote:
> On Monday 02 January 2017 16:27:05 Bastien Nocera wrote:
> > On Sun, 2016-12-25 at 11:04 +0100, Pali Rohár wrote:
> > > This patch allows user to disable events from any input device so
> > > events
> > > would not be delivered to userspace.
> > > 
> > > Currently there is no way to disable particular input device by
> > > kernel.
> > > User for different reasons would need it for integrated PS/2
> > > keyboard or
> > > touchpad in notebook or touchscreen on mobile device to prevent
> > > sending
> > > events. E.g. mobile phone in pocket or broken integrated PS/2
> > > keyboard.
> > > 
> > > This is just a RFC patch, not tested yet. Original post about
> > > motivation
> > > about this patch is there: https://lkml.org/lkml/2014/11/29/92
> > 
> > Having implemented something of that ilk in user-space (we
> > automatically disable touch devices when the associated screen is
> > turned off/suspended), I think this might need more thought.
> 
> How to implement such thing in userspace? I think you cannot do that 
> without rewriting every one userspace application which uses input.
> 
> > What happens when a device is opened and the device disabled
> through
> > sysfs, are the users revoked?
> 
> Applications will not receive events. Same as if input device does
> not 
> generates events.
> 
> > Does this put the device in suspend in the same way that closing
> the
> > device's last user does?
> 
> Current code not (this is just RFC prototype), but it should be
> possible 
> to implement.
> 
> > Is this not better implemented in user-space at the session level,
> > where it knows about which output corresponds to which input
> device?
> 
> How to do that without rewriting existing applications?
> 
> > Is this useful enough to disable misbehaving devices on hardware,
> so
> > that the device is not effective on boot?  
> 
> In case integrated device is absolutely unusable and generates
> always 
> random events, it does not solve problem at boot time.
> 
> But more real case is laptop with closed LID press buttons and here
> it 
> is useful.

There's usually a display manager in between the application and the
input device. Whether it's X.org, or a Wayland compositor. Even David's
 https://github.com/dvdhrm/kmscon could help for console applications.

[toc] | [prev] | [next] | [standalone]

#1549293

Hi

On Sun, Dec 25, 2016 at 11:04 AM, Pali Rohár <pali.rohar@gmail.com> wrote:
> This patch allows user to disable events from any input device so events
> would not be delivered to userspace.
>
> Currently there is no way to disable particular input device by kernel.
> User for different reasons would need it for integrated PS/2 keyboard or
> touchpad in notebook or touchscreen on mobile device to prevent sending
> events. E.g. mobile phone in pocket or broken integrated PS/2 keyboard.
>
> This is just a RFC patch, not tested yet. Original post about motivation
> about this patch is there: https://lkml.org/lkml/2014/11/29/92
>
> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> ---
>  drivers/input/input.c |   35 +++++++++++++++++++++++++++++++++++
>  include/linux/input.h |    4 ++++
>  2 files changed, 39 insertions(+)

Don't open the device, if you don't want events from it. Really.

I assume the reason behind this is that you don't know how to make
your user-space ignore devices. But with this patch in place, you now
end up with user-space trying to use the device, but not getting any
events. Anyone trying to debug this will go nuts because the setup
looks like the device is used, but ends up being muted.

I strongly recommend improving your user-space code to do what you
want it to do (meaning, make your user-space be configurable, if you
need this).

Btw., as a workaround, you can always disable input-drivers that you
don't want. Or you can run EVIOCGRAB on a device to get the same
effect as your patch.

Thanks
David

> diff --git a/drivers/input/input.c b/drivers/input/input.c
> index d95c34e..9f0da7e 100644
> --- a/drivers/input/input.c
> +++ b/drivers/input/input.c
> @@ -430,6 +430,9 @@ void input_event(struct input_dev *dev,
>  {
>         unsigned long flags;
>
> +       if (unlikely(dev->disabled))
> +               return;
> +
>         if (is_event_supported(type, dev->evbit, EV_MAX)) {
>
>                 spin_lock_irqsave(&dev->event_lock, flags);
> @@ -457,6 +460,9 @@ void input_inject_event(struct input_handle *handle,
>         struct input_handle *grab;
>         unsigned long flags;
>
> +       if (unlikely(dev->disabled))
> +               return;
> +
>         if (is_event_supported(type, dev->evbit, EV_MAX)) {
>                 spin_lock_irqsave(&dev->event_lock, flags);
>
> @@ -1389,12 +1395,41 @@ static ssize_t input_dev_show_properties(struct device *dev,
>  }
>  static DEVICE_ATTR(properties, S_IRUGO, input_dev_show_properties, NULL);
>
> +static ssize_t input_dev_show_disable(struct device *dev,
> +                                     struct device_attribute *attr,
> +                                     char *buf)
> +{
> +       struct input_dev *input_dev = to_input_dev(dev);
> +
> +       return snprintf(buf, PAGE_SIZE, "%d\n", input_dev->disabled ? 1 : 0);
> +}
> +static ssize_t input_dev_store_disable(struct device *dev,
> +                                      struct device_attribute *attr,
> +                                      const char *buf, size_t count)
> +{
> +       struct input_dev *input_dev = to_input_dev(dev);
> +       int disable;
> +       int ret;
> +
> +       ret = kstrtoint(buf, 0, &disable);
> +       if (ret)
> +               return ret;
> +
> +       if (disable != 0 && disable != 1)
> +               return -EINVAL;
> +
> +       input_dev->disabled = disable;
> +       return count;
> +}
> +static DEVICE_ATTR(disable, S_IRUGO | S_IWUSR, input_dev_show_disable, input_dev_store_disable);
> +
>  static struct attribute *input_dev_attrs[] = {
>         &dev_attr_name.attr,
>         &dev_attr_phys.attr,
>         &dev_attr_uniq.attr,
>         &dev_attr_modalias.attr,
>         &dev_attr_properties.attr,
> +       &dev_arrr_disable.attr,
>         NULL
>  };
>
> diff --git a/include/linux/input.h b/include/linux/input.h
> index a65e3b2..e390b56 100644
> --- a/include/linux/input.h
> +++ b/include/linux/input.h
> @@ -117,6 +117,8 @@ struct input_value {
>   * @vals: array of values queued in the current frame
>   * @devres_managed: indicates that devices is managed with devres framework
>   *     and needs not be explicitly unregistered or freed.
> + * @disabled: indicates that device is in disabled state and kernel drop
> + *     all events from it
>   */
>  struct input_dev {
>         const char *name;
> @@ -187,6 +189,8 @@ struct input_dev {
>         struct input_value *vals;
>
>         bool devres_managed;
> +
> +       bool disabled;
>  };
>  #define to_input_dev(d) container_of(d, struct input_dev, dev)
>
> --
> 1.7.9.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[toc] | [prev] | [next] | [standalone]

#1549324

[Multipart message — attachments visible in raw view] — view raw

On Monday 02 January 2017 17:44:38 David Herrmann wrote:
> Don't open the device, if you don't want events from it. Really.

There are existing applications which are doing it. This advice is good 
but not for past.

> I assume the reason behind this is that you don't know how to make
> your user-space ignore devices.

Yes.

And another reason is to disable particular keyboard in linux tty 
console.

> But with this patch in place, you now
> end up with user-space trying to use the device, but not getting any
> events.

Exactly. Same situation as if input device does not generate any event.

> Anyone trying to debug this will go nuts because the setup
> looks like the device is used, but ends up being muted.

Such argument can be used in any situation.

E.g. I own file on disk with executable bit and I cannot execute it. 
Looks like bug, but instead selinux policy.

Proper way is to fully document behaviour and configuration. Situation 
that somebody is something expecting and have not read needed 
documentation and already started something debugging is wrong.

> I strongly recommend improving your user-space code to do what you
> want it to do (meaning, make your user-space be configurable, if you
> need this).

Problem with integrated notebook keyboard which press some buttons in 
linux tty console cannot be fixed in user-space.

But, my original motivation about this disabling input devices is for 
tsc2005 touchscreen on Nokia N900.

There is existing userspace code for Nokia N900, some is closed & 
proprietary (so not possible to modify or change or fix) which already 
expects that kernel provide "disable" sysfs option for tsc2005 
touchscreen. But that sysfs entry was removed in commit 
5cb81d19bae47adcb073a5e5a3bc40dd252f239e and userspace stopped 
working...

Such problems are hard to fix now, but what we can see is that any 
userspace application can open input device and let it open for its own 
and process events which read. It is fully valid code and fully correct.

Just user and admin too cannot force kernel to stop sending events to 
application in specific cases when those events are either invalid or 
nor events which applications expect in current state.

And this is reason why I chose and suggest to have some option which 
"mute" input device. And which should be used when user knows that 
events should not be generated by input kernel driver or when nobody 
should read them.

> Btw., as a workaround, you can always disable input-drivers that you
> don't want.

No, you cannot. If you connect external PS/2 keyboard to notebook with 
broken integrated keyboard, then both devices are handled by one driver.

Also in some cases userspace application may expect that input device 
will exists and would not work without it. E.g. when you want to disable 
input device temporary, just when notebook LID is closed or when screen 
is locked (on touchscreen).

> Or you can run EVIOCGRAB on a device to get the same
> effect as your patch.

Next part is to implement runtime pm or autosuspend of device. So this 
will break pm. And also will break applications which grab device 
itself.

-- 
Pali Rohár
pali.rohar@gmail.com

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web