* [PATCH] platform/chrome: wilco_ec: Add batt_ppid_info command to telemetry driver
@ 2019-08-05 20:22 Nick Crews
2019-08-22 19:33 ` Enric Balletbo i Serra
2019-08-22 19:43 ` Nick Crews
0 siblings, 2 replies; 3+ messages in thread
From: Nick Crews @ 2019-08-05 20:22 UTC (permalink / raw)
To: enric.balletbo, bleung
Cc: linux-kernel, dlaurie, djkurtz, dtor, sjg, Nick Crews
Add the GET_BATT_PPID_INFO=0x8A command to the allowlist of accepted
telemetry commands. In addition, since this new command requires
verifying the contents of some of the arguments, I also restructure
the request to use a union of the argument structs. Also, zero out the
request buffer before each request, and change "whitelist" to
"allowlist".
Signed-off-by: Nick Crews <ncrews@chromium.org>
---
drivers/platform/chrome/wilco_ec/telemetry.c | 64 +++++++++++++-------
1 file changed, 43 insertions(+), 21 deletions(-)
diff --git a/drivers/platform/chrome/wilco_ec/telemetry.c b/drivers/platform/chrome/wilco_ec/telemetry.c
index 94cdc166c840..b9d03c33d8dc 100644
--- a/drivers/platform/chrome/wilco_ec/telemetry.c
+++ b/drivers/platform/chrome/wilco_ec/telemetry.c
@@ -9,7 +9,7 @@
* the OS sends a command to the EC via a write() to a char device,
* and can read the response with a read(). The write() request is
* verified by the driver to ensure that it is performing only one
- * of the whitelisted commands, and that no extraneous data is
+ * of the allowlisted commands, and that no extraneous data is
* being transmitted to the EC. The response is passed directly
* back to the reader with no modification.
*
@@ -59,21 +59,10 @@ static DEFINE_IDA(telem_ida);
#define WILCO_EC_TELEM_GET_TEMP_INFO 0x95
#define WILCO_EC_TELEM_GET_TEMP_READ 0x2C
#define WILCO_EC_TELEM_GET_BATT_EXT_INFO 0x07
+#define WILCO_EC_TELEM_GET_BATT_PPID_INFO 0x8A
#define TELEM_ARGS_SIZE_MAX 30
-/**
- * struct wilco_ec_telem_request - Telemetry command and arguments sent to EC.
- * @command: One of WILCO_EC_TELEM_GET_* command codes.
- * @reserved: Must be 0.
- * @args: The first N bytes are one of telem_args_get_* structs, the rest is 0.
- */
-struct wilco_ec_telem_request {
- u8 command;
- u8 reserved;
- u8 args[TELEM_ARGS_SIZE_MAX];
-} __packed;
-
/*
* The following telem_args_get_* structs are embedded within the |args| field
* of wilco_ec_telem_request.
@@ -122,6 +111,32 @@ struct telem_args_get_batt_ext_info {
u8 var_args[5];
} __packed;
+struct telem_args_get_batt_ppid_info {
+ u8 always1; /* Should always be 1 */
+} __packed;
+
+/**
+ * struct wilco_ec_telem_request - Telemetry command and arguments sent to EC.
+ * @command: One of WILCO_EC_TELEM_GET_* command codes.
+ * @reserved: Must be 0.
+ * @args: The first N bytes are one of telem_args_get_* structs, the rest is 0.
+ */
+struct wilco_ec_telem_request {
+ u8 command;
+ u8 reserved;
+ union {
+ u8 buf[TELEM_ARGS_SIZE_MAX];
+ struct telem_args_get_log get_log;
+ struct telem_args_get_version get_version;
+ struct telem_args_get_fan_info get_fan_info;
+ struct telem_args_get_diag_info get_diag_info;
+ struct telem_args_get_temp_info get_temp_info;
+ struct telem_args_get_temp_read get_temp_read;
+ struct telem_args_get_batt_ext_info get_batt_ext_info;
+ struct telem_args_get_batt_ppid_info get_batt_ppid_info;
+ } args;
+} __packed;
+
/**
* check_telem_request() - Ensure that a request from userspace is valid.
* @rq: Request buffer copied from userspace.
@@ -133,7 +148,7 @@ struct telem_args_get_batt_ext_info {
* We do not want to allow userspace to send arbitrary telemetry commands to
* the EC. Therefore we check to ensure that
* 1. The request follows the format of struct wilco_ec_telem_request.
- * 2. The supplied command code is one of the whitelisted commands.
+ * 2. The supplied command code is one of the allowlisted commands.
* 3. The request only contains the necessary data for the header and arguments.
*/
static int check_telem_request(struct wilco_ec_telem_request *rq,
@@ -146,25 +161,31 @@ static int check_telem_request(struct wilco_ec_telem_request *rq,
switch (rq->command) {
case WILCO_EC_TELEM_GET_LOG:
- max_size += sizeof(struct telem_args_get_log);
+ max_size += sizeof(rq->args.get_log);
break;
case WILCO_EC_TELEM_GET_VERSION:
- max_size += sizeof(struct telem_args_get_version);
+ max_size += sizeof(rq->args.get_version);
break;
case WILCO_EC_TELEM_GET_FAN_INFO:
- max_size += sizeof(struct telem_args_get_fan_info);
+ max_size += sizeof(rq->args.get_fan_info);
break;
case WILCO_EC_TELEM_GET_DIAG_INFO:
- max_size += sizeof(struct telem_args_get_diag_info);
+ max_size += sizeof(rq->args.get_diag_info);
break;
case WILCO_EC_TELEM_GET_TEMP_INFO:
- max_size += sizeof(struct telem_args_get_temp_info);
+ max_size += sizeof(rq->args.get_temp_info);
break;
case WILCO_EC_TELEM_GET_TEMP_READ:
- max_size += sizeof(struct telem_args_get_temp_read);
+ max_size += sizeof(rq->args.get_temp_read);
break;
case WILCO_EC_TELEM_GET_BATT_EXT_INFO:
- max_size += sizeof(struct telem_args_get_batt_ext_info);
+ max_size += sizeof(rq->args.get_batt_ext_info);
+ break;
+ case WILCO_EC_TELEM_GET_BATT_PPID_INFO:
+ if (rq->args.get_batt_ppid_info.always1 != 1)
+ return -EINVAL;
+
+ max_size += sizeof(rq->args.get_batt_ppid_info);
break;
default:
return -EINVAL;
@@ -250,6 +271,7 @@ static ssize_t telem_write(struct file *filp, const char __user *buf,
if (count > sizeof(sess_data->request))
return -EMSGSIZE;
+ memset(&sess_data->request, 0, sizeof(sess_data->request));
if (copy_from_user(&sess_data->request, buf, count))
return -EFAULT;
ret = check_telem_request(&sess_data->request, count);
--
2.20.1
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] platform/chrome: wilco_ec: Add batt_ppid_info command to telemetry driver
2019-08-05 20:22 [PATCH] platform/chrome: wilco_ec: Add batt_ppid_info command to telemetry driver Nick Crews
@ 2019-08-22 19:33 ` Enric Balletbo i Serra
2019-08-22 19:43 ` Nick Crews
1 sibling, 0 replies; 3+ messages in thread
From: Enric Balletbo i Serra @ 2019-08-22 19:33 UTC (permalink / raw)
To: Nick Crews, bleung; +Cc: linux-kernel, dlaurie, djkurtz, dtor, sjg
Hi Nick,
On 5/8/19 22:22, Nick Crews wrote:
> Add the GET_BATT_PPID_INFO=0x8A command to the allowlist of accepted
> telemetry commands. In addition, since this new command requires
> verifying the contents of some of the arguments, I also restructure
> the request to use a union of the argument structs. Also, zero out the
> request buffer before each request, and change "whitelist" to
> "allowlist".
>
> Signed-off-by: Nick Crews <ncrews@chromium.org>
Sorry for my late reply on this, I took some vacations :)
I added for the patch for the autobuilders to play with and if everything goes
well I'll add the patch to chrome-platform-5.4
Thanks,
Enric
> ---
> drivers/platform/chrome/wilco_ec/telemetry.c | 64 +++++++++++++-------
> 1 file changed, 43 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/platform/chrome/wilco_ec/telemetry.c b/drivers/platform/chrome/wilco_ec/telemetry.c
> index 94cdc166c840..b9d03c33d8dc 100644
> --- a/drivers/platform/chrome/wilco_ec/telemetry.c
> +++ b/drivers/platform/chrome/wilco_ec/telemetry.c
> @@ -9,7 +9,7 @@
> * the OS sends a command to the EC via a write() to a char device,
> * and can read the response with a read(). The write() request is
> * verified by the driver to ensure that it is performing only one
> - * of the whitelisted commands, and that no extraneous data is
> + * of the allowlisted commands, and that no extraneous data is
> * being transmitted to the EC. The response is passed directly
> * back to the reader with no modification.
> *
> @@ -59,21 +59,10 @@ static DEFINE_IDA(telem_ida);
> #define WILCO_EC_TELEM_GET_TEMP_INFO 0x95
> #define WILCO_EC_TELEM_GET_TEMP_READ 0x2C
> #define WILCO_EC_TELEM_GET_BATT_EXT_INFO 0x07
> +#define WILCO_EC_TELEM_GET_BATT_PPID_INFO 0x8A
>
> #define TELEM_ARGS_SIZE_MAX 30
>
> -/**
> - * struct wilco_ec_telem_request - Telemetry command and arguments sent to EC.
> - * @command: One of WILCO_EC_TELEM_GET_* command codes.
> - * @reserved: Must be 0.
> - * @args: The first N bytes are one of telem_args_get_* structs, the rest is 0.
> - */
> -struct wilco_ec_telem_request {
> - u8 command;
> - u8 reserved;
> - u8 args[TELEM_ARGS_SIZE_MAX];
> -} __packed;
> -
> /*
> * The following telem_args_get_* structs are embedded within the |args| field
> * of wilco_ec_telem_request.
> @@ -122,6 +111,32 @@ struct telem_args_get_batt_ext_info {
> u8 var_args[5];
> } __packed;
>
> +struct telem_args_get_batt_ppid_info {
> + u8 always1; /* Should always be 1 */
> +} __packed;
> +
> +/**
> + * struct wilco_ec_telem_request - Telemetry command and arguments sent to EC.
> + * @command: One of WILCO_EC_TELEM_GET_* command codes.
> + * @reserved: Must be 0.
> + * @args: The first N bytes are one of telem_args_get_* structs, the rest is 0.
> + */
> +struct wilco_ec_telem_request {
> + u8 command;
> + u8 reserved;
> + union {
> + u8 buf[TELEM_ARGS_SIZE_MAX];
> + struct telem_args_get_log get_log;
> + struct telem_args_get_version get_version;
> + struct telem_args_get_fan_info get_fan_info;
> + struct telem_args_get_diag_info get_diag_info;
> + struct telem_args_get_temp_info get_temp_info;
> + struct telem_args_get_temp_read get_temp_read;
> + struct telem_args_get_batt_ext_info get_batt_ext_info;
> + struct telem_args_get_batt_ppid_info get_batt_ppid_info;
> + } args;
> +} __packed;
> +
> /**
> * check_telem_request() - Ensure that a request from userspace is valid.
> * @rq: Request buffer copied from userspace.
> @@ -133,7 +148,7 @@ struct telem_args_get_batt_ext_info {
> * We do not want to allow userspace to send arbitrary telemetry commands to
> * the EC. Therefore we check to ensure that
> * 1. The request follows the format of struct wilco_ec_telem_request.
> - * 2. The supplied command code is one of the whitelisted commands.
> + * 2. The supplied command code is one of the allowlisted commands.
> * 3. The request only contains the necessary data for the header and arguments.
> */
> static int check_telem_request(struct wilco_ec_telem_request *rq,
> @@ -146,25 +161,31 @@ static int check_telem_request(struct wilco_ec_telem_request *rq,
>
> switch (rq->command) {
> case WILCO_EC_TELEM_GET_LOG:
> - max_size += sizeof(struct telem_args_get_log);
> + max_size += sizeof(rq->args.get_log);
> break;
> case WILCO_EC_TELEM_GET_VERSION:
> - max_size += sizeof(struct telem_args_get_version);
> + max_size += sizeof(rq->args.get_version);
> break;
> case WILCO_EC_TELEM_GET_FAN_INFO:
> - max_size += sizeof(struct telem_args_get_fan_info);
> + max_size += sizeof(rq->args.get_fan_info);
> break;
> case WILCO_EC_TELEM_GET_DIAG_INFO:
> - max_size += sizeof(struct telem_args_get_diag_info);
> + max_size += sizeof(rq->args.get_diag_info);
> break;
> case WILCO_EC_TELEM_GET_TEMP_INFO:
> - max_size += sizeof(struct telem_args_get_temp_info);
> + max_size += sizeof(rq->args.get_temp_info);
> break;
> case WILCO_EC_TELEM_GET_TEMP_READ:
> - max_size += sizeof(struct telem_args_get_temp_read);
> + max_size += sizeof(rq->args.get_temp_read);
> break;
> case WILCO_EC_TELEM_GET_BATT_EXT_INFO:
> - max_size += sizeof(struct telem_args_get_batt_ext_info);
> + max_size += sizeof(rq->args.get_batt_ext_info);
> + break;
> + case WILCO_EC_TELEM_GET_BATT_PPID_INFO:
> + if (rq->args.get_batt_ppid_info.always1 != 1)
> + return -EINVAL;
> +
> + max_size += sizeof(rq->args.get_batt_ppid_info);
> break;
> default:
> return -EINVAL;
> @@ -250,6 +271,7 @@ static ssize_t telem_write(struct file *filp, const char __user *buf,
>
> if (count > sizeof(sess_data->request))
> return -EMSGSIZE;
> + memset(&sess_data->request, 0, sizeof(sess_data->request));
> if (copy_from_user(&sess_data->request, buf, count))
> return -EFAULT;
> ret = check_telem_request(&sess_data->request, count);
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] platform/chrome: wilco_ec: Add batt_ppid_info command to telemetry driver
2019-08-05 20:22 [PATCH] platform/chrome: wilco_ec: Add batt_ppid_info command to telemetry driver Nick Crews
2019-08-22 19:33 ` Enric Balletbo i Serra
@ 2019-08-22 19:43 ` Nick Crews
1 sibling, 0 replies; 3+ messages in thread
From: Nick Crews @ 2019-08-22 19:43 UTC (permalink / raw)
To: Enric Balletbo i Serra, Benson Leung
Cc: linux-kernel, Duncan Laurie, Daniel Kurtz, Dmitry Torokhov, Simon Glass
Friendly bump on this :)
On Mon, Aug 5, 2019 at 2:22 PM Nick Crews <ncrews@chromium.org> wrote:
>
> Add the GET_BATT_PPID_INFO=0x8A command to the allowlist of accepted
> telemetry commands. In addition, since this new command requires
> verifying the contents of some of the arguments, I also restructure
> the request to use a union of the argument structs. Also, zero out the
> request buffer before each request, and change "whitelist" to
> "allowlist".
>
> Signed-off-by: Nick Crews <ncrews@chromium.org>
> ---
> drivers/platform/chrome/wilco_ec/telemetry.c | 64 +++++++++++++-------
> 1 file changed, 43 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/platform/chrome/wilco_ec/telemetry.c b/drivers/platform/chrome/wilco_ec/telemetry.c
> index 94cdc166c840..b9d03c33d8dc 100644
> --- a/drivers/platform/chrome/wilco_ec/telemetry.c
> +++ b/drivers/platform/chrome/wilco_ec/telemetry.c
> @@ -9,7 +9,7 @@
> * the OS sends a command to the EC via a write() to a char device,
> * and can read the response with a read(). The write() request is
> * verified by the driver to ensure that it is performing only one
> - * of the whitelisted commands, and that no extraneous data is
> + * of the allowlisted commands, and that no extraneous data is
> * being transmitted to the EC. The response is passed directly
> * back to the reader with no modification.
> *
> @@ -59,21 +59,10 @@ static DEFINE_IDA(telem_ida);
> #define WILCO_EC_TELEM_GET_TEMP_INFO 0x95
> #define WILCO_EC_TELEM_GET_TEMP_READ 0x2C
> #define WILCO_EC_TELEM_GET_BATT_EXT_INFO 0x07
> +#define WILCO_EC_TELEM_GET_BATT_PPID_INFO 0x8A
>
> #define TELEM_ARGS_SIZE_MAX 30
>
> -/**
> - * struct wilco_ec_telem_request - Telemetry command and arguments sent to EC.
> - * @command: One of WILCO_EC_TELEM_GET_* command codes.
> - * @reserved: Must be 0.
> - * @args: The first N bytes are one of telem_args_get_* structs, the rest is 0.
> - */
> -struct wilco_ec_telem_request {
> - u8 command;
> - u8 reserved;
> - u8 args[TELEM_ARGS_SIZE_MAX];
> -} __packed;
> -
> /*
> * The following telem_args_get_* structs are embedded within the |args| field
> * of wilco_ec_telem_request.
> @@ -122,6 +111,32 @@ struct telem_args_get_batt_ext_info {
> u8 var_args[5];
> } __packed;
>
> +struct telem_args_get_batt_ppid_info {
> + u8 always1; /* Should always be 1 */
> +} __packed;
> +
> +/**
> + * struct wilco_ec_telem_request - Telemetry command and arguments sent to EC.
> + * @command: One of WILCO_EC_TELEM_GET_* command codes.
> + * @reserved: Must be 0.
> + * @args: The first N bytes are one of telem_args_get_* structs, the rest is 0.
> + */
> +struct wilco_ec_telem_request {
> + u8 command;
> + u8 reserved;
> + union {
> + u8 buf[TELEM_ARGS_SIZE_MAX];
> + struct telem_args_get_log get_log;
> + struct telem_args_get_version get_version;
> + struct telem_args_get_fan_info get_fan_info;
> + struct telem_args_get_diag_info get_diag_info;
> + struct telem_args_get_temp_info get_temp_info;
> + struct telem_args_get_temp_read get_temp_read;
> + struct telem_args_get_batt_ext_info get_batt_ext_info;
> + struct telem_args_get_batt_ppid_info get_batt_ppid_info;
> + } args;
> +} __packed;
> +
> /**
> * check_telem_request() - Ensure that a request from userspace is valid.
> * @rq: Request buffer copied from userspace.
> @@ -133,7 +148,7 @@ struct telem_args_get_batt_ext_info {
> * We do not want to allow userspace to send arbitrary telemetry commands to
> * the EC. Therefore we check to ensure that
> * 1. The request follows the format of struct wilco_ec_telem_request.
> - * 2. The supplied command code is one of the whitelisted commands.
> + * 2. The supplied command code is one of the allowlisted commands.
> * 3. The request only contains the necessary data for the header and arguments.
> */
> static int check_telem_request(struct wilco_ec_telem_request *rq,
> @@ -146,25 +161,31 @@ static int check_telem_request(struct wilco_ec_telem_request *rq,
>
> switch (rq->command) {
> case WILCO_EC_TELEM_GET_LOG:
> - max_size += sizeof(struct telem_args_get_log);
> + max_size += sizeof(rq->args.get_log);
> break;
> case WILCO_EC_TELEM_GET_VERSION:
> - max_size += sizeof(struct telem_args_get_version);
> + max_size += sizeof(rq->args.get_version);
> break;
> case WILCO_EC_TELEM_GET_FAN_INFO:
> - max_size += sizeof(struct telem_args_get_fan_info);
> + max_size += sizeof(rq->args.get_fan_info);
> break;
> case WILCO_EC_TELEM_GET_DIAG_INFO:
> - max_size += sizeof(struct telem_args_get_diag_info);
> + max_size += sizeof(rq->args.get_diag_info);
> break;
> case WILCO_EC_TELEM_GET_TEMP_INFO:
> - max_size += sizeof(struct telem_args_get_temp_info);
> + max_size += sizeof(rq->args.get_temp_info);
> break;
> case WILCO_EC_TELEM_GET_TEMP_READ:
> - max_size += sizeof(struct telem_args_get_temp_read);
> + max_size += sizeof(rq->args.get_temp_read);
> break;
> case WILCO_EC_TELEM_GET_BATT_EXT_INFO:
> - max_size += sizeof(struct telem_args_get_batt_ext_info);
> + max_size += sizeof(rq->args.get_batt_ext_info);
> + break;
> + case WILCO_EC_TELEM_GET_BATT_PPID_INFO:
> + if (rq->args.get_batt_ppid_info.always1 != 1)
> + return -EINVAL;
> +
> + max_size += sizeof(rq->args.get_batt_ppid_info);
> break;
> default:
> return -EINVAL;
> @@ -250,6 +271,7 @@ static ssize_t telem_write(struct file *filp, const char __user *buf,
>
> if (count > sizeof(sess_data->request))
> return -EMSGSIZE;
> + memset(&sess_data->request, 0, sizeof(sess_data->request));
> if (copy_from_user(&sess_data->request, buf, count))
> return -EFAULT;
> ret = check_telem_request(&sess_data->request, count);
> --
> 2.20.1
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-08-22 19:43 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-05 20:22 [PATCH] platform/chrome: wilco_ec: Add batt_ppid_info command to telemetry driver Nick Crews
2019-08-22 19:33 ` Enric Balletbo i Serra
2019-08-22 19:43 ` Nick Crews
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®