lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAKdAkRS7-hrnMn-ibWT_+1eM859GuPZAg=fSqGQrY8bT7zC+rw@mail.gmail.com>
Date:   Thu, 18 Aug 2016 09:29:37 -0700
From:   Dmitry Torokhov <dmitry.torokhov@...il.com>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     lkml <linux-kernel@...r.kernel.org>,
        "3.8+" <stable@...r.kernel.org>, Mark Laws <mdl@...z.org>
Subject: Re: [PATCH 4.7 172/186] Input: i8042 - break load dependency between
 atkbd/psmouse and i8042

On Thu, Aug 18, 2016 at 6:59 AM, Greg Kroah-Hartman
<gregkh@...uxfoundation.org> wrote:
> 4.7-stable review patch.  If anyone has any objections, please let me know.

Greg, please hold this patch off till your next cycle. I will be
sending a fixup for it to Linus today or tomorrow.

Thanks!

>
> ------------------
>
> From: Dmitry Torokhov <dmitry.torokhov@...il.com>
>
> commit 4097461897df91041382ff6fcd2bfa7ee6b2448c upstream.
>
> As explained in 1407814240-4275-1-git-send-email-decui@...rosoft.com we
> have a hard load dependency between i8042 and atkbd which prevents
> keyboard from working on Gen2 Hyper-V VMs.
>
>> hyperv_keyboard invokes serio_interrupt(), which needs a valid serio
>> driver like atkbd.c.  atkbd.c depends on libps2.c because it invokes
>> ps2_command().  libps2.c depends on i8042.c because it invokes
>> i8042_check_port_owner().  As a result, hyperv_keyboard actually
>> depends on i8042.c.
>>
>> For a Generation 2 Hyper-V VM (meaning no i8042 device emulated), if a
>> Linux VM (like Arch Linux) happens to configure CONFIG_SERIO_I8042=m
>> rather than =y, atkbd.ko can't load because i8042.ko can't load(due to
>> no i8042 device emulated) and finally hyperv_keyboard can't work and
>> the user can't input: https://bugs.archlinux.org/task/39820
>> (Ubuntu/RHEL/SUSE aren't affected since they use CONFIG_SERIO_I8042=y)
>
> To break the dependency we move away from using i8042_check_port_owner()
> and instead allow serio port owner specify a mutex that clients should use
> to serialize PS/2 command stream.
>
> Reported-by: Mark Laws <mdl@...z.org>
> Tested-by: Mark Laws <mdl@...z.org>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@...il.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
>
> ---
>  drivers/input/serio/i8042.c  |   16 +---------------
>  drivers/input/serio/libps2.c |   10 ++++------
>  include/linux/i8042.h        |    6 ------
>  include/linux/serio.h        |   24 +++++++++++++++++++-----
>  4 files changed, 24 insertions(+), 32 deletions(-)
>
> --- a/drivers/input/serio/i8042.c
> +++ b/drivers/input/serio/i8042.c
> @@ -1277,6 +1277,7 @@ static int __init i8042_create_kbd_port(
>         serio->start            = i8042_start;
>         serio->stop             = i8042_stop;
>         serio->close            = i8042_port_close;
> +       serio->ps2_cmd_mutex    = &i8042_mutex;
>         serio->port_data        = port;
>         serio->dev.parent       = &i8042_platform_device->dev;
>         strlcpy(serio->name, "i8042 KBD port", sizeof(serio->name));
> @@ -1373,21 +1374,6 @@ static void i8042_unregister_ports(void)
>         }
>  }
>
> -/*
> - * Checks whether port belongs to i8042 controller.
> - */
> -bool i8042_check_port_owner(const struct serio *port)
> -{
> -       int i;
> -
> -       for (i = 0; i < I8042_NUM_PORTS; i++)
> -               if (i8042_ports[i].serio == port)
> -                       return true;
> -
> -       return false;
> -}
> -EXPORT_SYMBOL(i8042_check_port_owner);
> -
>  static void i8042_free_irqs(void)
>  {
>         if (i8042_aux_irq_registered)
> --- a/drivers/input/serio/libps2.c
> +++ b/drivers/input/serio/libps2.c
> @@ -56,19 +56,17 @@ EXPORT_SYMBOL(ps2_sendbyte);
>
>  void ps2_begin_command(struct ps2dev *ps2dev)
>  {
> -       mutex_lock(&ps2dev->cmd_mutex);
> +       struct mutex *m = ps2dev->serio->ps2_cmd_mutex ?: &ps2dev->cmd_mutex;
>
> -       if (i8042_check_port_owner(ps2dev->serio))
> -               i8042_lock_chip();
> +       mutex_lock(m);
>  }
>  EXPORT_SYMBOL(ps2_begin_command);
>
>  void ps2_end_command(struct ps2dev *ps2dev)
>  {
> -       if (i8042_check_port_owner(ps2dev->serio))
> -               i8042_unlock_chip();
> +       struct mutex *m = ps2dev->serio->ps2_cmd_mutex ?: &ps2dev->cmd_mutex;
>
> -       mutex_unlock(&ps2dev->cmd_mutex);
> +       mutex_unlock(m);
>  }
>  EXPORT_SYMBOL(ps2_end_command);
>
> --- a/include/linux/i8042.h
> +++ b/include/linux/i8042.h
> @@ -62,7 +62,6 @@ struct serio;
>  void i8042_lock_chip(void);
>  void i8042_unlock_chip(void);
>  int i8042_command(unsigned char *param, int command);
> -bool i8042_check_port_owner(const struct serio *);
>  int i8042_install_filter(bool (*filter)(unsigned char data, unsigned char str,
>                                         struct serio *serio));
>  int i8042_remove_filter(bool (*filter)(unsigned char data, unsigned char str,
> @@ -83,11 +82,6 @@ static inline int i8042_command(unsigned
>         return -ENODEV;
>  }
>
> -static inline bool i8042_check_port_owner(const struct serio *serio)
> -{
> -       return false;
> -}
> -
>  static inline int i8042_install_filter(bool (*filter)(unsigned char data, unsigned char str,
>                                         struct serio *serio))
>  {
> --- a/include/linux/serio.h
> +++ b/include/linux/serio.h
> @@ -31,7 +31,8 @@ struct serio {
>
>         struct serio_device_id id;
>
> -       spinlock_t lock;                /* protects critical sections from port's interrupt handler */
> +       /* Protects critical sections from port's interrupt handler */
> +       spinlock_t lock;
>
>         int (*write)(struct serio *, unsigned char);
>         int (*open)(struct serio *);
> @@ -40,16 +41,29 @@ struct serio {
>         void (*stop)(struct serio *);
>
>         struct serio *parent;
> -       struct list_head child_node;    /* Entry in parent->children list */
> +       /* Entry in parent->children list */
> +       struct list_head child_node;
>         struct list_head children;
> -       unsigned int depth;             /* level of nesting in serio hierarchy */
> +       /* Level of nesting in serio hierarchy */
> +       unsigned int depth;
>
> -       struct serio_driver *drv;       /* accessed from interrupt, must be protected by serio->lock and serio->sem */
> -       struct mutex drv_mutex;         /* protects serio->drv so attributes can pin driver */
> +       /*
> +        * serio->drv is accessed from interrupt handlers; when modifying
> +        * caller should acquire serio->drv_mutex and serio->lock.
> +        */
> +       struct serio_driver *drv;
> +       /* Protects serio->drv so attributes can pin current driver */
> +       struct mutex drv_mutex;
>
>         struct device dev;
>
>         struct list_head node;
> +
> +       /*
> +        * For use by PS/2 layer when several ports share hardware and
> +        * may get indigestion when exposed to concurrent access (i8042).
> +        */
> +       struct mutex *ps2_cmd_mutex;
>  };
>  #define to_serio_port(d)       container_of(d, struct serio, dev)
>
>
>



-- 
Dmitry

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ