Sorry, Had hit "Reply"" while responding, instead of "Reply All" - so
it went only to Greg. Now added back everyone else.


On Fri, Jul 23, 2021 at 4:43 PM Rajat Jain [off-list ref] wrote:

Hello,


On Fri, Jul 23, 2021 at 12:05 AM Greg KH [off-list ref] wrote:

quoted

On Thu, Jul 22, 2021 at 06:28:34PM -0700, Rajat Jain wrote:

quoted

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... :)

Sorry, fixed now.

quoted

quoted

Signed-off-by: Rajat Jain <redacted>
---
 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?

Sorry I should have clarified and elaborated (now done in the new
commit log). The field / value being exposed is that of the existing
sysfs attribute "authorized"
(/sys/bus/thunderbolt/devices/.../authorized), which is already
documented. I made it clearer in the commit log now. I looked at other
uses of kobject_uevent_env() and couldn't find examples of documenting
the Udev environment in Documentation/.

quoted

And what userspace tool will be looking for this?

It will likely be a udev rule which will trigger a script when it see
device authorization change event. Something like this:
SUBSYSTEM=="thunderbolt", ACTION=="change", ENV{AUTHORIZED}=="1",
RUN+="alert.sh"

However, now that I say it, is it possible to check for such (kernel
supplied) udev event environment key value pair, using
udev_device_get_property_value()? If so, that makes it very easy for
us, and the tool to use it would be Chromeos daemon called
cros_healthd.

Thanks,

Rajat

quoted

thanks,

greg k-h