1. 30 May, 2019 1 commit
  2. 06 May, 2019 1 commit
    • Kirill Smelkov's avatar
      *: convert stream-like files from nonseekable_open -> stream_open · c5bf68fe
      Kirill Smelkov authored
      Using scripts/coccinelle/api/stream_open.cocci added in 10dce8af
      ("fs: stream_open - opener for stream-like files so that read and write
      can run simultaneously without deadlock"), search and convert to
      stream_open all in-kernel nonseekable_open users for which read and
      write actually do not depend on ppos and where there is no other methods
      in file_operations which assume @offset access.
      
      I've verified each generated change manually - that it is correct to convert -
      and each other nonseekable_open instance left - that it is either not correct
      to convert there, or that it is not converted due to current stream_open.cocci
      limitations. The script also does not convert files that should be valid to
      convert, but that currently have .llseek = noop_llseek or generic_file_llseek
      for unknown reason despite file being opened with nonseekable_open (e.g.
      drivers/input/mousedev.c)
      
      Among cases converted 14 were potentially vulnerable to read vs write deadlock
      (see details in 10dce8af):
      
      	drivers/char/pcmcia/cm4000_cs.c:1685:7-23: ERROR: cm4000_fops: .read() can deadlock .write(); change nonseekable_open -> stream_open to fix.
      	drivers/gnss/core.c:45:1-17: ERROR: gnss_fops: .read() can deadlock .write(); change nonseekable_open -> stream_open to fix.
      	drivers/hid/uhid.c:635:1-17: ERROR: uhid_fops: .read() can deadlock .write(); change nonseekable_open -> stream_open to fix.
      	drivers/infiniband/core/user_mad.c:988:1-17: ERROR: umad_fops: .read() can deadlock .write(); change nonseekable_open -> stream_open to fix.
      	drivers/input/evdev.c:527:1-17: ERROR: evdev_fops: .read() can deadlock .write(); change nonseekable_open -> stream_open to fix.
      	drivers/input/misc/uinput.c:401:1-17: ERROR: uinput_fops: .read() can deadlock .write(); change nonseekable_open -> stream_open to fix.
      	drivers/isdn/capi/capi.c:963:8-24: ERROR: capi_fops: .read() can deadlock .write(); change nonseekable_open -> stream_open to fix.
      	drivers/leds/uleds.c:77:1-17: ERROR: uleds_fops: .read() can deadlock .write(); change nonseekable_open -> stream_open to fix.
      	drivers/media/rc/lirc_dev.c:198:1-17: ERROR: lirc_fops: .read() can deadlock .write(); change nonseekable_open -> stream_open to fix.
      	drivers/s390/char/fs3270.c:488:1-17: ERROR: fs3270_fops: .read() can deadlock .write(); change nonseekable_open -> stream_open to fix.
      	drivers/usb/misc/ldusb.c:310:1-17: ERROR: ld_usb_fops: .read() can deadlock .write(); change nonseekable_open -> stream_open to fix.
      	drivers/xen/evtchn.c:667:8-24: ERROR: evtchn_fops: .read() can deadlock .write(); change nonseekable_open -> stream_open to fix.
      	net/batman-adv/icmp_socket.c:80:1-17: ERROR: batadv_fops: .read() can deadlock .write(); change nonseekable_open -> stream_open to fix.
      	net/rfkill/core.c:1146:8-24: ERROR: rfkill_fops: .read() can deadlock .write(); change nonseekable_open -> stream_open to fix.
      
      and the rest were just safe to convert to stream_open because their read and
      write do not use ppos at all and corresponding file_operations do not
      have methods that assume @offset file access(*):
      
      	arch/powerpc/platforms/52xx/mpc52xx_gpt.c:631:8-24: WARNING: mpc52xx_wdt_fops: .write() has stream semantic; safe to change nonseekable_open -> stream_open.
      	arch/powerpc/platforms/cell/spufs/file.c:591:8-24: WARNING: spufs_ibox_fops: .read() has stream semantic; safe to change nonseekable_open -> stream_open.
      	arch/powerpc/platforms/cell/spufs/file.c:591:8-24: WARNING: spufs_ibox_stat_fops: .read() has stream semantic; safe to change nonseekable_open -> stream_open.
      	arch/powerpc/platforms/cell/spufs/file.c:591:8-24: WARNING: spufs_mbox_fops: .read() has stream semantic; safe to change nonseekable_open -> stream_open.
      	arch/powerpc/platforms/cell/spufs/file.c:591:8-24: WARNING: spufs_mbox_stat_fops: .read() has stream semantic; safe to change nonseekable_open -> stream_open.
      	arch/powerpc/platforms/cell/spufs/file.c:591:8-24: WARNING: spufs_wbox_fops: .write() has stream semantic; safe to change nonseekable_open -> stream_open.
      	arch/powerpc/platforms/cell/spufs/file.c:591:8-24: WARNING: spufs_wbox_stat_fops: .read() has stream semantic; safe to change nonseekable_open -> stream_open.
      	arch/um/drivers/harddog_kern.c:88:8-24: WARNING: harddog_fops: .write() has stream semantic; safe to change nonseekable_open -> stream_open.
      	arch/x86/kernel/cpu/microcode/core.c:430:33-49: WARNING: microcode_fops: .write() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/char/ds1620.c:215:8-24: WARNING: ds1620_fops: .read() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/char/dtlk.c:301:1-17: WARNING: dtlk_fops: .read() and .write() have stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/char/ipmi/ipmi_watchdog.c:840:9-25: WARNING: ipmi_wdog_fops: .read() and .write() have stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/char/pcmcia/scr24x_cs.c:95:8-24: WARNING: scr24x_fops: .read() and .write() have stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/char/tb0219.c:246:9-25: WARNING: tb0219_fops: .read() and .write() have stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/firewire/nosy.c:306:8-24: WARNING: nosy_ops: .read() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/hwmon/fschmd.c:840:8-24: WARNING: watchdog_fops: .write() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/hwmon/w83793.c:1344:8-24: WARNING: watchdog_fops: .write() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/infiniband/core/ucma.c:1747:8-24: WARNING: ucma_fops: .write() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/infiniband/core/ucm.c:1178:8-24: WARNING: ucm_fops: .write() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/infiniband/core/uverbs_main.c:1086:8-24: WARNING: uverbs_fops: .write() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/input/joydev.c:282:1-17: WARNING: joydev_fops: .read() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/pci/switch/switchtec.c:393:1-17: WARNING: switchtec_fops: .read() and .write() have stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/platform/chrome/cros_ec_debugfs.c:135:8-24: WARNING: cros_ec_console_log_fops: .read() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/rtc/rtc-ds1374.c:470:9-25: WARNING: ds1374_wdt_fops: .read() and .write() have stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/rtc/rtc-m41t80.c:805:9-25: WARNING: wdt_fops: .read() and .write() have stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/s390/char/tape_char.c:293:2-18: WARNING: tape_fops: .read() and .write() have stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/s390/char/zcore.c:194:8-24: WARNING: zcore_reipl_fops: .write() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/s390/crypto/zcrypt_api.c:528:8-24: WARNING: zcrypt_fops: .read() and .write() have stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/spi/spidev.c:594:1-17: WARNING: spidev_fops: .read() and .write() have stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/staging/pi433/pi433_if.c:974:1-17: WARNING: pi433_fops: .read() and .write() have stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/watchdog/acquirewdt.c:203:8-24: WARNING: acq_fops: .write() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/watchdog/advantechwdt.c:202:8-24: WARNING: advwdt_fops: .write() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/watchdog/alim1535_wdt.c:252:8-24: WARNING: ali_fops: .write() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/watchdog/alim7101_wdt.c:217:8-24: WARNING: wdt_fops: .write() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/watchdog/ar7_wdt.c:166:8-24: WARNING: ar7_wdt_fops: .write() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/watchdog/at91rm9200_wdt.c:113:8-24: WARNING: at91wdt_fops: .write() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/watchdog/ath79_wdt.c:135:8-24: WARNING: ath79_wdt_fops: .write() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/watchdog/bcm63xx_wdt.c:119:8-24: WARNING: bcm63xx_wdt_fops: .write() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/watchdog/cpu5wdt.c:143:8-24: WARNING: cpu5wdt_fops: .write() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/watchdog/cpwd.c:397:8-24: WARNING: cpwd_fops: .read() and .write() have stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/watchdog/eurotechwdt.c:319:8-24: WARNING: eurwdt_fops: .write() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/watchdog/f71808e_wdt.c:528:8-24: WARNING: watchdog_fops: .write() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/watchdog/gef_wdt.c:232:8-24: WARNING: gef_wdt_fops: .write() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/watchdog/geodewdt.c:95:8-24: WARNING: geodewdt_fops: .write() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/watchdog/ib700wdt.c:241:8-24: WARNING: ibwdt_fops: .write() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/watchdog/ibmasr.c:326:8-24: WARNING: asr_fops: .write() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/watchdog/indydog.c:80:8-24: WARNING: indydog_fops: .write() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/watchdog/intel_scu_watchdog.c:307:8-24: WARNING: intel_scu_fops: .write() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/watchdog/iop_wdt.c:104:8-24: WARNING: iop_wdt_fops: .write() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/watchdog/it8712f_wdt.c:330:8-24: WARNING: it8712f_wdt_fops: .write() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/watchdog/ixp4xx_wdt.c:68:8-24: WARNING: ixp4xx_wdt_fops: .write() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/watchdog/ks8695_wdt.c:145:8-24: WARNING: ks8695wdt_fops: .write() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/watchdog/m54xx_wdt.c:88:8-24: WARNING: m54xx_wdt_fops: .write() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/watchdog/machzwd.c:336:8-24: WARNING: zf_fops: .write() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/watchdog/mixcomwd.c:153:8-24: WARNING: mixcomwd_fops: .write() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/watchdog/mtx-1_wdt.c:121:8-24: WARNING: mtx1_wdt_fops: .write() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/watchdog/mv64x60_wdt.c:136:8-24: WARNING: mv64x60_wdt_fops: .write() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/watchdog/nuc900_wdt.c:134:8-24: WARNING: nuc900wdt_fops: .write() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/watchdog/nv_tco.c:164:8-24: WARNING: nv_tco_fops: .write() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/watchdog/pc87413_wdt.c:289:8-24: WARNING: pc87413_fops: .write() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/watchdog/pcwd.c:698:8-24: WARNING: pcwd_fops: .write() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/watchdog/pcwd.c:737:8-24: WARNING: pcwd_temp_fops: .read() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/watchdog/pcwd_pci.c:581:8-24: WARNING: pcipcwd_fops: .write() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/watchdog/pcwd_pci.c:623:8-24: WARNING: pcipcwd_temp_fops: .read() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/watchdog/pcwd_usb.c:488:8-24: WARNING: usb_pcwd_fops: .write() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/watchdog/pcwd_usb.c:527:8-24: WARNING: usb_pcwd_temperature_fops: .read() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/watchdog/pika_wdt.c:121:8-24: WARNING: pikawdt_fops: .write() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/watchdog/pnx833x_wdt.c:119:8-24: WARNING: pnx833x_wdt_fops: .write() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/watchdog/rc32434_wdt.c:153:8-24: WARNING: rc32434_wdt_fops: .write() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/watchdog/rdc321x_wdt.c:145:8-24: WARNING: rdc321x_wdt_fops: .write() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/watchdog/riowd.c:79:1-17: WARNING: riowd_fops: .write() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/watchdog/sa1100_wdt.c:62:8-24: WARNING: sa1100dog_fops: .write() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/watchdog/sbc60xxwdt.c:211:8-24: WARNING: wdt_fops: .write() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/watchdog/sbc7240_wdt.c:139:8-24: WARNING: wdt_fops: .write() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/watchdog/sbc8360.c:274:8-24: WARNING: sbc8360_fops: .write() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/watchdog/sbc_epx_c3.c:81:8-24: WARNING: epx_c3_fops: .write() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/watchdog/sbc_fitpc2_wdt.c:78:8-24: WARNING: fitpc2_wdt_fops: .write() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/watchdog/sb_wdog.c:108:1-17: WARNING: sbwdog_fops: .write() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/watchdog/sc1200wdt.c:181:8-24: WARNING: sc1200wdt_fops: .write() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/watchdog/sc520_wdt.c:261:8-24: WARNING: wdt_fops: .write() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/watchdog/sch311x_wdt.c:319:8-24: WARNING: sch311x_wdt_fops: .write() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/watchdog/scx200_wdt.c:105:8-24: WARNING: scx200_wdt_fops: .write() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/watchdog/smsc37b787_wdt.c:369:8-24: WARNING: wb_smsc_wdt_fops: .write() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/watchdog/w83877f_wdt.c:227:8-24: WARNING: wdt_fops: .write() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/watchdog/w83977f_wdt.c:301:8-24: WARNING: wdt_fops: .write() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/watchdog/wafer5823wdt.c:200:8-24: WARNING: wafwdt_fops: .write() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/watchdog/watchdog_dev.c:828:8-24: WARNING: watchdog_fops: .write() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/watchdog/wdrtas.c:379:8-24: WARNING: wdrtas_fops: .write() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/watchdog/wdrtas.c:445:8-24: WARNING: wdrtas_temp_fops: .read() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/watchdog/wdt285.c:104:1-17: WARNING: watchdog_fops: .write() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/watchdog/wdt977.c:276:8-24: WARNING: wdt977_fops: .write() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/watchdog/wdt.c:424:8-24: WARNING: wdt_fops: .write() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/watchdog/wdt.c:484:8-24: WARNING: wdt_temp_fops: .read() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/watchdog/wdt_pci.c:464:8-24: WARNING: wdtpci_fops: .write() has stream semantic; safe to change nonseekable_open -> stream_open.
      	drivers/watchdog/wdt_pci.c:527:8-24: WARNING: wdtpci_temp_fops: .read() has stream semantic; safe to change nonseekable_open -> stream_open.
      	net/batman-adv/log.c:105:1-17: WARNING: batadv_log_fops: .read() has stream semantic; safe to change nonseekable_open -> stream_open.
      	sound/core/control.c:57:7-23: WARNING: snd_ctl_f_ops: .read() has stream semantic; safe to change nonseekable_open -> stream_open.
      	sound/core/rawmidi.c:385:7-23: WARNING: snd_rawmidi_f_ops: .read() and .write() have stream semantic; safe to change nonseekable_open -> stream_open.
      	sound/core/seq/seq_clientmgr.c:310:7-23: WARNING: snd_seq_f_ops: .read() and .write() have stream semantic; safe to change nonseekable_open -> stream_open.
      	sound/core/timer.c:1428:7-23: WARNING: snd_timer_f_ops: .read() has stream semantic; safe to change nonseekable_open -> stream_open.
      
      One can also recheck/review the patch via generating it with explanation comments included via
      
      	$ make coccicheck MODE=patch COCCI=scripts/coccinelle/api/stream_open.cocci SPFLAGS="-D explain"
      
      (*) This second group also contains cases with read/write deadlocks that
      stream_open.cocci don't yet detect, but which are still valid to convert to
      stream_open since ppos is not used. For example drivers/pci/switch/switchtec.c
      calls wait_for_completion_interruptible() in its .read, but stream_open.cocci
      currently detects only "wait_event*" as blocking.
      
      Cc: Michael Kerrisk <[email protected]>
      Cc: Yongzhi Pan <[email protected]>
      Cc: Jonathan Corbet <[email protected]>
      Cc: David Vrabel <[email protected]>
      Cc: Juergen Gross <[email protected]>
      Cc: Miklos Szeredi <[email protected]>
      Cc: Tejun Heo <[email protected]>
      Cc: Kirill Tkhai <[email protected]>
      Cc: Arnd Bergmann <[email protected]>
      Cc: Christoph Hellwig <[email protected]>
      Cc: Greg Kroah-Hartman <[email protected]>
      Cc: Julia Lawall <[email protected]>
      Cc: Nikolaus Rath <[email protected]>
      Cc: Han-Wen Nienhuys <[email protected]>
      Cc: Anatolij Gustschin <[email protected]>
      Cc: Jeff Dike <[email protected]>
      Cc: Richard Weinberger <[email protected]>
      Cc: Anton Ivanov <[email protected]>
      Cc: Borislav Petkov <[email protected]>
      Cc: Thomas Gleixner <[email protected]>
      Cc: Ingo Molnar <[email protected]>
      Cc: "James R. Van Zandt" <[email protected]>
      Cc: Corey Minyard <[email protected]>
      Cc: Harald Welte <[email protected]>
      Acked-by: Lubomir Rintel <[email protected]> [scr24x_cs]
      Cc: Stefan Richter <[email protected]>
      Cc: Johan Hovold <[email protected]>
      Cc: David Herrmann <[email protected]>
      Cc: Jiri Kosina <[email protected]>
      Cc: Benjamin Tissoires <[email protected]>
      Cc: Jean Delvare <[email protected]>
      Acked-by: Guenter Roeck <[email protected]>	[watchdog/* hwmon/*]
      Cc: Rudolf Marek <[email protected]>
      Cc: Dmitry Torokhov <[email protected]>
      Cc: Karsten Keil <[email protected]>
      Cc: Jacek Anaszewski <[email protected]>
      Cc: Pavel Machek <[email protected]>
      Cc: Mauro Carvalho Chehab <[email protected]>
      Cc: Kurt Schwemmer <[email protected]>
      Acked-by: Logan Gunthorpe <[email protected]> [drivers/pci/switch/switchtec]
      Acked-by: Bjorn Helgaas <[email protected]> [drivers/pci/switch/switchtec]
      Cc: Benson Leung <[email protected]>
      Acked-by: Enric Balletbo i Serra <[email protected]> [platform/chrome]
      Cc: Alessandro Zummo <[email protected]>
      Acked-by: Alexandre Belloni <[email protected]> [rtc/*]
      Cc: Mark Brown <[email protected]>
      Cc: Wim Van Sebroeck <[email protected]>
      Cc: Florian Fainelli <[email protected]>
      Cc: [email protected]
      Cc: Wan ZongShun <[email protected]>
      Cc: Zwane Mwaikambo <[email protected]>
      Cc: Marek Lindner <[email protected]>
      Cc: Simon Wunderlich <[email protected]>
      Cc: Antonio Quartulli <[email protected]>
      Cc: "David S. Miller" <[email protected]>
      Cc: Johannes Berg <[email protected]>
      Cc: Jaroslav Kysela <[email protected]>
      Cc: Takashi Iwai <[email protected]>
      Signed-off-by: Kirill Smelkov's avatarKirill Smelkov <[email protected]>
      c5bf68fe
  3. 19 Nov, 2018 2 commits
    • David Herrmann's avatar
      Revert "HID: uhid: use strlcpy() instead of strncpy()" · 4d26d1d1
      David Herrmann authored
      This reverts commit 336fd4f5.
      
      Please note that `strlcpy()` does *NOT* do what you think it does.
      strlcpy() *ALWAYS* reads the full input string, regardless of the
      'length' parameter. That is, if the input is not zero-terminated,
      strlcpy() will *READ* beyond input boundaries. It does this, because it
      always returns the size it *would* copy if the target was big enough,
      not the truncated size it actually copied.
      
      The original code was perfectly fine. The hid device is
      zero-initialized and the strncpy() functions copied up to n-1
      characters. The result is always zero-terminated this way.
      
      This is the third time someone tried to replace strncpy with strlcpy in
      this function, and gets it wrong. I now added a comment that should at
      least make people reconsider.
      Signed-off-by: David Herrmann's avatarDavid Herrmann <[email protected]>
      Signed-off-by: default avatarJiri Kosina <[email protected]>
      4d26d1d1
    • Eric Biggers's avatar
      HID: uhid: forbid UHID_CREATE under KERNEL_DS or elevated privileges · 8c01db76
      Eric Biggers authored
      When a UHID_CREATE command is written to the uhid char device, a
      copy_from_user() is done from a user pointer embedded in the command.
      When the address limit is KERNEL_DS, e.g. as is the case during
      sys_sendfile(), this can read from kernel memory.  Alternatively,
      information can be leaked from a setuid binary that is tricked to write
      to the file descriptor.  Therefore, forbid UHID_CREATE in these cases.
      
      No other commands in uhid_char_write() are affected by this bug and
      UHID_CREATE is marked as "obsolete", so apply the restriction to
      UHID_CREATE only rather than to uhid_char_write() entirely.
      
      Thanks to Dmitry Vyukov for adding uhid definitions to syzkaller and to
      Jann Horn for commit 9da3f2b7 ("x86/fault: BUG() when uaccess
      helpers fault on kernel addresses"), allowing this bug to be found.
      
      Reported-by: [email protected]
      Fixes: d365c6cf ("HID: uhid: add UHID_CREATE and UHID_DESTROY events")
      Cc: <[email protected]> # v3.6+
      Cc: Jann Horn <[email protected]>
      Cc: Andy Lutomirski <[email protected]>
      Signed-off-by: default avatarEric Biggers <[email protected]>
      Reviewed-by: default avatarJann Horn <[email protected]>
      Signed-off-by: default avatarJiri Kosina <[email protected]>
      8c01db76
  4. 16 Feb, 2018 1 commit
    • Xiongfeng Wang's avatar
      HID: uhid: use strlcpy() instead of strncpy() · 336fd4f5
      Xiongfeng Wang authored
      gcc-8 reports
      
      drivers/hid/uhid.c: In function 'uhid_dev_create2':
      ./include/linux/string.h:245:9: warning: '__builtin_strncpy' output may
      be truncated copying 127 bytes from a string of length 127
      [-Wstringop-truncation]
      
      The compiler require that the input param 'len' of strncpy() should be
      greater than the length of the src string, so that '\0' is copied as
      well. We can just use strlcpy() to avoid this warning.
      Signed-off-by: default avatarXiongfeng Wang <[email protected]>
      Signed-off-by: default avatarJiri Kosina <[email protected]>
      336fd4f5
  5. 11 Feb, 2018 1 commit
    • Linus Torvalds's avatar
      vfs: do bulk POLL* -> EPOLL* replacement · a9a08845
      Linus Torvalds authored
      This is the mindless scripted replacement of kernel use of POLL*
      variables as described by Al, done by this script:
      
          for V in IN OUT PRI ERR RDNORM RDBAND WRNORM WRBAND HUP RDHUP NVAL MSG; do
              L=`git grep -l -w POLL$V | grep -v '^t' | grep -v /um/ | grep -v '^sa' | grep -v '/poll.h$'|grep -v '^D'`
              for f in $L; do sed -i "-es/^\([^\"]*\)\(\<POLL$V\>\)/\\1E\\2/" $f; done
          done
      
      with de-mangling cleanups yet to come.
      
      NOTE! On almost all architectures, the EPOLL* constants have the same
      values as the POLL* constants do.  But they keyword here is "almost".
      For various bad reasons they aren't the same, and epoll() doesn't
      actually work quite correctly in some cases due to this on Sparc et al.
      
      The next patch from Al will sort out the final differences, and we
      should be all done.
      Scripted-by: default avatarAl Viro <[email protected]>
      Signed-off-by: default avatarLinus Torvalds <[email protected]>
      a9a08845
  6. 28 Nov, 2017 1 commit
  7. 27 Jul, 2017 1 commit
    • Jason Gerecke's avatar
      HID: introduce hid_is_using_ll_driver · fc2237a7
      Jason Gerecke authored
      Although HID itself is transport-agnostic, occasionally a driver may
      want to interact with the low-level transport that a device is connected
      through. To do this, we need to know what kind of bus is in use. The
      first guess may be to look at the 'bus' field of the 'struct hid_device',
      but this field may be emulated in some cases (e.g. uhid).
      
      More ideally, we can check which ll_driver a device is using. This
      function introduces a 'hid_is_using_ll_driver' function and makes the
      'struct hid_ll_driver' of the four most common transports accessible
      through hid.h.
      Signed-off-by: default avatarJason Gerecke <[email protected]>
      Acked-By: default avatarBenjamin Tissoires <[email protected]>
      Signed-off-by: default avatarJiri Kosina <[email protected]>
      fc2237a7
  8. 31 Aug, 2016 1 commit
  9. 01 Jun, 2016 1 commit
    • Roderick Colenbrander's avatar
      HID: uhid: fix timeout when probe races with IO · 67f8ecc5
      Roderick Colenbrander authored
      Many devices use userspace bluetooth stacks like BlueZ or Bluedroid in combination
      with uhid. If any of these stacks is used with a HID device for which the driver
      performs a HID request as part .probe (or technically another HID operation),
      this results in a deadlock situation. The deadlock results in a 5 second timeout
      for I/O operations in HID drivers, so isn't fatal, but none of the I/O operations
      have a chance of succeeding.
      
      The root cause for the problem is that uhid only allows for one request to be
      processed at a time per uhid instance and locks out other operations. This means
      that if a user space is creating a new HID device through 'UHID_CREATE', which
      ultimately triggers '.probe' through the HID layer. Then any HID request e.g. a
      read for calibration data would trigger a HID operation on uhid again, but it
      won't go out to userspace, because it is still stuck in UHID_CREATE.
      In addition bluetooth stacks are typically single threaded, so they wouldn't be
      able to handle any requests while waiting on uhid.
      
      Lucikly the UHID spec is somewhat flexible and allows for fixing the issue,
      without breaking user space. The idea which the patch implements as discussed
      with David Herrmann is to decouple adding of a hid device (which triggers .probe)
      from UHID_CREATE. The work will kick off roughly once UHID_CREATE completed (or
      else will wait a tiny bit of time in .probe for a lock). A HID driver has to call
      HID to call 'hid_hw_start()' as part of .probe once it is ready for I/O, which
      triggers UHID_START to user space. Any HID operations should function now within
      .probe and won't deadlock because userspace is stuck on UHID_CREATE.
      
      We verified this patch on Bluedroid with Android 6.0 and on desktop Linux with
      BlueZ stacks. Prior to the patch they had the deadlock issue.
      
      [[email protected]: reword subject]
      Signed-off-by: default avatarRoderick Colenbrander <[email protected]>
      Cc: [email protected]
      Signed-off-by: default avatarJiri Kosina <[email protected]>
      67f8ecc5
  10. 22 Mar, 2016 1 commit
  11. 01 Oct, 2014 1 commit
  12. 25 Aug, 2014 10 commits
    • David Herrmann's avatar
      HID: uhid: report to user-space whether reports are numbered · c2b2f16c
      David Herrmann authored
      This makes UHID_START include a "dev_flags" field that describes details
      of the hid-device in the kernel. The first flags we introduce describe
      whether a given report-type uses numbered reports. This is useful for
      transport layers that force report-numbers and therefore might have to
      prefix kernel-provided HID-messages with the report-number.
      
      Currently, only HoG needs this and the spec only talks about "global
      report numbers". That is, it's a global boolean not a per-type boolean.
      However, given the quirks we already have in kernel-space, a per-type
      value seems much more appropriate.
      Signed-off-by: David Herrmann's avatarDavid Herrmann <[email protected]>
      Signed-off-by: default avatarJiri Kosina <[email protected]>
      c2b2f16c
    • David Herrmann's avatar
      HID: uhid: implement SET_REPORT · 11c22155
      David Herrmann authored
      We so far lacked support for hid_hw_raw_request(..., HID_REQ_SET_REPORT);
      Add support for it and simply forward the request to user-space. Note that
      SET_REPORT is synchronous, just like GET_REPORT, even though it does not
      provide any data back besides an error code.
      
      If a transport layer does SET_REPORT asynchronously, they can just ACK it
      immediately by writing an uhid_set_report_reply to uhid.
      
      This patch re-uses the synchronous uhid-report infrastructure to query
      user-space. Note that this means you cannot run SET_REPORT and GET_REPORT
      in parallel. However, that has always been a restriction of HID and due to
      its blocking nature, this is just fine. Maybe some future transport layer
      supports parallel requests (very unlikely), however, until then lets not
      over-complicate things and avoid request-lookup-tables.
      Signed-off-by: David Herrmann's avatarDavid Herrmann <[email protected]>
      Signed-off-by: default avatarJiri Kosina <[email protected]>
      11c22155
    • David Herrmann's avatar
      HID: uhid: rename uhid_raw_request to uhid_hid_raw_request · 7c4003bc
      David Herrmann authored
      We use strict prefixed in uhid.c:
        uhid_char_*: implement char-dev callbacks
        uhid_dev_*: implement uhid device management and runtime
        uhid_hid_*: implement hid-dev callbacks
      
      uhid_raw_request is an hid callback, so rename it to uhid_hid_raw_request.
      
      While at it, move it closer to it's extracted helpers and keep the same
      order as in "struct hid_driver".
      Signed-off-by: David Herrmann's avatarDavid Herrmann <[email protected]>
      Signed-off-by: default avatarJiri Kosina <[email protected]>
      7c4003bc
    • David Herrmann's avatar
      HID: uhid: add ABI compatible UHID_GET_REPORT replacing UHID_FEATURE · fa71f32b
      David Herrmann authored
      The old hdev->hid_get_raw_report() was broken by design. It was never
      clear what kind of HW request it should trigger. Benjamin fixed that with
      the core HID cleanup, though we never really adjusted uhid.
      
      Unfortunately, our old UHID_FEATURE command was modelled around the broken
      hid_get_raw_report(). We converted it silently to the new GET_REPORT and
      nothing broke. Make this explicit by renaming UHID_FEATURE to
      UHID_GET_REPORT and UHID_FEATURE_ANSWER to UHID_GET_REPORT_REPLY.
      
      Note that this is 100% ABI compatible to UHID_FEATURE. This is just a
      rename. But we have to keep the old definitions around to not break API.
      
      >From now on, UHID_GET_REPORT must trigger a GET_REPORT request on the
      user-space hardware layer. All the ambiguity due to the weird "feature"
      name should be gone now.
      Signed-off-by: David Herrmann's avatarDavid Herrmann <[email protected]>
      Signed-off-by: default avatarJiri Kosina <[email protected]>
      fa71f32b
    • David Herrmann's avatar
      HID: uhid: invert report_done and make non-atomic · 5942b849
      David Herrmann authored
      All accesses to @report_done are protected by qlock (or report-contexts).
      No need to use an atomic.
      
      While at it, invert the logic and call it "report_running". This is
      similar to the uhid->running field and easier to read.
      Signed-off-by: David Herrmann's avatarDavid Herrmann <[email protected]>
      Signed-off-by: default avatarJiri Kosina <[email protected]>
      5942b849
    • David Herrmann's avatar
      HID: uhid: turn report_id into u32 · 8cad5b01
      David Herrmann authored
      All accesses to @report_id are protected by @qlock. No need to use an
      atomic.
      Signed-off-by: David Herrmann's avatarDavid Herrmann <[email protected]>
      Signed-off-by: default avatarJiri Kosina <[email protected]>
      8cad5b01
    • David Herrmann's avatar
      HID: uhid: avoid magic-numbers when setting strings · 25be7fe2
      David Herrmann authored
      Avoid hard-coding the target buffer sizes and use sizeof() instead. This
      also makes us future-proof to buffer-extensions later on.
      Signed-off-by: David Herrmann's avatarDavid Herrmann <[email protected]>
      Signed-off-by: default avatarJiri Kosina <[email protected]>
      25be7fe2
    • David Herrmann's avatar
      HID: uhid: avoid dangling pointers in uhid context · 41c4a464
      David Herrmann authored
      Avoid keeping uhid->rd_data and uhid->rd_size set in case
      uhid_dev_create2() fails. This is non-critical as we never flip
      uhid->running and thus never enter uhid_dev_destroy(). However, it's much
      nicer for debugging if pointers are only set if they point to valid data.
      Signed-off-by: David Herrmann's avatarDavid Herrmann <[email protected]>
      Signed-off-by: default avatarJiri Kosina <[email protected]>
      41c4a464
    • David Herrmann's avatar
      HID: uhid: forward create_req to create2_req · 56c47754
      David Herrmann authored
      Instead of hard-coding the uhid_dev_create() function twice, copy any
      create_req into a create2_req structure and forward it.
      
      We allocate uhid_create_req on the stack here, but that should be fine.
      Unlike uhid_create2_req it is fairly small (<1KB) and it's only used
      temporarily to swap entries. uhid_dev_create2() doesn't access it.
      Signed-off-by: David Herrmann's avatarDavid Herrmann <[email protected]>
      Signed-off-by: default avatarJiri Kosina <[email protected]>
      56c47754
    • David Herrmann's avatar
      HID: uhid: simplify report-cb shutdown · 0e0d7520
      David Herrmann authored
      The report-query is blocking, so when user-space destroys a device we have
      to wake up any blocking kernel context that is currently in the report-cb.
      We used some broken correlation between @report_done and @running so far.
      Replace it by a much more obvious use.
      
      We now wake up the report-cb if either @report_done or @running is set.
      wake_up() and wait_event() serve as implicit barriers (as they always do)
      so no need to use smp_rmb/wmb directly.
      
      Note that @report_done is never reset by anyone but the report-cb, thus
      it cannot flip twice while we wait for it. And whenever we set @running,
      we afterwards synchronously remove the HID device. Therefore, we wait for
      all report-cbs to finish before we return. This way, @running can never
      flip to true while we wait for it.
      Signed-off-by: David Herrmann's avatarDavid Herrmann <[email protected]>
      Signed-off-by: default avatarJiri Kosina <[email protected]>
      0e0d7520
  13. 26 May, 2014 1 commit
  14. 01 Apr, 2014 1 commit
    • Petri Gynther's avatar
      HID: uhid: Add UHID_CREATE2 + UHID_INPUT2 · 4522643a
      Petri Gynther authored
      UHID_CREATE2:
      HID report descriptor data (rd_data) is an array in struct uhid_create2_req,
      instead of a pointer. Enables use from languages that don't support pointers,
      e.g. Python.
      
      UHID_INPUT2:
      Data array is the last field of struct uhid_input2_req. Enables userspace to
      write only the required bytes to kernel (ev.type + ev.u.input2.size + the part
      of the data array that matters), instead of the entire struct uhid_input2_req.
      
      Note:
      UHID_CREATE2 increases the total size of struct uhid_event slightly, thus
      increasing the size of messages that are queued for userspace. However, this
      won't affect the userspace processing of these events.
      
      [Jiri Kosina <[email protected]>: adjust to hid_get_raw_report() and
      				hid_output_raw_report() API changes]
      Signed-off-by: default avatarPetri Gynther <[email protected]>
      Reviewed-by: David Herrmann's avatarDavid Herrmann <[email protected]>
      Signed-off-by: default avatarJiri Kosina <[email protected]>
      4522643a
  15. 14 Mar, 2014 1 commit
  16. 17 Feb, 2014 5 commits
  17. 29 Jan, 2014 1 commit
  18. 27 Nov, 2013 1 commit
  19. 26 Sep, 2013 1 commit
    • David Herrmann's avatar
      HID: uhid: allocate static minor · 19872d20
      David Herrmann authored
      udev has this nice feature of creating "dead" /dev/<node> device-nodes if
      it finds a devnode:<node> modalias. Once the node is accessed, the kernel
      automatically loads the module that provides the node. However, this
      requires udev to know the major:minor code to use for the node. This
      feature was introduced by:
      
        commit 578454ff
        Author: Kay Sievers <[email protected]>
        Date:   Thu May 20 18:07:20 2010 +0200
      
            driver core: add devname module aliases to allow module on-demand auto-loading
      
      However, uhid uses dynamic minor numbers so this doesn't actually work. We
      need to load uhid to know which minor it's going to use.
      
      Hence, allocate a static minor (just like uinput does) and we're good
      to go.
      Reported-by: default avatarTom Gundersen <[email protected]>
      Signed-off-by: David Herrmann's avatarDavid Herrmann <[email protected]>
      Signed-off-by: default avatarJiri Kosina <[email protected]>
      19872d20
  20. 02 Sep, 2013 1 commit
  21. 31 Jul, 2013 1 commit
  22. 18 Feb, 2013 1 commit
  23. 20 Jul, 2012 1 commit
  24. 18 Jun, 2012 3 commits
    • Jiri Kosina's avatar
      HID: uhid: silence gcc warning · 1a8b294c
      Jiri Kosina authored
      gcc is giving me:
      
      drivers/hid/uhid.c: In function ‘uhid_hid_get_raw’:
      drivers/hid/uhid.c:157: warning: ‘len’ may be used uninitialized in this function
      
      which is clearly bogus, as
      
      - when used as memcpy() argument, it's initialized properly
      - the code is structured in a way that either 'ret' or 'len'
        is always initialized, so the return statement always has
        an initialized value.
      Signed-off-by: default avatarJiri Kosina <[email protected]>
      1a8b294c
    • David Herrmann's avatar
      HID: uhid: implement feature requests · fcfcf0de
      David Herrmann authored
      HID standard allows sending a feature request to the device which is
      answered by an HID report. uhid implements this by sending a UHID_FEATURE
      event to user-space which then must answer with UHID_FEATURE_ANSWER. If it
      doesn't do this in a timely manner, the request is discarded silently.
      
      We serialize the feature requests, that is, there is always only a single
      active feature-request sent to user-space, other requests have to wait.
      HIDP and USB-HID do it the same way.
      
      Because we discard feature-requests silently, we must make sure to match
      a response to the corresponding request. We use sequence-IDs for this so
      user-space must copy the ID from the request into the answer.
      Feature-answers are ignored if they do not contain the same ID as the
      currently pending feature request.
      
      Internally, we must make sure that feature-requests are synchronized with
      UHID_DESTROY and close() events. We must not dead-lock when closing the
      HID device, either, so we have to use separate locks.
      Signed-off-by: default avatarDavid Herrmann <[email protected]>
      Signed-off-by: default avatarJiri Kosina <[email protected]>
      fcfcf0de
    • David Herrmann's avatar
      HID: uhid: forward raw output reports to user-space · 3b3baa82
      David Herrmann authored
      Some drivers that use non-standard HID features require raw output reports
      sent to the device. We now forward these requests directly to user-space
      so the transport-level driver can correctly send it to the device or
      handle it correspondingly.
      
      There is no way to signal back whether the transmission was successful,
      moreover, there might be lots of messages coming out from the driver
      flushing the output-queue. However, there is currently no driver that
      causes this so we are safe. If some drivers need to transmit lots of data
      this way, we need a method to synchronize this and can implement another
      UHID_OUTPUT_SYNC event.
      Signed-off-by: default avatarDavid Herrmann <[email protected]>
      Signed-off-by: default avatarJiri Kosina <[email protected]>
      3b3baa82