| 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
| ||
|
Date: Thu, 17 Nov 2016 16:37:19 +0100
From: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To: Tomas Winkler <tomas.winkler@...el.com>
Cc: Alexander Usyskin <alexander.usyskin@...el.com>,
linux-kernel@...r.kernel.org
Subject: Re: [char-misc-next 4/4] mei: bus: enable non-blocking RX
On Wed, Nov 16, 2016 at 10:51:30PM +0200, Tomas Winkler wrote:
> From: Alexander Usyskin <alexander.usyskin@...el.com>
>
> Enable non-blocking receive for drivers on mei bus, this allows checking
> for data availability by mei client drivers. This is most effective for
> fixed address clients, that lacks flow control.
>
> Signed-off-by: Alexander Usyskin <alexander.usyskin@...el.com>
> Signed-off-by: Tomas Winkler <tomas.winkler@...el.com>
> ---
> drivers/misc/mei/bus-fixup.c | 4 ++--
> drivers/misc/mei/bus.c | 19 ++++++++++++++++---
> drivers/misc/mei/mei_dev.h | 7 ++++++-
> drivers/nfc/mei_phy.c | 7 ++++---
> drivers/watchdog/mei_wdt.c | 2 +-
> include/linux/mei_cl_bus.h | 3 ++-
> 6 files changed, 31 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/misc/mei/bus-fixup.c b/drivers/misc/mei/bus-fixup.c
> index 7f2cef9011ae..18e05ca7584f 100644
> --- a/drivers/misc/mei/bus-fixup.c
> +++ b/drivers/misc/mei/bus-fixup.c
> @@ -141,7 +141,7 @@ static int mei_osver(struct mei_cl_device *cldev)
> if (ret < 0)
> return ret;
>
> - ret = __mei_cl_recv(cldev->cl, buf, length);
> + ret = __mei_cl_recv(cldev->cl, buf, length, 0);
> if (ret < 0)
> return ret;
>
> @@ -272,7 +272,7 @@ static int mei_nfc_if_version(struct mei_cl *cl,
> return -ENOMEM;
>
> ret = 0;
> - bytes_recv = __mei_cl_recv(cl, (u8 *)reply, if_version_length);
> + bytes_recv = __mei_cl_recv(cl, (u8 *)reply, if_version_length, 0);
> if (bytes_recv < if_version_length) {
> dev_err(bus->dev, "Could not read IF version\n");
> ret = -EIO;
> diff --git a/drivers/misc/mei/bus.c b/drivers/misc/mei/bus.c
> index 2fd254ecde2f..45e937937675 100644
> --- a/drivers/misc/mei/bus.c
> +++ b/drivers/misc/mei/bus.c
> @@ -98,15 +98,18 @@ ssize_t __mei_cl_send(struct mei_cl *cl, u8 *buf, size_t length,
> * @cl: host client
> * @buf: buffer to receive
> * @length: buffer length
> + * @mode: io mode
> *
> * Return: read size in bytes of < 0 on error
> */
> -ssize_t __mei_cl_recv(struct mei_cl *cl, u8 *buf, size_t length)
> +ssize_t __mei_cl_recv(struct mei_cl *cl, u8 *buf, size_t length,
> + unsigned int mode)
> {
> struct mei_device *bus;
> struct mei_cl_cb *cb;
> size_t r_length;
> ssize_t rets;
> + bool nonblock = !!(mode & MEI_CL_IO_RX_NONBLOCK);
>
> if (WARN_ON(!cl || !cl->dev))
> return -ENODEV;
> @@ -127,6 +130,11 @@ ssize_t __mei_cl_recv(struct mei_cl *cl, u8 *buf, size_t length)
> if (rets && rets != -EBUSY)
> goto out;
>
> + if (nonblock) {
> + rets = -EAGAIN;
> + goto out;
> + }
> +
> /* wait on event only if there is no other waiter */
> /* synchronized under device mutex */
> if (!waitqueue_active(&cl->rx_wait)) {
> @@ -197,14 +205,19 @@ EXPORT_SYMBOL_GPL(mei_cldev_send);
> * @cldev: me client device
> * @buf: buffer to receive
> * @length: buffer length
> + * @flags: read io flags [O_NONBLOCK]
> *
> * Return: read size in bytes of < 0 on error
> */
> -ssize_t mei_cldev_recv(struct mei_cl_device *cldev, u8 *buf, size_t length)
> +ssize_t mei_cldev_recv(struct mei_cl_device *cldev, u8 *buf, size_t length,
> + unsigned int flags)
> {
> struct mei_cl *cl = cldev->cl;
> + unsigned int mode;
> +
> + mode = (flags & O_NONBLOCK) ? MEI_CL_IO_RX_NONBLOCK : 0;
>
> - return __mei_cl_recv(cl, buf, length);
> + return __mei_cl_recv(cl, buf, length, mode);
> }
> EXPORT_SYMBOL_GPL(mei_cldev_recv);
>
> diff --git a/drivers/misc/mei/mei_dev.h b/drivers/misc/mei/mei_dev.h
> index 82e69a00b7a1..f16bd1209848 100644
> --- a/drivers/misc/mei/mei_dev.h
> +++ b/drivers/misc/mei/mei_dev.h
> @@ -115,10 +115,14 @@ enum mei_cb_file_ops {
> *
> * @MEI_CL_IO_TX_BLOCKING: send is blocking
> * @MEI_CL_IO_TX_INTERNAL: internal communication between driver and FW
> + *
> + * @MEI_CL_IO_RX_NONBLOCK: recv is non-blocking
> */
> enum mei_cl_io_mode {
> MEI_CL_IO_TX_BLOCKING = BIT(0),
> MEI_CL_IO_TX_INTERNAL = BIT(1),
> +
> + MEI_CL_IO_RX_NONBLOCK = BIT(2),
> };
>
> /*
> @@ -318,7 +322,8 @@ void mei_cl_bus_rescan_work(struct work_struct *work);
> void mei_cl_bus_dev_fixup(struct mei_cl_device *dev);
> ssize_t __mei_cl_send(struct mei_cl *cl, u8 *buf, size_t length,
> unsigned int mode);
> -ssize_t __mei_cl_recv(struct mei_cl *cl, u8 *buf, size_t length);
> +ssize_t __mei_cl_recv(struct mei_cl *cl, u8 *buf, size_t length,
> + unsigned int mode);
> bool mei_cl_bus_rx_event(struct mei_cl *cl);
> bool mei_cl_bus_notify_event(struct mei_cl *cl);
> void mei_cl_bus_remove_devices(struct mei_device *bus);
> diff --git a/drivers/nfc/mei_phy.c b/drivers/nfc/mei_phy.c
> index 8a04c5e02999..0e8158c5f81b 100644
> --- a/drivers/nfc/mei_phy.c
> +++ b/drivers/nfc/mei_phy.c
> @@ -132,7 +132,8 @@ static int mei_nfc_if_version(struct nfc_mei_phy *phy)
> if (!reply)
> return -ENOMEM;
>
> - bytes_recv = mei_cldev_recv(phy->cldev, (u8 *)reply, if_version_length);
> + bytes_recv = mei_cldev_recv(phy->cldev, (u8 *)reply,
> + if_version_length, 0);
> if (bytes_recv < 0 || bytes_recv < if_version_length) {
> pr_err("Could not read IF version\n");
> r = -EIO;
> @@ -193,7 +194,7 @@ static int mei_nfc_connect(struct nfc_mei_phy *phy)
> }
>
> bytes_recv = mei_cldev_recv(phy->cldev, (u8 *)reply,
> - connect_resp_length);
> + connect_resp_length, 0);
> if (bytes_recv < 0) {
> r = bytes_recv;
> pr_err("Could not read connect response %d\n", r);
> @@ -279,7 +280,7 @@ static int mei_nfc_recv(struct nfc_mei_phy *phy, u8 *buf, size_t length)
> struct mei_nfc_hdr *hdr;
> int received_length;
>
> - received_length = mei_cldev_recv(phy->cldev, buf, length);
> + received_length = mei_cldev_recv(phy->cldev, buf, length, 0);
> if (received_length < 0)
> return received_length;
>
> diff --git a/drivers/watchdog/mei_wdt.c b/drivers/watchdog/mei_wdt.c
> index b29c6fde7473..3e29af534f8e 100644
> --- a/drivers/watchdog/mei_wdt.c
> +++ b/drivers/watchdog/mei_wdt.c
> @@ -423,7 +423,7 @@ static void mei_wdt_rx(struct mei_cl_device *cldev)
> const size_t res_len = sizeof(res);
> int ret;
>
> - ret = mei_cldev_recv(wdt->cldev, (u8 *)&res, res_len);
> + ret = mei_cldev_recv(wdt->cldev, (u8 *)&res, res_len, 0);
> if (ret < 0) {
> dev_err(&cldev->dev, "failure in recv %d\n", ret);
> return;
> diff --git a/include/linux/mei_cl_bus.h b/include/linux/mei_cl_bus.h
> index 017f5232b3de..c3bb565f0745 100644
> --- a/include/linux/mei_cl_bus.h
> +++ b/include/linux/mei_cl_bus.h
> @@ -86,7 +86,8 @@ void mei_cldev_driver_unregister(struct mei_cl_driver *cldrv);
> mei_cldev_driver_unregister)
>
> ssize_t mei_cldev_send(struct mei_cl_device *cldev, u8 *buf, size_t length);
> -ssize_t mei_cldev_recv(struct mei_cl_device *cldev, u8 *buf, size_t length);
> +ssize_t mei_cldev_recv(struct mei_cl_device *cldev, u8 *buf, size_t length,
> + unsigned int flags);
Functions like this are horrid! You have to always go around and dig up
what "flags" means, and if 1 means this or 0 means that or whatever.
It makes it so you can not read the code that calls this function at all
and know what is going on.
Just make a new function mei_cldev_recv_async() and then call a local,
static function, that does the work with the correct flag set. That way
the developer always knows exactly what is going on.
thanks,
greg k-h
Powered by blists - more mailing lists