KASAN reports a use-after-free during startup, in mei_cl_write:
BUG: KASAN: use-after-free in mei_cl_write+0x601/0x870 [mei]
(drivers/misc/mei/client.c:1770)
This is caused by commit 98e70866aa ("mei: add support for variable
length mei headers."), which changed the return value from len, to
buf->size. That ends up using a stale buf pointer, because blocking
call, the cb (callback) is deleted in me_cl_complete() function.
However, fortunately, len remains unchanged throughout the function
(and I don't see anything else that would require re-reading buf->size
either), so the fix is to simply revert the change, and return len, as
before.
Fixes: 98e70866aa ("mei: add support for variable length mei headers.")
CC: Arnd Bergmann <arnd@arndb.de>
CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: John Hubbard <jhubbard@nvidia.com>
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Remove header size knowledge from me and txe hw layers,
this requires to change the write handler to accept
header and its length as well as data and its length.
HBM messages are fixed to use basic header, hence we add mei_hbm2slots()
that converts HBM message length and mei message header,
while mei_data2slots() converts data length directly to the slots.
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Wrap the mei header boilerplate initialization code in
mei_msg_hdr_init function. On the way remove 'completed'
field from mei_cl_cb structure as this information
is already included in the header and is local to particular
fragment.
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The host buffer depth is hardware specific so it's better to
handle it inside the me and txe hw modules. In me the depth
is read from register in txe it's a constant number.
The value is now retrieved via mei_hbuf_depth accessor,
while it replaces mei_hbuf_max_len.
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cleanup conversions between slots and data.
Define MEI_SLOT_SIZE instead of using 4 or sizeof(u32) across
the source code.
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Use ssize_t for rets variables in mei_write(), mei_read(), and
mei_cl_write() as well as change the return type of mei_cl_write()
to ssize_t, to prevent assignment of possible 64bit size_t
to int 32 bit variable.
As by product also eliminate warning
drivers/misc/mei/client.c:1702:11: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
mei_hbuf_empty_slots() may return with an error in case
of circular buffer overflow. This type of error may
be caused only by a bug. However currently, the error
won't be detected due signed type promotion in comparison to u32.
We add explicit check for less then zero and explicit cast
in comparison to suppress singn-compare warning.
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Limit the number of queued writes per client.
Writes above this threshold are blocked till place
in the transmit queue is available.
The limit is configurable via sysfs and defaults to 50.
The implementation should provide blocking I/O behavior.
Prior to this change one would end up in the hands of OOM.
Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Module reference counting is relevant only to the
mei client devices. Make the implementation clean
and move it to bus.c
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This fixes regression introduced by
commit 8d52af6795 ("mei: speed up the power down flow")
In mei_cldev_disable during device power down flow, such as
suspend or system power off, it jumps over disconnecting function
to speed up the power down process, however, because the client is
unlinked from the file_list (mei_cl_unlink) mei_cl_set_disconnected
is not called from mei_cl_all_disconnect leaving resource leaking.
The most visible is reference counter on underlying HW module is
not decreased preventing to remove modules after suspend/resume cycles.
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
Fixes: 8d52af6795 ("mei: speed up the power down flow")
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
AMTHIF has special support in the mei drive, it handles multiplexing
multiple user space connection above single me client connection.
Since there is no additional addressing information there is a strict
requirement on the traffic order on each connection and on the "read
after write" order within the connection. This creates a lot of
complexity mostly because the other client types do not necessarily fall
under the same restriction. After carefully studying the use of the
AMTHIF client, we came to conclusion that the multiplexing is not really
utilized by any application and we may safely remove that support and
significantly simplify the driver.
Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Specify in function names by which object is the io list filtered:
cl for a client and fp for file descriptor.
In that course a code duplication is resolved by dropping
mei_cl_read_cb_flush and mei_clear_list and using
mei_io_list_free_fp function.
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
mei_dev structure used struct mei_cl_cb type variables as for holding
callbacks list heads. Replace them by the actual struct list_head
as there is no other info that is handled. This slims down
the mei_dev structure and mostly streamline the code.
Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
In order to prevent memory leak clean up the amthif command
queue upon disconnection. The issue may happen only on error path
as the command queue is cleaned upon file descriptor close.
And remove the cleanup from mei_cl_flush_queues as this code
is never reached for amthif client.
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
On legacy HW, pre Skylake, the notifications are not supported,
return -EOPNOTSUPP in mei_cl_notify_get and prevent
waiting indefinitely.
Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Request for a notification from a disconnected client will be ignored
silently by the FW but the caller should know that the operation hasn't
succeeded.
Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The hardware module should not be unloaded if the bus
has active devices.
Get get_/put_ bus parent module upon client device
connection/disconnection, to prevent the hardware managing
module to disappear underneath.
Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The credentials handling was pushed to the write handlers
but error handling wasn't done properly.
Move write callbacks to completion queue to destroy them
and to notify a blocked writer about the failure
Fixes: 136698e535 (mei: push credentials inside the irq write handler)
Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
pm_runtime_autosuspend can take synchronous or asynchronous
paths, Because we are calling pm_runtime_mark_last_busy just before
this most of the cases it takes the asynchronous way. However,
when the FW or driver resets during already running runtime suspend,
the call will result in calling to the driver's rpm callback and results
in a deadlock on device_lock.
The simplest fix is to replace pm_runtime_autosuspend with
asynchronous pm_request_autosuspend.
Cc: <stable@vger.kernel.org> # 4.4+
Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Split callbacks for RX and async notification events on mei bus to
eliminate synchronization problems and to open way for RX optimizations.
Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Introduce a new host client state, MEI_FILE_UNINITIALIZED,
to distinguish client objects that was unlinked,
but not destroyed and can be linked again.
Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
John Hubbard
Tomas Winkler
Alexander Usyskin
Ingo Molnar