| Message ID | 1435260310-10074-1-git-send-email-cpaul@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, 2015-06-25 at 15:25 -0400, cpaul@redhat.com wrote: > From: Stephen Chandler Paul <cpaul@redhat.com> > > 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. Seems sensible. Coupla trivial bits: > diff --git a/drivers/input/serio/i8042.c b/drivers/input/serio/i8042.c [] > @@ -88,6 +88,23 @@ 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"); It's unclear this depends on i8042_debug. Maybe add something like " (enabling requires i8042_debug=1)" > +#define str_dbg(str, data, format, args...) \ > + do { \ > + if (!i8042_debug) \ > + break; \ > + \ > + if (str & I8042_STR_AUXDATA || i8042_debug_kbd) \ > + dbg("%02x " format, data, ##args); \ > + else \ > + dbg("** " format, ##args); \ > + } while (0) > +#else > +#define str_dbg(str, data, format, args...) do { } while (0) > #endif This should probably go into i8042.h where the dbg macro is #defined -- 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
On Thu, Jun 25, 2015 at 03:25:10PM -0400, cpaul@redhat.com wrote: > From: Stephen Chandler Paul <cpaul@redhat.com> > > 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@redhat.com> > Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> > --- > Documentation/kernel-parameters.txt | 7 +++++++ > drivers/input/serio/i8042.c | 25 +++++++++++++++++++++---- > 2 files changed, 28 insertions(+), 4 deletions(-) > > diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt > index ae44749..9e00234 100644 > --- a/Documentation/kernel-parameters.txt > +++ b/Documentation/kernel-parameters.txt > @@ -1304,6 +1304,13 @@ 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 > + As a side effect, this option will mask some of the > + interrupts sent back from the keyboard during the > + initialization of the KBD port on the i8042, if you > + need to see this, you will need to enable this > + option. Hmm, can we maybe use the bus notifier and react to BUS_NOTIFY_BOUND_DRIVER/BUS_NOTIFY_UNBIND_DRIVER to decide if we want to see keyboard data stream? Thanks.
On Thu, 2015-06-25 at 13:32 -0700, Dmitry Torokhov wrote: > On Thu, Jun 25, 2015 at 03:25:10PM -0400, cpaul@redhat.com wrote: > > From: Stephen Chandler Paul <cpaul@redhat.com> > > > > 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@redhat.com> > > Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> > > --- > > Documentation/kernel-parameters.txt | 7 +++++++ > > drivers/input/serio/i8042.c | 25 +++++++++++++++++++++---- > > 2 files changed, 28 insertions(+), 4 deletions(-) > > > > diff --git a/Documentation/kernel-parameters.txt > > b/Documentation/kernel-parameters.txt > > index ae44749..9e00234 100644 > > --- a/Documentation/kernel-parameters.txt > > +++ b/Documentation/kernel-parameters.txt > > @@ -1304,6 +1304,13 @@ 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 > > + As a side effect, this option will > > mask some of the > > + interrupts sent back from the > > keyboard during the > > + initialization of the KBD port on the > > i8042, if you > > + need to see this, you will need to > > enable this > > + option. > > Hmm, can we maybe use the bus notifier and react to > BUS_NOTIFY_BOUND_DRIVER/BUS_NOTIFY_UNBIND_DRIVER to decide if we want > to > see keyboard data stream? Out of curiosity, are there devices that aren't keyboards that actually make use of the KBD port? It was my understanding keyboards used the KBD port, and everything else uses the AUX port. Regardless, I'm looking into doing this as we speak. Cheers, Stephen Chandler Paul > > Thanks. > -- 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
On Thu, Jun 25, 2015 at 05:31:25PM -0400, Stephen Chandler Paul wrote: > On Thu, 2015-06-25 at 13:32 -0700, Dmitry Torokhov wrote: > > On Thu, Jun 25, 2015 at 03:25:10PM -0400, cpaul@redhat.com wrote: > > > From: Stephen Chandler Paul <cpaul@redhat.com> > > > > > > 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@redhat.com> > > > Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> > > > --- > > > Documentation/kernel-parameters.txt | 7 +++++++ > > > drivers/input/serio/i8042.c | 25 +++++++++++++++++++++---- > > > 2 files changed, 28 insertions(+), 4 deletions(-) > > > > > > diff --git a/Documentation/kernel-parameters.txt > > > b/Documentation/kernel-parameters.txt > > > index ae44749..9e00234 100644 > > > --- a/Documentation/kernel-parameters.txt > > > +++ b/Documentation/kernel-parameters.txt > > > @@ -1304,6 +1304,13 @@ 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 > > > + As a side effect, this option will > > > mask some of the > > > + interrupts sent back from the > > > keyboard during the > > > + initialization of the KBD port on the > > > i8042, if you > > > + need to see this, you will need to > > > enable this > > > + option. > > > > Hmm, can we maybe use the bus notifier and react to > > BUS_NOTIFY_BOUND_DRIVER/BUS_NOTIFY_UNBIND_DRIVER to decide if we want > > to > > see keyboard data stream? > Out of curiosity, are there devices that aren't keyboards that actually > make use of the KBD port? It was my understanding keyboards used the > KBD port, and everything else uses the AUX port. On desktops if you plug keyboard into mouse port and mouse into keyboard port there is a chance they will work (depends on the BIOS). But I was not talking about supporting that necessarily, but use bus notifiers to allow seeing KBC port stream until the driver is fully bound and when it is to be unbound from the KBC port (whatever driver that might be). Thanks.
On Thu, 2015-06-25 at 14:35 -0700, Dmitry Torokhov wrote: > On Thu, Jun 25, 2015 at 05:31:25PM -0400, Stephen Chandler Paul > wrote: > > On Thu, 2015-06-25 at 13:32 -0700, Dmitry Torokhov wrote: > > > On Thu, Jun 25, 2015 at 03:25:10PM -0400, cpaul@redhat.com wrote: > > > > From: Stephen Chandler Paul <cpaul@redhat.com> > > > > > > > > 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@redhat.com> > > > > Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> > > > > --- > > > > Documentation/kernel-parameters.txt | 7 +++++++ > > > > drivers/input/serio/i8042.c | 25 +++++++++++++++++++++ > > > > ---- > > > > 2 files changed, 28 insertions(+), 4 deletions(-) > > > > > > > > diff --git a/Documentation/kernel-parameters.txt > > > > b/Documentation/kernel-parameters.txt > > > > index ae44749..9e00234 100644 > > > > --- a/Documentation/kernel-parameters.txt > > > > +++ b/Documentation/kernel-parameters.txt > > > > @@ -1304,6 +1304,13 @@ 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 > > > > + As a side effect, this option > > > > will > > > > mask some of the > > > > + interrupts sent back from the > > > > keyboard during the > > > > + initialization of the KBD port on > > > > the > > > > i8042, if you > > > > + need to see this, you will need > > > > to > > > > enable this > > > > + option. > > > > > > Hmm, can we maybe use the bus notifier and react to > > > BUS_NOTIFY_BOUND_DRIVER/BUS_NOTIFY_UNBIND_DRIVER to decide if we > > > want > > > to > > > see keyboard data stream? > > Out of curiosity, are there devices that aren't keyboards that > > actually > > make use of the KBD port? It was my understanding keyboards used > > the > > KBD port, and everything else uses the AUX port. > > On desktops if you plug keyboard into mouse port and mouse into > keyboard > port there is a chance they will work (depends on the BIOS). But I > was > not talking about supporting that necessarily, but use bus notifiers > to > allow seeing KBC port stream until the driver is fully bound and when > it > is to be unbound from the KBC port (whatever driver that might be). Alright, I'm following you now. This is definitely doable, we don't need it for our use case (this is mostly just to stop people from accidentally giving their passwords to us), but I'll be happy to add it and get back to you with the new version of the patch when I'm finished. Cheers, Stephen Chandler Paul > > Thanks. > -- 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
diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt index ae44749..9e00234 100644 --- a/Documentation/kernel-parameters.txt +++ b/Documentation/kernel-parameters.txt @@ -1304,6 +1304,13 @@ 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 + As a side effect, this option will mask some of the + interrupts sent back from the keyboard during the + initialization of the KBD port on the i8042, if you + need to see this, you will need to enable this + option. + (disabled by default) 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..084fc05 100644 --- a/drivers/input/serio/i8042.c +++ b/drivers/input/serio/i8042.c @@ -88,6 +88,23 @@ 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"); + +#define str_dbg(str, data, format, args...) \ + do { \ + if (!i8042_debug) \ + break; \ + \ + if (str & I8042_STR_AUXDATA || i8042_debug_kbd) \ + dbg("%02x " format, data, ##args); \ + else \ + dbg("** " format, ##args); \ + } while (0) +#else +#define str_dbg(str, data, format, args...) do { } while (0) #endif static bool i8042_bypass_aux_irq_test; @@ -528,10 +545,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" : ""); + str_dbg(str, 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);