Doing `ioctl(HIDIOCGFEATURE)` in a tight loop on a hidraw device
and then disconnecting the device, or unloading the driver, can
cause a NULL pointer dereference.
When a hidraw device is destroyed it sets 0 to `dev->exist`.
Most functions check 'dev->exist' before doing its work, but
`hidraw_get_report()` was missing that check.
Cc: stable@vger.kernel.org
Signed-off-by: Rodrigo Rivas Costa <rodrigorivascosta@gmail.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
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: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
We should not try to bring HID device out of full power state before
calling hid_hw_close(), so that transport driver operates on powered up
device (making this inverse of the opening sequence).
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Reviewed-by: Guenter Roeck <groeck@chromium.org>
Reviewed-by: Benson Leung <bleung@chromium.org>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Fix up affected files that include this signal functionality via sched.h.
Acked-by: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
My static checker complains that "devid" can be uninitialized if
alloc_chrdev_region() fails. Fix this by moving the error hanling
forward a couple lines.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Instead of open-coding memory allocation and copying form user memory
sequence let's use memdup_user().
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
hid_out_raw_report is going to be obsoleted as it is not part of the
unified HID low level transport documentation
(Documentation/hid/hid-transport.txt)
To do so, we need to introduce two new quirks:
* HID_QUIRK_NO_OUTPUT_REPORTS_ON_INTR_EP: this quirks prevents the
transport driver to use the interrupt channel to send output report
(and thus force to use HID_REQ_SET_REPORT command)
* HID_QUIRK_SKIP_OUTPUT_REPORT_ID: this one forces usbhid to not
include the report ID in the buffer it sends to the device through
HID_REQ_SET_REPORT in case of an output report
This also fixes a regression introduced in commit 3a75b24949
(HID: hidraw: replace hid_output_raw_report() calls by appropriates ones).
The hidraw API was not able to communicate with the PS3 SixAxis
controllers in USB mode.
Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Tested-by: Antonio Ospite <ao2@ao2.it>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
I noticed that after hot unplugging a Logitech unifying receiver
(drivers/hid/hid-logitech-dj.c) the kernel would occasionally spew a
stack trace similar to this:
usb 1-1.1.2: USB disconnect, device number 7
WARNING: CPU: 0 PID: 2865 at fs/sysfs/group.c:216 device_del+0x40/0x1b0()
sysfs group ffffffff8187fa20 not found for kobject 'hidraw0'
[...]
CPU: 0 PID: 2865 Comm: upowerd Tainted: G W 3.14.0-rc4 #7
Hardware name: LENOVO 7783PN4/ , BIOS 9HKT43AUS 07/11/2011
0000000000000009 ffffffff814cd684 ffff880427ccfdf8 ffffffff810616e7
ffff88041ec61800 ffff880427ccfe48 ffff88041e444d80 ffff880426fab8e8
ffff880429359960 ffffffff8106174c ffffffff81714b98 0000000000000028
Call Trace:
[<ffffffff814cd684>] ? dump_stack+0x41/0x51
[<ffffffff810616e7>] ? warn_slowpath_common+0x77/0x90
[<ffffffff8106174c>] ? warn_slowpath_fmt+0x4c/0x50
[<ffffffff81374fd0>] ? device_del+0x40/0x1b0
[<ffffffff8137516f>] ? device_unregister+0x2f/0x50
[<ffffffff813751fa>] ? device_destroy+0x3a/0x40
[<ffffffffa03ca245>] ? drop_ref+0x55/0x120 [hid]
[<ffffffffa03ca3e6>] ? hidraw_release+0x96/0xb0 [hid]
[<ffffffff811929da>] ? __fput+0xca/0x210
[<ffffffff8107fe17>] ? task_work_run+0x97/0xd0
[<ffffffff810139a9>] ? do_notify_resume+0x69/0xa0
[<ffffffff814dbd22>] ? int_signal+0x12/0x17
---[ end trace 63f4a46f6566d737 ]---
During device removal hid_disconnect() is called via hid_hw_stop() to
stop the device and free all its resources, including the sysfs
files. The problem is that if a user space process, such as upowerd,
holds a reference to a hidraw file the corresponding sysfs files will
be kept around (drop_ref() does not call device_destroy() if the open
counter is not 0) and it will be usb_disconnect() who, by calling
device_del() for the USB device, will indirectly remove the sysfs
files of the hidraw device (sysfs_remove_dir() is recursive these
days). Because of this, by the time user space releases the last
reference to the hidraw file and drop_ref() tries to destroy the
device the sysfs files are already gone and the kernel will print
the warning above.
Fix this by calling device_destroy() at USB disconnect time.
Signed-off-by: Fernando Luis Vazquez Cao <fernando@oss.ntt.co.jp>
Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
Cc: stable@vger.kernel.org # 3.13
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Remove hid_output_raw_report() call as it is not a ll_driver callbacj,
and switch to the hid_hw_* implementation. USB-HID used to fallback
into SET_REPORT when there were no output interrupt endpoint,
so emulating this if hid_hw_output_report() returns -ENOSYS.
Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Add a helper to access hdev->hid_output_raw_report().
To convert the drivers, use the following snippets:
for i in drivers/hid/*.c
do
sed -i.bak "s/[^ \t]*->hid_output_raw_report(/hid_output_raw_report(/g" $i
done
Then manually fix for checkpatch.pl
Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
dev->hid_get_raw_report(X) and hid_hw_raw_request(X, HID_REQ_GET_REPORT)
are strictly equivalent. Switch the hid subsystem to the hid_hw notation
and remove the field .hid_get_raw_report in struct hid_device.
Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Even though device exist bit is set the underlying
HW device should be closed when the last reader
of the device is closed i.e. open count drops to zero.
Signed-off-by: Manoj Chourasia <mchourasia@nvidia.com>
Reported-by: mika.westerberg@linux.intel.com
Tested-by: mika.westerberg@linux.intel.com
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Sync with Linus' tree to be able to apply fixup patch on top
of 9d9a04ee75 ("HID: apple: Add support for the 2013 Macbook Air")
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
It is unsafe to call list_for_each_entry in hidraw_report_event to
traverse each hidraw_list node without a lock protection, the list
could be modified if someone calls hidraw_release and list_del to
remove itself from the list, this can cause hidraw_report_event
to touch a deleted list struct and panic.
To prevent this, introduce a spinlock in struct hidraw to protect
list from concurrent access.
Signed-off-by: Yonghua Zheng <younghua.zheng@gmail.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
This changes puts the commit 4fe9f8e203 back in place
with the fixes for slab corruption because of the commit.
When a device is unplugged, wait for all processes that
have opened the device to close before deallocating the device.
This commit was solving kernel crash because of the corruption in
rb tree of vmalloc. The rootcause was the device data pointer was
geting excessed after the memory associated with hidraw was freed.
The commit 4fe9f8e203 was buggy as it was also freeing the hidraw
first and then calling delete operation on the list associated with
that hidraw leading to slab corruption.
Signed-off-by: Manoj Chourasia <mchourasia@nvidia.com>
Tested-by: Peter Wu <lekensteyn@gmail.com>
Cc: stable@vger.kernel.org
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Mutex can not be released unless all hid_device members are properly
initialized. Otherwise it would result in a race condition that can
cause NULL pointer kernel panic issue in hidraw_open where it uses
uninitialized 'list' member in list_add_tail().
Signed-off-by: Yonghua Zheng <younghua.zheng@gmail.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Pull vfs pile (part one) from Al Viro:
"Assorted stuff - cleaning namei.c up a bit, fixing ->d_name/->d_parent
locking violations, etc.
The most visible changes here are death of FS_REVAL_DOT (replaced with
"has ->d_weak_revalidate()") and a new helper getting from struct file
to inode. Some bits of preparation to xattr method interface changes.
Misc patches by various people sent this cycle *and* ocfs2 fixes from
several cycles ago that should've been upstream right then.
PS: the next vfs pile will be xattr stuff."
* 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs: (46 commits)
saner proc_get_inode() calling conventions
proc: avoid extra pde_put() in proc_fill_super()
fs: change return values from -EACCES to -EPERM
fs/exec.c: make bprm_mm_init() static
ocfs2/dlm: use GFP_ATOMIC inside a spin_lock
ocfs2: fix possible use-after-free with AIO
ocfs2: Fix oops in ocfs2_fast_symlink_readpage() code path
get_empty_filp()/alloc_file() leave both ->f_pos and ->f_version zero
target: writev() on single-element vector is pointless
export kernel_write(), convert open-coded instances
fs: encode_fh: return FILEID_INVALID if invalid fid_type
kill f_vfsmnt
vfs: kill FS_REVAL_DOT by adding a d_weak_revalidate dentry op
nfsd: handle vfs_getattr errors in acl protocol
switch vfs_getattr() to struct path
default SET_PERSONALITY() in linux/elf.h
ceph: prepopulate inodes only when request is aborted
d_hash_and_lookup(): export, switch open-coded instances
9p: switch v9fs_set_create_acl() to inode+fid, do it before d_instantiate()
9p: split dropping the acls from v9fs_set_create_acl()
...
This patch fixes sending SIGIO from hidraw_report_event by creating a fasync
handler which adds the fasync entry.
Signed-off-by: Andrew Duggan <aduggan@synaptics.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
When nonblock read the condition check (file->f_flags & O_NONBLOCK) always be
true, signal_pending and device exist checking never get a chance to run, so
the user mode code always get EAGAIN even if device removed. move nonblock mode
checking to the last can fix this problem.
Signed-off-by: Founder Fang <founder.fang@gmail.com>
Reviewed-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
This basically reverts commit 4fe9f8e203. It causes multiple problems,
namely:
- after rmmod/modprobe cycle of bus driver, the input is not claimed any
more. This is likely because of misplaced hid_hw_close()
- it causes memory corruption on hidraw_list
As original patch author is not responding to requests to fix his patch,
and the original deallocation mechanism is not exposing any problems, I
am reverting back to it.
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
When a device is unplugged, wait for all processes that have opened the device
to close before deallocating the device.
Signed-off-by: Ratan Nalumasu <ratan@google.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Several improvements in error handling:
- do not report success if alloc_chrdev_region() failed
- check for error code of cdev_add()
- use unregister_chrdev_region() instead of unregister_chrdev()
if class_create() failed
Found by Linux Driver Verification project (linuxtesting.org).
Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
If we don't read fast enough hidraw device, hidraw_report_event
will cycle and we will leak list->buffer.
Also list->buffer are not free on release.
After this patch, kmemleak report nothing.
Signed-off-by: Matthieu CASTET <matthieu.castet@parrot.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
If kmemdup() in hidraw_report_event() fails, we are not propagating
this fact properly.
Let hidraw_report_event() and hid_report_raw_event() return an error
value to the caller.
Reported-by: Oliver Neukum <oneukum@suse.de>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
In hidraw_open, if hid_hw_power returns with error, hidraw device open count
should not increase.
Signed-off-by: Amit Nagal <helloin.amit@gmail.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
The function hidraw_disconnect() only acquires the hidraw minors_lock
when clearing the entry in hidraw_table. However the device_destroy()
call can cause a userland read/write to return with an error. It may
cause the program to release the file descripter before the disconnect
is finished. hidraw_disconnect() has already set hidraw->exist to 0,
which makes hidraw_release() kfree the hidraw structure, which
hidraw_disconnect() continues to access and even tries to kfree again.
Similarly if a hidraw_release() occurs after setting hidraw->exist to 0,
the same thing can happen.
This is fixed by expanding the mutex critical section to cover the whole
function from setting hidraw->exist to 0 to freeing the hidraw
structure, preventing a hidraw_release() from interfering.
Signed-off-by: James Hogan <james.hogan@imgtec.com>
Tested-by: David Herrmann <dh.herrmann@googlemail.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
In function hidraw_open struct hidraw_list *list should be freed for
all error conditions.
Signed-off-by: Amit Nagal <helloin.amit@gmail.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
As they are static members of fix size, there is no need to NULL-check them.
Signed-off-by: Daniel Mack <zonque@gmail.com>
Cc: Dmitry Torokhov <dtor@mail.ru>
Cc: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Per the HID Specification, Feature reports must be sent and received on
the Configuration endpoint (EP 0) through the Set_Report/Get_Report
interfaces. This patch adds two ioctls to hidraw to set and get feature
reports to and from the device. Modifications were made to hidraw and
usbhid.
New hidraw ioctls:
HIDIOCSFEATURE - Perform a Set_Report transfer of a Feature report.
HIDIOCGFEATURE - Perform a Get_Report transfer of a Feature report.
Signed-off-by: Alan Ott <alan@signal11.us>
Signed-off-by: Antonio Ospite <ospite@studenti.unina.it>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
hidraw_disconnect() first sets an entry in hidraw_table to NULL
and calls device_destroy() afterwards. The thereby called
hidraw_release() tries to read this already cleared value resulting
in never removing any device from the list.
This got fixed by changing the order of events.
Signed-off-by: Stefan Achatz <erazor_de@users.sourceforge.net>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Setting of the return value of hidraw_read() uses the += operator when
= is more appropriate. There is no case where ret can be anything
other than zero when the assignment is made, making = equivalent to
+= and much more clear.
Signed-off-by: Alan Ott <alan@signal11.us>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Added the ioctl function to the compat_ioctl pointer in the file_operations
struct. Before this, some ioctls would fail for 32-bit apps on 64-bit systems.
Signed-off-by: Alan Ott <alan@signal11.us>
Acked-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Neaten current uses of dev_<level> by adding and using
hid specific hid_<level> macros.
Convert existing uses of dev_<level> uses to hid_<level>.
Convert hid-pidff printk uses to hid_<level>.
Remove err_hid and use hid_err instead.
Add missing newlines to logging messages where necessary.
Coalesce format strings.
Add and use pr_fmt(fmt) KBUILD_MODNAME ": " fmt
Other miscellaneous changes:
Add const struct hid_device * argument to hid-core functions
extract() and implement() so hid_<level> can be used by them.
Fix bad indentation in hid-core hid_input_field function
that calls extract() function above.
Signed-off-by: Joe Perches <joe@perches.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Instead of exposing the guts of hid->ll_driver relationship to HID
sub-drivers provide these helpers to encapsulate the details.
Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
The big kernel lock has been removed from all these files at some point,
leaving only the #include.
Remove this too as a cleanup.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
* 'llseek' of git://git.kernel.org/pub/scm/linux/kernel/git/arnd/bkl:
vfs: make no_llseek the default
vfs: don't use BKL in default_llseek
llseek: automatically add .llseek fop
libfs: use generic_file_llseek for simple_attr
mac80211: disallow seeks in minstrel debug code
lirc: make chardev nonseekable
viotape: use noop_llseek
raw: use explicit llseek file operations
ibmasmfs: use generic_file_llseek
spufs: use llseek in all file operations
arm/omap: use generic_file_llseek in iommu_debug
lkdtm: use generic_file_llseek in debugfs
net/wireless: use generic_file_llseek in debugfs
drm: use noop_llseek
There is a window between hidraw_table check and its dereference.
In that window, the device may be unplugged and removed form the
system and we will then dereference NULL.
Lock that place properly so that either we get NULL and jump out or we
can work with real pointer.
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Kees Cook