| 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: Mon, 15 Jul 2019 10:08:49 -0700
From: Gwendal Grignou <gwendal@...omium.org>
To: Enric Balletbo i Serra <enric.balletbo@...labora.com>
Cc: linux-kernel <linux-kernel@...r.kernel.org>,
Jonathan Corbet <corbet@....net>,
Krzysztof Kozlowski <krzk@...nel.org>,
Will Deacon <will.deacon@....com>,
MyungJoo Ham <myungjoo.ham@...sung.com>,
Chanwoo Choi <cw00.choi@...sung.com>,
Benson Leung <bleung@...omium.org>,
Guenter Roeck <groeck@...omium.org>,
Jonathan Cameron <jic23@...nel.org>,
Dmitry Torokhov <dmitry.torokhov@...il.com>,
Mauro Carvalho Chehab <mchehab@...nel.org>,
Lee Jones <lee.jones@...aro.org>,
Sebastian Reichel <sre@...nel.org>,
Thierry Reding <thierry.reding@...il.com>,
Alexandre Belloni <alexandre.belloni@...tlin.com>,
Liam Girdwood <lgirdwood@...il.com>,
Mark Brown <broonie@...nel.org>,
Neil Armstrong <narmstrong@...libre.com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Collabora kernel ML <kernel@...labora.com>,
Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Subject: Re: [PATCH v4 03/11] mfd / platform: cros_ec: Miscellaneous character
device to talk with the EC
On Thu, Jun 27, 2019 at 3:40 AM Enric Balletbo i Serra
<enric.balletbo@...labora.com> wrote:
>
> That's a driver to talk with the ChromeOS Embedded Controller via a
> miscellaneous character device, it creates an entry in /dev for every
> instance and implements basic file operations for communicating with the
> Embedded Controller with an userspace application. The API is moved to
> the uapi folder, which is supposed to contain the user space API of the
> kernel.
>
> Note that this will replace current character device interface
> implemented in the cros-ec-dev driver in the MFD subsystem. The idea is
> to move all the functionality that extends the bounds of what MFD was
> designed to platform/chrome subsystem.
>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@...labora.com>
> Acked-by: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
> Tested-by: Gwendal Grignou <gwendal@...omium.org>
> ---
>
> Changes in v4: None
> Changes in v3:
> - Fix 'linux/mfd/cros_ec.h' is not exported (reported by lkp)
>
> Changes in v2:
> - Remove the list, and the lock, as are not needed (Greg Kroah-Hartman)
> - Remove dev_info in probe, anyway we will see the chardev or not if the
> probe fails (Greg Kroah-Hartman)
>
> Documentation/ioctl/ioctl-number.txt | 2 +-
> drivers/mfd/cros_ec_dev.c | 4 +-
> drivers/platform/chrome/Kconfig | 11 +
> drivers/platform/chrome/Makefile | 1 +
> drivers/platform/chrome/cros_ec_chardev.c | 253 ++++++++++++++++++
> .../uapi/linux/cros_ec_chardev.h | 21 +-
> 6 files changed, 273 insertions(+), 19 deletions(-)
> create mode 100644 drivers/platform/chrome/cros_ec_chardev.c
> rename drivers/mfd/cros_ec_dev.h => include/uapi/linux/cros_ec_chardev.h (50%)
>
> diff --git a/Documentation/ioctl/ioctl-number.txt b/Documentation/ioctl/ioctl-number.txt
> index c9558146ac58..8bd7907ee36d 100644
> --- a/Documentation/ioctl/ioctl-number.txt
> +++ b/Documentation/ioctl/ioctl-number.txt
> @@ -340,7 +340,7 @@ Code Seq#(hex) Include File Comments
> 0xDD 00-3F ZFCP device driver see drivers/s390/scsi/
> <mailto:aherrman@...ibm.com>
> 0xE5 00-3F linux/fuse.h
> -0xEC 00-01 drivers/platform/chrome/cros_ec_dev.h ChromeOS EC driver
> +0xEC 00-01 include/uapi/linux/cros_ec_chardev.h ChromeOS EC driver
> 0xF3 00-3F drivers/usb/misc/sisusbvga/sisusb.h sisfb (in development)
> <mailto:thomas@...ischhofer.net>
> 0xF4 00-1F video/mbxfb.h mbxfb
> diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c
> index eb77bf6406f3..3334bc4e42ee 100644
> --- a/drivers/mfd/cros_ec_dev.c
> +++ b/drivers/mfd/cros_ec_dev.c
> @@ -19,6 +19,8 @@
>
> #include <linux/fs.h>
> #include <linux/mfd/core.h>
> +#include <linux/mfd/cros_ec.h>
> +#include <linux/mfd/cros_ec_commands.h>
> #include <linux/module.h>
> #include <linux/mod_devicetable.h>
> #include <linux/of_platform.h>
> @@ -27,7 +29,7 @@
> #include <linux/slab.h>
> #include <linux/uaccess.h>
>
> -#include "cros_ec_dev.h"
> +#include <uapi/linux/cros_ec_chardev.h>
>
> #define DRV_NAME "cros-ec-dev"
>
> diff --git a/drivers/platform/chrome/Kconfig b/drivers/platform/chrome/Kconfig
> index 1e7a10500b3f..221e709358c0 100644
> --- a/drivers/platform/chrome/Kconfig
> +++ b/drivers/platform/chrome/Kconfig
> @@ -133,6 +133,17 @@ config CROS_KBD_LED_BACKLIGHT
> To compile this driver as a module, choose M here: the
> module will be called cros_kbd_led_backlight.
>
> +config CROS_EC_CHARDEV
> + tristate "ChromeOS EC miscdevice"
> + depends on MFD_CROS_EC_CHARDEV
> + default MFD_CROS_EC_CHARDEV
> + help
> + This driver adds file operations support to talk with the
> + ChromeOS EC from userspace via a character device.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called cros_ec_chardev.
> +
> config CROS_EC_LIGHTBAR
> tristate "Chromebook Pixel's lightbar support"
> depends on MFD_CROS_EC_CHARDEV
> diff --git a/drivers/platform/chrome/Makefile b/drivers/platform/chrome/Makefile
> index f69e0be98bd6..e6758e967ac5 100644
> --- a/drivers/platform/chrome/Makefile
> +++ b/drivers/platform/chrome/Makefile
> @@ -15,6 +15,7 @@ cros_ec_lpcs-$(CONFIG_CROS_EC_LPC_MEC) += cros_ec_lpc_mec.o
> obj-$(CONFIG_CROS_EC_LPC) += cros_ec_lpcs.o
> obj-$(CONFIG_CROS_EC_PROTO) += cros_ec_proto.o cros_ec_trace.o
> obj-$(CONFIG_CROS_KBD_LED_BACKLIGHT) += cros_kbd_led_backlight.o
> +obj-$(CONFIG_CROS_EC_CHARDEV) += cros_ec_chardev.o
> obj-$(CONFIG_CROS_EC_LIGHTBAR) += cros_ec_lightbar.o
> obj-$(CONFIG_CROS_EC_VBC) += cros_ec_vbc.o
> obj-$(CONFIG_CROS_EC_DEBUGFS) += cros_ec_debugfs.o
> diff --git a/drivers/platform/chrome/cros_ec_chardev.c b/drivers/platform/chrome/cros_ec_chardev.c
> new file mode 100644
> index 000000000000..ce77839b0dcb
> --- /dev/null
> +++ b/drivers/platform/chrome/cros_ec_chardev.c
> @@ -0,0 +1,253 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Miscellaneous character driver for ChromeOS Embedded Controller
> + *
> + * Copyright 2014 Google, Inc.
> + * Copyright 2019 Google LLC
> + *
> + * This file is a rework and part of the code is ported from
> + * drivers/mfd/cros_ec_dev.c that was originally written by
> + * Bill Richardson.
> + */
> +
> +#include <linux/init.h>
> +#include <linux/device.h>
> +#include <linux/fs.h>
> +#include <linux/mfd/cros_ec.h>
> +#include <linux/mfd/cros_ec_commands.h>
> +#include <linux/miscdevice.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/types.h>
> +#include <linux/uaccess.h>
> +
> +#include <uapi/linux/cros_ec_chardev.h>
> +
> +#define DRV_NAME "cros-ec-chardev"
> +
> +struct chardev_data {
> + struct cros_ec_dev *ec_dev;
> + struct miscdevice misc;
> +};
> +
> +static int ec_get_version(struct cros_ec_dev *ec, char *str, int maxlen)
> +{
> + static const char * const current_image_name[] = {
> + "unknown", "read-only", "read-write", "invalid",
> + };
> + struct ec_response_get_version *resp;
> + struct cros_ec_command *msg;
> + int ret;
> +
> + msg = kzalloc(sizeof(*msg) + sizeof(*resp), GFP_KERNEL);
> + if (!msg)
> + return -ENOMEM;
> +
> + msg->command = EC_CMD_GET_VERSION + ec->cmd_offset;
> + msg->insize = sizeof(*resp);
> +
> + ret = cros_ec_cmd_xfer_status(ec->ec_dev, msg);
> + if (ret < 0) {
> + snprintf(str, maxlen,
> + "Unknown EC version, returned error: %d\n",
> + msg->result);
> + goto exit;
> + }
> +
> + resp = (struct ec_response_get_version *)msg->data;
> + if (resp->current_image >= ARRAY_SIZE(current_image_name))
> + resp->current_image = 3; /* invalid */
> +
> + snprintf(str, maxlen, "%s\n%s\n%s\n",
We need to keep the same output as it was implemented in
ec_get_version in cros_ec_dev.c [See "mfd: cros_ec: Switch to use the
new cros-ec-chardev driver"]:
In particular, we need to prefix the versions strings with
CROS_EC_DEV_VERSION ["1.0.0"], as user space tools are relying on that
marker to keep going.
> + resp->version_string_ro,
> + resp->version_string_rw,
> + current_image_name[resp->current_image]);
> +
> + ret = 0;
> +exit:
> + kfree(msg);
> + return ret;
> +}
> +
> +/*
> + * Device file ops
> + */
> +static int cros_ec_chardev_open(struct inode *inode, struct file *filp)
> +{
> + struct miscdevice *mdev = filp->private_data;
> + struct cros_ec_dev *ec_dev = dev_get_drvdata(mdev->parent);
> +
> + filp->private_data = ec_dev;
> + nonseekable_open(inode, filp);
> +
> + return 0;
> +}
> +
> +static ssize_t cros_ec_chardev_read(struct file *filp, char __user *buffer,
> + size_t length, loff_t *offset)
> +{
> + char msg[sizeof(struct ec_response_get_version) +
> + sizeof(CROS_EC_DEV_VERSION)];
> + struct cros_ec_dev *ec = filp->private_data;
> + size_t count;
> + int ret;
> +
> + if (*offset != 0)
> + return 0;
> +
> + ret = ec_get_version(ec, msg, sizeof(msg));
> + if (ret)
> + return ret;
> +
> + count = min(length, strlen(msg));
> +
> + if (copy_to_user(buffer, msg, count))
> + return -EFAULT;
> +
> + *offset = count;
> + return count;
> +}
> +
> +/*
> + * Ioctls
> + */
> +static long cros_ec_chardev_ioctl_xcmd(struct cros_ec_dev *ec, void __user *arg)
> +{
> + struct cros_ec_command *s_cmd;
> + struct cros_ec_command u_cmd;
> + long ret;
> +
> + if (copy_from_user(&u_cmd, arg, sizeof(u_cmd)))
> + return -EFAULT;
> +
> + if (u_cmd.outsize > EC_MAX_MSG_BYTES ||
> + u_cmd.insize > EC_MAX_MSG_BYTES)
> + return -EINVAL;
> +
> + s_cmd = kmalloc(sizeof(*s_cmd) + max(u_cmd.outsize, u_cmd.insize),
> + GFP_KERNEL);
> + if (!s_cmd)
> + return -ENOMEM;
> +
> + if (copy_from_user(s_cmd, arg, sizeof(*s_cmd) + u_cmd.outsize)) {
> + ret = -EFAULT;
> + goto exit;
> + }
> +
> + if (u_cmd.outsize != s_cmd->outsize ||
> + u_cmd.insize != s_cmd->insize) {
> + ret = -EINVAL;
> + goto exit;
> + }
> +
> + s_cmd->command += ec->cmd_offset;
> + ret = cros_ec_cmd_xfer(ec->ec_dev, s_cmd);
> + /* Only copy data to userland if data was received. */
> + if (ret < 0)
> + goto exit;
> +
> + if (copy_to_user(arg, s_cmd, sizeof(*s_cmd) + s_cmd->insize))
> + ret = -EFAULT;
> +exit:
> + kfree(s_cmd);
> + return ret;
> +}
> +
> +static long cros_ec_chardev_ioctl_readmem(struct cros_ec_dev *ec,
> + void __user *arg)
> +{
> + struct cros_ec_device *ec_dev = ec->ec_dev;
> + struct cros_ec_readmem s_mem = { };
> + long num;
> +
> + /* Not every platform supports direct reads */
> + if (!ec_dev->cmd_readmem)
> + return -ENOTTY;
> +
> + if (copy_from_user(&s_mem, arg, sizeof(s_mem)))
> + return -EFAULT;
> +
> + num = ec_dev->cmd_readmem(ec_dev, s_mem.offset, s_mem.bytes,
> + s_mem.buffer);
> + if (num <= 0)
> + return num;
> +
> + if (copy_to_user((void __user *)arg, &s_mem, sizeof(s_mem)))
> + return -EFAULT;
> +
> + return num;
> +}
> +
> +static long cros_ec_chardev_ioctl(struct file *filp, unsigned int cmd,
> + unsigned long arg)
> +{
> + struct cros_ec_dev *ec = filp->private_data;
> +
> + if (_IOC_TYPE(cmd) != CROS_EC_DEV_IOC)
> + return -ENOTTY;
> +
> + switch (cmd) {
> + case CROS_EC_DEV_IOCXCMD:
> + return cros_ec_chardev_ioctl_xcmd(ec, (void __user *)arg);
> + case CROS_EC_DEV_IOCRDMEM:
> + return cros_ec_chardev_ioctl_readmem(ec, (void __user *)arg);
> + }
> +
> + return -ENOTTY;
> +}
> +
> +static const struct file_operations chardev_fops = {
> + .open = cros_ec_chardev_open,
> + .read = cros_ec_chardev_read,
> + .unlocked_ioctl = cros_ec_chardev_ioctl,
> +#ifdef CONFIG_COMPAT
> + .compat_ioctl = cros_ec_chardev_ioctl,
> +#endif
> +};
> +
> +static int cros_ec_chardev_probe(struct platform_device *pdev)
> +{
> + struct cros_ec_dev *ec_dev = dev_get_drvdata(pdev->dev.parent);
> + struct cros_ec_platform *ec_platform = dev_get_platdata(ec_dev->dev);
> + struct chardev_data *data;
> +
> + /* Create a char device: we want to create it anew */
> + data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> +
> + data->ec_dev = ec_dev;
> + data->misc.minor = MISC_DYNAMIC_MINOR;
> + data->misc.fops = &chardev_fops;
> + data->misc.name = ec_platform->ec_name;
> + data->misc.parent = pdev->dev.parent;
> +
> + dev_set_drvdata(&pdev->dev, data);
> +
> + return misc_register(&data->misc);
> +}
> +
> +static int cros_ec_chardev_remove(struct platform_device *pdev)
> +{
> + struct chardev_data *data = dev_get_drvdata(&pdev->dev);
> +
> + misc_deregister(&data->misc);
> +
> + return 0;
> +}
> +
> +static struct platform_driver cros_ec_chardev_driver = {
> + .driver = {
> + .name = DRV_NAME,
> + },
> + .probe = cros_ec_chardev_probe,
> + .remove = cros_ec_chardev_remove,
> +};
> +
> +module_platform_driver(cros_ec_chardev_driver);
> +
> +MODULE_ALIAS("platform:" DRV_NAME);
> +MODULE_AUTHOR("Enric Balletbo i Serra <enric.balletbo@...labora.com>");
> +MODULE_DESCRIPTION("ChromeOS EC Miscellaneous Character Driver");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/mfd/cros_ec_dev.h b/include/uapi/linux/cros_ec_chardev.h
> similarity index 50%
> rename from drivers/mfd/cros_ec_dev.h
> rename to include/uapi/linux/cros_ec_chardev.h
> index ec750433455a..8aceb4ac8ec5 100644
> --- a/drivers/mfd/cros_ec_dev.h
> +++ b/include/uapi/linux/cros_ec_chardev.h
> @@ -1,28 +1,15 @@
> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> /*
> - * cros_ec_dev - expose the Chrome OS Embedded Controller to userspace
> + * ChromeOS EC device interface.
> *
> * Copyright (C) 2014 Google, Inc.
> - *
> - * 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.
> - *
> - * This program is distributed in the hope that it will be useful,
> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> - * GNU General Public License for more details.
> - *
> - * You should have received a copy of the GNU General Public License
> - * along with this program. If not, see <http://www.gnu.org/licenses/>.
> */
>
> -#ifndef _CROS_EC_DEV_H_
> -#define _CROS_EC_DEV_H_
> +#ifndef _UAPI_LINUX_CROS_EC_DEV_H_
> +#define _UAPI_LINUX_CROS_EC_DEV_H_
>
> #include <linux/ioctl.h>
> #include <linux/types.h>
> -#include <linux/mfd/cros_ec.h>
>
> #define CROS_EC_DEV_VERSION "1.0.0"
>
> --
> 2.20.1
>
Powered by blists - more mailing lists