From: Greg KH <gregkh@linuxfoundation.org>
To: Rajat Jain <rajatja@google.com>
Cc: Andreas Noever <andreas.noever@gmail.com>,
Michael Jamet <michael.jamet@intel.com>,
Mika Westerberg <mika.westerberg@linux.intel.com>,
Yehezkel Bernat <yehezkelshb@gmail.com>,
"open list:ULTRA-WIDEBAND (UWB) SUBSYSTEM:"
<linux-usb@vger.kernel.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
Rajat Jain <rajatxjain@gmail.com>
Subject: Re: [PATCH] thunderbolt: For dev authorization changes, include the actual event in udev change notification
Date: Mon, 26 Jul 2021 09:39:26 +0200 [thread overview]
Message-ID: <YP5mrnaI1hyAU5Wp@kroah.com> (raw)
In-Reply-To: <CACK8Z6EdBYWG7nv0ViumA72NK4h2G0cW9d2rn3BbHFTrLqhU-g@mail.gmail.com>
On Sun, Jul 25, 2021 at 11:46:32AM -0700, Rajat Jain wrote:
> 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 <rajatja@google.com> wrote:
> >
> > Hello,
> >
> >
> > On Fri, Jul 23, 2021 at 12:05 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> > >
> > > 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... :)
> >
> > Sorry, fixed now.
> >
> > >
> > > >
> > > > 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?
> >
> > 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/.
Perhaps a comment here showing that this is the same value as that
specific sysfs attribute as well?
> > > 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.
It's been a long time since I last touched the udev codebase, sorry, try
it out yourself and see!
thanks,
greg k-h
prev parent reply other threads:[~2021-07-26 7:39 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-23 1:28 [PATCH] thunderbolt: For dev authorization changes, include the actual event in udev change notification Rajat Jain
2021-07-23 7:05 ` Greg KH
[not found] ` <CACK8Z6FXLY8p=15JbYp3x3QvTgeWhmrRb_ACyNr+tNe68MOstw@mail.gmail.com>
2021-07-25 18:46 ` Rajat Jain
2021-07-26 7:39 ` Greg KH [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YP5mrnaI1hyAU5Wp@kroah.com \
--to=gregkh@linuxfoundation.org \
--cc=andreas.noever@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=michael.jamet@intel.com \
--cc=mika.westerberg@linux.intel.com \
--cc=rajatja@google.com \
--cc=rajatxjain@gmail.com \
--cc=yehezkelshb@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).