On Fri, 2015-06-26 at 16:05 -0700, Dmitry Torokhov wrote:
> Hi Stephen,
>
> On Fri, Jun 26, 2015 at 06:28:18PM -0400, cpaul@xxxxxxxxxx wrote:
> > From: Stephen Chandler Paul <cpaul@xxxxxxxxxx>
> >
> > A big problem with the current i8042 debugging option is that it
> > outputs
> > data going to and from the keyboard by default. As a result, many
> > dmesg
> > logs uploaded by users will unintentionally contain sensitive
> > information
> > such as their password, as such it's probably a good idea not to
> > output
> > data coming from the keyboard unless specifically enabled by the
> > user.
> >
> > Signed-off-by: Stephen Chandler Paul <cpaul@xxxxxxxxxx>
> > ---
> > That patch was not working at all like I thought it was, the
> > results on the
> > machine I tested it on were very misleading so I apologize for any
> > trouble that
> > may have caused! I've done more thorough testing of this, and it
> > should work
> > perfectly on any machine.
> >
> > Changes
> > * Bus structs are shared between devices, whoops. Now when the
> > notifier goes
> > off we check two things:
> > * That the device actually belongs to the i8042 platform
> > driver. This is
> > something that's difficult to reproduce, since devices on the
> > i8042 bus
> > are usually handled by the plaform driver. If they don't
> > happen to be
> > however (which currently I've only seen with ps2emu, a kernel
> > module I'm
> > working on to replay PS/2 devices in the kernel), then we
> > make the false
> > assumption the port_data variable points to an i8042_port
> > struct, and
> > very likely crash the machine or worse.
> > * That the IRQ of the port is equivalent to that of the KBD
> > port, so that
> > we only mask the interrupts coming from the keyboard.
> >
> > Documentation/kernel-parameters.txt | 3 ++
> > drivers/input/serio/i8042.c | 64
> > +++++++++++++++++++++++++++++++++----
> > drivers/input/serio/i8042.h | 13 ++++++++
> > 3 files changed, 74 insertions(+), 6 deletions(-)
> >
> > diff --git a/Documentation/kernel-parameters.txt
> > b/Documentation/kernel-parameters.txt
> > index ae44749..a9d2b19 100644
> > --- a/Documentation/kernel-parameters.txt
> > +++ b/Documentation/kernel-parameters.txt
> > @@ -1304,6 +1304,9 @@ bytes respectively. Such letter suffixes can
> > also be entirely omitted.
> > <bus_id>,<clkrate>
> >
> > i8042.debug [HW] Toggle i8042 debug mode
> > + i8042.debug_kbd [HW] Enable printing of interrupt data
> > from the KBD port
> > + (disabled by default, requires that
> > i8042.debug=1
> > + be enabled)
> > i8042.direct [HW] Put keyboard port into non-translated
> > mode
> > i8042.dumbkbd [HW] Pretend that controller can only read
> > data from
> > keyboard and cannot control its state
> > diff --git a/drivers/input/serio/i8042.c
> > b/drivers/input/serio/i8042.c
> > index cb5ece7..0e17bdd 100644
> > --- a/drivers/input/serio/i8042.c
> > +++ b/drivers/input/serio/i8042.c
> > @@ -88,6 +88,10 @@ MODULE_PARM_DESC(nopnp, "Do not use PNP to
> > detect controller settings");
> > static bool i8042_debug;
> > module_param_named(debug, i8042_debug, bool, 0600);
> > MODULE_PARM_DESC(debug, "Turn i8042 debugging mode on and off");
> > +
> > +static bool i8042_debug_kbd;
> > +module_param_named(debug_kbd, i8042_debug_kbd, bool, 0600);
> > +MODULE_PARM_DESC(i8042_kbd, "Turn i8042 kbd debugging output on or
> > off (requires i8042.debug=1)");
> > #endif
> >
> > static bool i8042_bypass_aux_irq_test;
> > @@ -116,6 +120,9 @@ struct i8042_port {
> > struct serio *serio;
> > int irq;
> > bool exists;
> > +#ifdef DEBUG
> > + bool filter_dbg;
> > +#endif
>
> Could we call it "driver_bound" and drop #ifdef DEBUG? In fact, I'd
> rager we dropped the #ifdef DEBUG arounf all bus notifier code.
>
> > signed char mux;
> > };
> >
> > @@ -133,6 +140,9 @@ static bool i8042_kbd_irq_registered;
> > static bool i8042_aux_irq_registered;
> > static unsigned char i8042_suppress_kbd_ack;
> > static struct platform_device *i8042_platform_device;
> > +#ifdef DEBUG
> > +static struct notifier_block i8042_kbd_bind_notifier_block;
> > +#endif
> >
> > static irqreturn_t i8042_interrupt(int irq, void *dev_id);
> > static bool (*i8042_platform_filter)(unsigned char data, unsigned
> > char str,
> > @@ -528,10 +538,10 @@ static irqreturn_t i8042_interrupt(int irq,
> > void *dev_id)
> > port = &i8042_ports[port_no];
> > serio = port->exists ? port->serio : NULL;
> >
> > - dbg("%02x <- i8042 (interrupt, %d, %d%s%s)\n",
> > - data, port_no, irq,
> > - dfl & SERIO_PARITY ? ", bad parity" : "",
> > - dfl & SERIO_TIMEOUT ? ", timeout" : "");
> > + filter_dbg(port->filter_dbg, data, "<- i8042 (interrupt,
> > %d, %d%s%s)\n",
> > + port_no, irq,
> > + dfl & SERIO_PARITY ? ", bad parity" : "",
> > + dfl & SERIO_TIMEOUT ? ", timeout" : "");
> >
> > filtered = i8042_filter(data, str, serio);
> >
> > @@ -1329,6 +1339,13 @@ static void __init
> > i8042_register_ports(void)
> > i8042_ports[i].irq);
> > serio_register_port(serio);
> > device_set_wakeup_capable(&serio->dev,
> > true);
> > +#ifdef DEBUG
> > + if (i == I8042_KBD_PORT_NO) {
> > + bus_register_notifier(
> > + serio->dev.bus,
> > + &i8042_kbd_bind_notifier_b
> > lock);
> > + }
>
>
> Umm, let's export serio_bus and register the notifier when we load
> the
> module.
>
Sorry, but would you mind clarifying what you mean by "export
serio_bus"?
Cheers,
Stephen Chandler Paul
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
