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: <1424099430.6633.18.camel@linux-0dmf.site>
Date:	Mon, 16 Feb 2015 16:10:30 +0100
From:	Oliver Neukum <oneukum@...e.de>
To:	曾婷葳 "(tammy_tseng)" 
	<tammy_tseng@....com>
Cc:	Dmitry Torokhov <dmitry.torokhov@...il.com>,
	lkml <linux-kernel@...r.kernel.org>, linux-input@...r.kernel.org,
	tammy0524@...il.com
Subject: Re: [PATCH 2/2] INPUT/HID: add touch support for SiS touch driver

On Thu, 2015-02-12 at 15:24 +0800, 曾婷葳 (tammy_tseng) wrote:

> Sorry. Re-send the code diff again.
> Here is the hid touch driver for sis touch controller.
> Thanks.

This driver does very strange things. It looks like you are
simulating a disconnect() to the usbhid driver currently driving
the device. This is unacceptable. Please add the device to the
generic blacklist and implement a clean open/close.

	Regards
		Oliver

> Tammy
> -----
> 
> linux-3.18.5/drivers/hid/Kconfig             |  14 ++
> linux-3.18.5/drivers/hid/hid-ids.h           |   1 +
> linux-3.18.5/drivers/hid/hid-multitouch.c    |  16 ++
> linux-3.18.5/drivers/hid/hid-sis_ctrl.c      | 360 +++++++++++++++++++++++++++
> linux-3.18.5/drivers/hid/usbhid/hid-quirks.c |   1 +
> 5 files changed, 392 insertions(+)
> 
> 
> -----
> diff --git a/linux-3.18.5/drivers/hid/Kconfig b/linux-3.18.5/drivers/hid/Kconfig
> index f42df4d..2235cfe 100644
> --- a/linux-3.18.5/drivers/hid/Kconfig
> +++ b/linux-3.18.5/drivers/hid/Kconfig
> @@ -496,6 +496,20 @@ config HID_MULTITOUCH
>         To compile this driver as a module, choose M here: the
>         module will be called hid-multitouch.
> 
> +config HID_SIS_CTRL
> +      bool "SiS Touch Device Controller"
> +      depends on HID_MULTITOUCH
> +      ---help---
> +      Support for SiS Touch devices update FW.
> +
> +config DEBUG_HID_SIS_UPDATE_FW
> +      bool "SiS Touch device debug message(update firmware)"
> +      depends on HID_SIS_CTRL
> +      default n
> +      ---help---
> +        Say Y here if you want to enable debug message(update firmware) for SiS Touch 
> +          devices. Must enable config HID_SIS_UPDATE_FW first.
> +
> config HID_NTRIG
>       tristate "N-Trig touch screen"
>       depends on USB_HID
> diff --git a/linux-3.18.5/drivers/hid/hid-ids.h b/linux-3.18.5/drivers/hid/hid-ids.h
> index 0e28190..b9ca441 100644
> --- a/linux-3.18.5/drivers/hid/hid-ids.h
> +++ b/linux-3.18.5/drivers/hid/hid-ids.h
> @@ -809,6 +809,7 @@
> #define USB_VENDOR_ID_SIS_TOUCH                0x0457
> #define USB_DEVICE_ID_SIS9200_TOUCH  0x9200
> #define USB_DEVICE_ID_SIS817_TOUCH    0x0817
> +#define USB_DEVICE_ID_SISF817_TOUCH  0xF817
> #define USB_DEVICE_ID_SIS_TS          0x1013
> #define USB_DEVICE_ID_SIS1030_TOUCH  0x1030
> 
> diff --git a/linux-3.18.5/drivers/hid/hid-multitouch.c b/linux-3.18.5/drivers/hid/hid-multitouch.c
> index 51e25b9..11d67bc 100644
> --- a/linux-3.18.5/drivers/hid/hid-multitouch.c
> +++ b/linux-3.18.5/drivers/hid/hid-multitouch.c
> @@ -54,6 +54,10 @@ MODULE_LICENSE("GPL");
> 
>  #include "hid-ids.h"
> 
> +#ifdef CONFIG_HID_SIS_CTRL
> +#include "hid-sis_ctrl.c"
> +#endif
> +
> /* quirks to control the device */
> #define MT_QUIRK_NOT_SEEN_MEANS_UP      (1 << 0)
> #define MT_QUIRK_SLOT_IS_CONTACTID   (1 << 1)
> @@ -951,6 +955,14 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
>               }
>       }
> 
> +#ifdef CONFIG_HID_SIS_CTRL
> +      if (hdev->vendor == USB_VENDOR_ID_SIS_TOUCH) {
> +              ret = sis_setup_chardev(hdev);
> +              if (ret)
> +                      dev_err(&hdev->dev, "sis_setup_chardev fail\n");
> +      }
> +#endif
> +
>       /* This allows the driver to correctly support devices
>        * that emit events over several HID messages.
>        */
> @@ -1043,6 +1055,10 @@ static void mt_remove(struct hid_device *hdev) {
>       sysfs_remove_group(&hdev->dev.kobj, &mt_attribute_group);
>       hid_hw_stop(hdev);
> +#ifdef CONFIG_HID_SIS_CTRL
> +      if (hdev->vendor == USB_VENDOR_ID_SIS_TOUCH)
> +              sis_deinit_chardev();
> +#endif
> }
> 
>  /*
> diff --git a/linux-3.18.5/drivers/hid/hid-sis_ctrl.c b/linux-3.18.5/drivers/hid/hid-sis_ctrl.c
> new file mode 100644
> index 0000000..3a7b3df
> --- /dev/null
> +++ b/linux-3.18.5/drivers/hid/hid-sis_ctrl.c
> @@ -0,0 +1,360 @@
> +/*
> + *  Character device driver for SIS multitouch panels control
> + *
> + *  Copyright (c) 2009 SIS, Ltd.
> + *
> + */
> +
> +/*
> + * This program is free software; you can redistribute it and/or modify 
> +it
> + * under the terms of the GNU General Public License as published by 
> +the Free
> + * Software Foundation; either version 2 of the License, or (at your 
> +option)
> + * any later version.
> + */
> +
> +#include <linux/hid.h>
> +#include <linux/module.h>
> +#include <linux/usb.h>
> +#include "usbhid/usbhid.h"
> +#include <linux/init.h>
> +
> +/*update FW*/
> +#include <linux/fs.h>
> +#include <linux/cdev.h>
> +/*#include <asm/uaccess.h>*/   /*copy_from_user() & copy_to_user()*/
> +#include <linux/uaccess.h>
> +
> +#include "hid-ids.h"
> +
> +#define SIS817_DEVICE_NAME "sis_aegis_hid_touch_device"
> +#define SISF817_DEVICE_NAME "sis_aegis_hid_bridge_touch_device"
> +
> +#define CTRL 0
> +#define ENDP_01 1
> +#define ENDP_02 2
> +#define DIR_IN 0x1
> +
> +#define SIS_ERR_ALLOCATE_KERNEL_MEM        -12 /* Allocate memory fail */
> +#define SIS_ERR_COPY_FROM_USER          -14 /* Copy data from user fail */
> +#define SIS_ERR_COPY_FROM_KERNEL      -19 /* Copy data from kernel fail */

You _must_ use the symbolic names. You cannot use numbers.

> +
> +static const int sis_char_devs_count = 1;           /* device count */
> +static int sis_char_major;
> +static struct cdev sis_char_cdev;
> +static struct class *sis_char_class;
> +
> +static struct hid_device *hid_dev_backup;        /*backup address*/
> +static struct urb *backup_urb;
> +
> +#ifdef CONFIG_DEBUG_HID_SIS_UPDATE_FW
> +      #define DBG_FW(fmt, arg...) printk(fmt, ##arg)
> +      static void sis_dbg_dump_array(u8 *ptr, u32 size)
> +      {
> +              u32 i;
> +
> +              for (i = 0; i < size; i++) {
> +                      DBG_FW("%02X ", ptr[i]);
> +                      if (((i+1)&0xF) == 0)
> +                              DBG_FW("\n");
> +              }
> +              if (size & 0xF)
> +                      DBG_FW("\n");
> +      }
> +#else
> +      #define DBG_FW(...)
> +      #define sis_dbg_dump_array(...)
> +#endif   /*CONFIG_DEBUG_HID_SIS_UPDATE_FW*/
> +
> +/*20120306 Yuger ioctl for tool*/
> +static int sis_cdev_open(struct inode *inode, struct file *filp) {
> +      struct usbhid_device *usbhid;
> +
> +      DBG_FW("%s\n" , __func__);
> +      /*20110511, Yuger, kill current urb by method of usbhid_stop*/
> +      if (!hid_dev_backup) {

Why is this needed?

> +              pr_info("(stop)hid_dev_backup is not initialized yet");
> +              return -1;
> +      }
> +
> +      usbhid = hid_dev_backup->driver_data;
> +
> +      pr_info("sys_sis_HID_stop\n");
> +
> +      /* printk( KERN_INFO "hid_dev_backup->vendor,
> +      * hid_dev_backup->product = %x %x\n", hid_dev_backup->vendor,
> +      * hid_dev_backup->product );*/
> +
> +      /*20110602, Yuger, fix bug: not contact usb cause kernel panic*/
> +      if (!usbhid) {
> +              pr_info("(stop)usbhid is not initialized yet");
> +              return -1;
> +      } else if (!usbhid->urbin) {
> +              pr_info("(stop)usbhid->urbin is not initialized yet");
> +              return -1;
> +      } else if (hid_dev_backup->vendor == USB_VENDOR_ID_SIS_TOUCH) {
> +              usb_fill_int_urb(backup_urb,
> +                      usbhid->urbin->dev,
> +                      usbhid->urbin->pipe,
> +                      usbhid->urbin->transfer_buffer,
> +                      usbhid->urbin->transfer_buffer_length,
> +                      usbhid->urbin->complete,
> +                      usbhid->urbin->context,
> +                      usbhid->urbin->interval);
> +              clear_bit(HID_STARTED, &usbhid->iofl);
> +              set_bit(HID_DISCONNECTED, &usbhid->iofl);
> +
> +              usb_kill_urb(usbhid->urbin);
> +              usb_free_urb(usbhid->urbin);
> +              usbhid->urbin = NULL;
> +              return 0;
> +      }
> +
> +      pr_info("This is not a SiS device");
> +      return -801;
> +}
> +
> +static int sis_cdev_release(struct inode *inode, struct file *filp) {
> +      /*20110505, Yuger, rebuild the urb which is at the same urb address,
> +      * then re-submit it*/
> +
> +      int ret;
> +      struct usbhid_device *usbhid;
> +      unsigned long flags;
> +
> +      DBG_FW("%s: ", __func__);
> +
> +      if (!hid_dev_backup) {
> +              pr_info("(stop)hid_dev_backup is not initialized yet");
> +              return -1;
> +      }
> +
> +      usbhid = hid_dev_backup->driver_data;
> +
> +      pr_info("sys_sis_HID_start");
> +
> +      if (!usbhid) {
> +              pr_info("(start)usbhid is not initialized yet");
> +              return -1;
> +      }
> +
> +      if (!backup_urb) {
> +              pr_info("(start)backup_urb is not initialized yet");
> +              return -1;
> +      }
> +
> +      clear_bit(HID_DISCONNECTED, &usbhid->iofl);
> +      usbhid->urbin = usb_alloc_urb(0, GFP_KERNEL);
> +
> +      if (!backup_urb->interval) {
> +              pr_info("(start)backup_urb->interval does not exist");
> +              return -1;
> +      }
> +
> +      usb_fill_int_urb(usbhid->urbin, backup_urb->dev, backup_urb->pipe,
> +      backup_urb->transfer_buffer, backup_urb->transfer_buffer_length,
> +      backup_urb->complete, backup_urb->context, backup_urb->interval);
> +      usbhid->urbin->transfer_dma = usbhid->inbuf_dma;
> +      usbhid->urbin->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
> +
> +      set_bit(HID_STARTED, &usbhid->iofl);
> +
> +      /*method at hid_start_in*/
> +      spin_lock_irqsave(&usbhid->lock, flags);
> +      ret = usb_submit_urb(usbhid->urbin, GFP_ATOMIC);
> +      spin_unlock_irqrestore(&usbhid->lock, flags);
> +      /*yy*/
> +
> +      DBG_FW("ret = %d", ret);
> +
> +      return ret;
> +}
> +
> +/*SiS 817 only*/
> +static ssize_t sis_cdev_read(struct file *file, char __user *buf,
> +                              size_t count, loff_t *ppos) {
> +      int actual_length = 0;
> +      u8 *rep_data = NULL;
> +      u32 size, timeout;
> +      long rep_ret = 0;
> +      struct usb_interface *intf =
> +                      to_usb_interface(hid_dev_backup->dev.parent);
> +      struct usb_device *dev = interface_to_usbdev(intf);
> +
> +      DBG_FW("%s\n", __func__);
> +
> +      rep_data = kzalloc(count, GFP_KERNEL);
> +      if (!rep_data)
> +              return SIS_ERR_ALLOCATE_KERNEL_MEM;
> +
> +      if (copy_from_user(rep_data, (void *)buf, count)) {
> +              pr_err("copy_to_user fail\n");
> +              rep_ret = SIS_ERR_COPY_FROM_USER;
> +              goto end;
> +      }
> +      size = be32_to_cpup((u32 *)&rep_data[64]);
> +      timeout = be32_to_cpup((u32 *)&rep_data[68]);
> +
> +      rep_ret = usb_interrupt_msg(dev, backup_urb->pipe,
> +              rep_data, size, &actual_length, timeout);
> +
> +      DBG_FW("%s: rep_data = ", __func__);
> +      sis_dbg_dump_array(rep_data, 8);
> +
> +      if (rep_ret == 0) {
> +              rep_ret = actual_length;
> +              if (copy_to_user((void *)buf, rep_data, rep_ret)) {
> +                      pr_err("copy_to_user fail\n");
> +                      rep_ret = SIS_ERR_COPY_FROM_KERNEL;
> +                      goto end;
> +              }
> +      }
> +      DBG_FW("%s: rep_ret = %ld\n", __func__, rep_ret);
> +end:
> +      /*free allocated data*/
> +      kfree(rep_data);
> +      return rep_ret;
> +}
> +
> +static ssize_t sis_cdev_write(struct file *file, const char __user *buf,
> +                      size_t count, loff_t *f_pos) {
> +      int actual_length = 0;
> +      u8 *rep_data = NULL;
> +      long rep_ret = 0;
> +      struct usb_interface *intf =
> +                      to_usb_interface(hid_dev_backup->dev.parent);
> +      struct usb_device *dev = interface_to_usbdev(intf);
> +      struct usbhid_device *usbhid = hid_dev_backup->driver_data;
> +      u32 size, timeout;
> +
> +      rep_data = kzalloc(count, GFP_KERNEL);
> +      if (!rep_data)
> +              return SIS_ERR_ALLOCATE_KERNEL_MEM;
> +
> +      if (copy_from_user(rep_data, (void *)buf, count)) {
> +              pr_err("copy_to_user fail\n");
> +              rep_ret = SIS_ERR_COPY_FROM_USER;
> +              goto end;
> +      }
> +
> +      size = be32_to_cpup((u32 *)&rep_data[64]);
> +      timeout = be32_to_cpup((u32 *)&rep_data[68]);
> +
> +      rep_ret = usb_interrupt_msg(dev, usbhid->urbout->pipe,
> +              rep_data, size, &actual_length, timeout);
> +
> +      DBG_FW("%s: rep_data = ", __func__);
> +      sis_dbg_dump_array(rep_data, size);
> +
> +      if (rep_ret == 0) {
> +              rep_ret = actual_length;
> +              if (copy_to_user((void *)buf, rep_data, rep_ret)) {
> +                      pr_err("copy_to_user fail\n");
> +                      rep_ret = SIS_ERR_COPY_FROM_KERNEL;
> +                      goto end;
> +              }
> +      }
> +      DBG_FW("%s: rep_ret = %ld\n", __func__, rep_ret);
> +      DBG_FW("End of sys_sis_HID_IO\n");
> +end:
> +      /*free allocated data*/
> +      kfree(rep_data);
> +      return rep_ret;
> +}
> +
> +
> +/*for ioctl*/
> +static const struct file_operations sis_cdev_fops = {
> +      .owner    = THIS_MODULE,
> +      .read       = sis_cdev_read,
> +      .write       = sis_cdev_write,
> +      .open      = sis_cdev_open,
> +      .release = sis_cdev_release,
> +};
> +
> +/*for ioctl*/
> +static int sis_setup_chardev(struct hid_device *hdev) {
> +      dev_t dev = MKDEV(sis_char_major, 0);
> +      int ret = 0;
> +      struct device *class_dev = NULL;
> +      u8 *name = (hdev->product == USB_DEVICE_ID_SISF817_TOUCH) ?
> +                      SISF817_DEVICE_NAME : SIS817_DEVICE_NAME;
> +
> +      dev_info(&hdev->dev, "sis_setup_chardev.\n");
> +
> +      backup_urb = usb_alloc_urb(0, GFP_KERNEL);    /*0721test*/
> +      if (!backup_urb) {
> +              dev_err(&hdev->dev, "cannot allocate backup_urb\n");
> +              return -ENOMEM;
> +      }
> +      hid_dev_backup = hdev;
> +      /*dynamic allocate driver handle*/
> +      ret = alloc_chrdev_region(&dev, 0, sis_char_devs_count, name);
> +      if (ret)
> +              goto err1;
> +
> +      sis_char_major = MAJOR(dev);
> +      cdev_init(&sis_char_cdev, &sis_cdev_fops);
> +      sis_char_cdev.owner = THIS_MODULE;
> +      ret = cdev_add(&sis_char_cdev, dev, sis_char_devs_count);
> +      if (ret)
> +              goto err2;
> +
> +      dev_info(&hdev->dev, "%s driver(major %d) installed.\n",
> +                                      name, sis_char_major);
> +
> +      /*register class*/
> +      sis_char_class = class_create(THIS_MODULE, name);
> +      if (IS_ERR(sis_char_class)) {
> +              ret = PTR_ERR(sis_char_class);
> +              goto err3;
> +      }
> +
> +      class_dev = device_create(sis_char_class, NULL, dev, NULL, name);
> +      if (IS_ERR(class_dev)) {
> +              ret = PTR_ERR(class_dev);
> +              goto err4;
> +      }
> +
> +      return 0;
> +err4:
> +      class_destroy(sis_char_class);
> +      sis_char_class = NULL;
> +err3:
> +      cdev_del(&sis_char_cdev);
> +      memset(&sis_char_cdev, 0, sizeof(struct cdev));
> +err2:
> +      sis_char_major = 0;
> +      unregister_chrdev_region(dev, sis_char_devs_count);
> +err1:
> +      usb_kill_urb(backup_urb);
> +      usb_free_urb(backup_urb);
> +      backup_urb = NULL;
> +      hid_dev_backup = NULL;
> +      return ret;
> +}
> +
> +/*for ioctl*/
> +static void sis_deinit_chardev(void)
> +{
> +      dev_t dev = MKDEV(sis_char_major, 0);
> +
> +      if (hid_dev_backup) {
> +              dev_info(&hid_dev_backup->dev, "sis_remove\n");
> +              device_destroy(sis_char_class, dev);
> +              class_destroy(sis_char_class);
> +              sis_char_class = NULL;
> +              cdev_del(&sis_char_cdev);
> +              memset(&sis_char_cdev, 0, sizeof(struct cdev));
> +              sis_char_major = 0;
> +              unregister_chrdev_region(dev, sis_char_devs_count);
> +              usb_kill_urb(backup_urb);
> +              usb_free_urb(backup_urb);
> +              backup_urb = NULL;
> +              hid_dev_backup = NULL;
> +      }
> +}
> diff --git a/linux-3.18.5/drivers/hid/usbhid/hid-quirks.c b/linux-3.18.5/drivers/hid/usbhid/hid-quirks.c
> index 4477eb7..b534321 100644
> --- a/linux-3.18.5/drivers/hid/usbhid/hid-quirks.c
> +++ b/linux-3.18.5/drivers/hid/usbhid/hid-quirks.c
> @@ -97,6 +97,7 @@ static const struct hid_blacklist {
>       { USB_VENDOR_ID_SIGMATEL, USB_DEVICE_ID_SIGMATEL_STMP3780, HID_QUIRK_NOGET },
>       { USB_VENDOR_ID_SIS_TOUCH, USB_DEVICE_ID_SIS9200_TOUCH, HID_QUIRK_NOGET },
>       { USB_VENDOR_ID_SIS_TOUCH, USB_DEVICE_ID_SIS817_TOUCH, HID_QUIRK_NOGET },
> +      { USB_VENDOR_ID_SIS_TOUCH, USB_DEVICE_ID_SISF817_TOUCH, 
> + HID_QUIRK_NOGET },
>       { USB_VENDOR_ID_SIS_TOUCH, USB_DEVICE_ID_SIS_TS, HID_QUIRK_NO_INIT_REPORTS },
>       { USB_VENDOR_ID_SIS_TOUCH, USB_DEVICE_ID_SIS1030_TOUCH, HID_QUIRK_NOGET },
>       { USB_VENDOR_ID_SUN, USB_DEVICE_ID_RARITAN_KVM_DONGLE, HID_QUIRK_NOGET },


-- 
Oliver Neukum <oneukum@...e.de>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ