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  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;
> 

Powered by blists - more mailing lists