linux-kernel - Re: [PATCH] platform: cros_ec_debugfs: control uptime information request

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

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 21 May 2020 11:21:16 +0200
From:   Enric Balletbo i Serra <enric.balletbo@...labora.com>
To:     Gwendal Grignou <gwendal@...omium.org>, groeck@...omium.org,
        bleung@...omium.org, twawrzynczak@...omium.org
Cc:     linux-kernel@...r.kernel.org
Subject: Re: [PATCH] platform: cros_ec_debugfs: control uptime information
 request

Hi Gwendal,

Thank you for your patch.

On 21/5/20 7:28, Gwendal Grignou wrote:
> When EC does not support uptime command (EC_CMD_GET_UPTIME_INFO),
> return -EPROTO to read of /sys/kernel/debug/cros_ec/uptime without
> calling the EC after the first try.
> 
> The EC console log will not contain EC_CMD_GET_UPTIME_INFO anymore.
> 

Oh, what you mean with this? Uptime is only exposed via sysfs or I am missing
something.

> Signed-off-by: Gwendal Grignou <gwendal@...omium.org>
> ---
>  drivers/platform/chrome/cros_ec_debugfs.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/platform/chrome/cros_ec_debugfs.c b/drivers/platform/chrome/cros_ec_debugfs.c
> index 6ae484989d1f5..70a29afb6d9e7 100644
> --- a/drivers/platform/chrome/cros_ec_debugfs.c
> +++ b/drivers/platform/chrome/cros_ec_debugfs.c
> @@ -49,6 +49,8 @@ struct cros_ec_debugfs {
>  	struct delayed_work log_poll_work;
>  	/* EC panicinfo */
>  	struct debugfs_blob_wrapper panicinfo_blob;
> +	/* EC uptime */
> +	bool uptime_supported;

Ideally, if uptime can be supported or not we should only expose uptime when is
supported, so the sysfs entry should only be created when is supported, similar
to what we do with the console_log. See cros_ec_create_console_log()

If doing this doesn't break userspace, I'd prefer doing in that way.

Thanks,
 Enric


>  };
>  
>  /*
> @@ -256,12 +258,19 @@ static ssize_t cros_ec_uptime_read(struct file *file, char __user *user_buf,
>  	char read_buf[32];
>  	int ret;
>  
> +	if (!debug_info->uptime_supported)
> +		return -EPROTO;
> +
>  	resp = (struct ec_response_uptime_info *)&msg.resp;
>  
>  	msg.cmd.command = EC_CMD_GET_UPTIME_INFO;
>  	msg.cmd.insize = sizeof(*resp);
>  
>  	ret = cros_ec_cmd_xfer_status(ec_dev, &msg.cmd);
> +	if (ret == -EPROTO && msg.cmd.result == EC_RES_INVALID_COMMAND) {
> +		debug_info->uptime_supported = false;
> +		return ret;
> +	}
>  	if (ret < 0)
>  		return ret;
>  
> @@ -434,6 +443,9 @@ static int cros_ec_debugfs_probe(struct platform_device *pd)
>  	debug_info->ec = ec;
>  	debug_info->dir = debugfs_create_dir(name, NULL);
>  
> +	/* Give uptime a chance to run. */
> +	debug_info->uptime_supported = true;
> +
>  	ret = cros_ec_create_panicinfo(debug_info);
>  	if (ret)
>  		goto remove_debugfs;
>