Re: [PATCH] thunderbolt: For dev authorization changes, include the actual event in udev change notification

[Greg KH <gregkh@linuxfoundation.org>]

On Thu, Jul 22, 2021 at 06:28:34PM -0700, Rajat Jain wrote:
> For security, we would like to monitor and track when the
> thunderbolt devices are authorized and deauthorized. Currently
> the userspace gets a udev change notification when there is a
> change, but the state may have changed (again) by the time we
> look at the authorized attribute in sysfs. So an authorization
> event may go unnoticed. Thus make it easier by informing the
> actual change (authorized/deauthorized) in the udev change
> notification.

We do have 72 columns to work with... :)

> 
> Signed-off-by: Rajat Jain <rajatja@google.com>
> ---
>  drivers/thunderbolt/switch.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c
> index 83b1ef3d5d03..5d3e9dcba44a 100644
> --- a/drivers/thunderbolt/switch.c
> +++ b/drivers/thunderbolt/switch.c
> @@ -1499,6 +1499,7 @@ static ssize_t authorized_show(struct device *dev,
>  static int disapprove_switch(struct device *dev, void *not_used)
>  {
>  	struct tb_switch *sw;
> +	char *envp[] = { "AUTHORIZED=0", NULL };
>  
>  	sw = tb_to_switch(dev);
>  	if (sw && sw->authorized) {
> @@ -1514,7 +1515,7 @@ static int disapprove_switch(struct device *dev, void *not_used)
>  			return ret;
>  
>  		sw->authorized = 0;
> -		kobject_uevent(&sw->dev.kobj, KOBJ_CHANGE);
> +		kobject_uevent_env(&sw->dev.kobj, KOBJ_CHANGE, envp);
>  	}
>  
>  	return 0;
> @@ -1523,6 +1524,8 @@ static int disapprove_switch(struct device *dev, void *not_used)
>  static int tb_switch_set_authorized(struct tb_switch *sw, unsigned int val)
>  {
>  	int ret = -EINVAL;
> +	char envp_string[13];
> +	char *envp[] = { envp_string, NULL };
>  
>  	if (!mutex_trylock(&sw->tb->lock))
>  		return restart_syscall();
> @@ -1560,7 +1563,8 @@ static int tb_switch_set_authorized(struct tb_switch *sw, unsigned int val)
>  	if (!ret) {
>  		sw->authorized = val;
>  		/* Notify status change to the userspace */
> -		kobject_uevent(&sw->dev.kobj, KOBJ_CHANGE);
> +		sprintf(envp_string, "AUTHORIZED=%u", val);
> +		kobject_uevent_env(&sw->dev.kobj, KOBJ_CHANGE, envp);

So now "val" is a userspace visable value?  Is that documented anywhere
what it is and what are you going to do to ensure it never changes in
the future?

Also this new value "field" should be documented somewhere as well,
otherwise how will any tool know it is there?

And what userspace tool will be looking for this?

thanks,

greg k-h