Before commit c5459b829b71 ("LSM: Plumb visibility into optional "enabled"
state"), /sys/module/apparmor/parameters/enabled would show "Y" or "N"
since it was using the "bool" handler. After being changed to "int",
this switched to "1" or "0", breaking the userspace AppArmor detection
of dbus-broker. This restores the Y/N output while keeping the LSM
infrastructure happy.
Before:
$ cat /sys/module/apparmor/parameters/enabled
1
After:
$ cat /sys/module/apparmor/parameters/enabled
Y
Reported-by: David Rheinsberg <[email protected]>
Link: https://lkml.kernel.org/r/CADyDSO6k8vYb1eryT4g6+EHrLCvb68GAbHVWuULkYjcZcYNhhw@mail.gmail.com
Fixes: c5459b829b71 ("LSM: Plumb visibility into optional "enabled" state")
Signed-off-by: Kees Cook <[email protected]>
---
This fix, if John is okay with it, is needed in v5.1 to correct the
userspace regression reported by David.
---
security/apparmor/lsm.c | 49 ++++++++++++++++++++++++++++++++++++++++-
1 file changed, 48 insertions(+), 1 deletion(-)
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index 49d664ddff44..87500bde5a92 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -1336,9 +1336,16 @@ module_param_named(path_max, aa_g_path_max, aauint, S_IRUSR);
bool aa_g_paranoid_load = true;
module_param_named(paranoid_load, aa_g_paranoid_load, aabool, S_IRUGO);
+static int param_get_aaintbool(char *buffer, const struct kernel_param *kp);
+static int param_set_aaintbool(const char *val, const struct kernel_param *kp);
+#define param_check_aaintbool param_check_int
+static const struct kernel_param_ops param_ops_aaintbool = {
+ .set = param_set_aaintbool,
+ .get = param_get_aaintbool
+};
/* Boot time disable flag */
static int apparmor_enabled __lsm_ro_after_init = 1;
-module_param_named(enabled, apparmor_enabled, int, 0444);
+module_param_named(enabled, apparmor_enabled, aaintbool, 0444);
static int __init apparmor_enabled_setup(char *str)
{
@@ -1413,6 +1420,46 @@ static int param_get_aauint(char *buffer, const struct kernel_param *kp)
return param_get_uint(buffer, kp);
}
+/* Can only be set before AppArmor is initialized (i.e. on boot cmdline). */
+static int param_set_aaintbool(const char *val, const struct kernel_param *kp)
+{
+ struct kernel_param kp_local;
+ bool value;
+ int error;
+
+ if (apparmor_initialized)
+ return -EPERM;
+
+ /* Create local copy, with arg pointing to bool type. */
+ value = !!*((int *)kp->arg);
+ memcpy(&kp_local, kp, sizeof(kp_local));
+ kp_local.arg = &value;
+
+ error = param_set_bool(val, &kp_local);
+ if (!error)
+ *((int *)kp->arg) = *((bool *)kp_local.arg);
+ return error;
+}
+
+/*
+ * To avoid changing /sys/module/apparmor/parameters/enabled from Y/N to
+ * 1/0, this converts the "int that is actually bool" back to bool for
+ * display in the /sys filesystem, while keeping it "int" for the LSM
+ * infrastructure.
+ */
+static int param_get_aaintbool(char *buffer, const struct kernel_param *kp)
+{
+ struct kernel_param kp_local;
+ bool value;
+
+ /* Create local copy, with arg pointing to bool type. */
+ value = !!*((int *)kp->arg);
+ memcpy(&kp_local, kp, sizeof(kp_local));
+ kp_local.arg = &value;
+
+ return param_get_bool(buffer, &kp_local);
+}
+
static int param_get_audit(char *buffer, const struct kernel_param *kp)
{
if (!apparmor_enabled)
--
2.17.1
--
Kees Cook
On 4/8/19 9:07 AM, Kees Cook wrote:
> Before commit c5459b829b71 ("LSM: Plumb visibility into optional "enabled"
> state"), /sys/module/apparmor/parameters/enabled would show "Y" or "N"
> since it was using the "bool" handler. After being changed to "int",
> this switched to "1" or "0", breaking the userspace AppArmor detection
> of dbus-broker. This restores the Y/N output while keeping the LSM
> infrastructure happy.
>
> Before:
> $ cat /sys/module/apparmor/parameters/enabled
> 1
>
> After:
> $ cat /sys/module/apparmor/parameters/enabled
> Y
>
> Reported-by: David Rheinsberg <[email protected]>
> Link: https://lkml.kernel.org/r/CADyDSO6k8vYb1eryT4g6+EHrLCvb68GAbHVWuULkYjcZcYNhhw@mail.gmail.com
> Fixes: c5459b829b71 ("LSM: Plumb visibility into optional "enabled" state")
> Signed-off-by: Kees Cook <[email protected]>
> ---
> This fix, if John is okay with it, is needed in v5.1 to correct the
> userspace regression reported by David.
> ---
> security/apparmor/lsm.c | 49 ++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 48 insertions(+), 1 deletion(-)
>
> diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
> index 49d664ddff44..87500bde5a92 100644
> --- a/security/apparmor/lsm.c
> +++ b/security/apparmor/lsm.c
> @@ -1336,9 +1336,16 @@ module_param_named(path_max, aa_g_path_max, aauint, S_IRUSR);
> bool aa_g_paranoid_load = true;
> module_param_named(paranoid_load, aa_g_paranoid_load, aabool, S_IRUGO);
>
> +static int param_get_aaintbool(char *buffer, const struct kernel_param *kp);
> +static int param_set_aaintbool(const char *val, const struct kernel_param *kp);
> +#define param_check_aaintbool param_check_int
> +static const struct kernel_param_ops param_ops_aaintbool = {
> + .set = param_set_aaintbool,
> + .get = param_get_aaintbool
> +};
> /* Boot time disable flag */
> static int apparmor_enabled __lsm_ro_after_init = 1;
> -module_param_named(enabled, apparmor_enabled, int, 0444);
> +module_param_named(enabled, apparmor_enabled, aaintbool, 0444);
>
> static int __init apparmor_enabled_setup(char *str)
> {
> @@ -1413,6 +1420,46 @@ static int param_get_aauint(char *buffer, const struct kernel_param *kp)
> return param_get_uint(buffer, kp);
> }
>
> +/* Can only be set before AppArmor is initialized (i.e. on boot cmdline). */
> +static int param_set_aaintbool(const char *val, const struct kernel_param *kp)
> +{
> + struct kernel_param kp_local;
> + bool value;
> + int error;
> +
> + if (apparmor_initialized)
> + return -EPERM;
> +
This isn't sufficient/correct. apparmor_initialized is only set after
apparmor has gone through and completed initialization. However if
apparmor is not selected as one of the LSMs to enable, then this check
won't stop apparmor_enabled from being set post boot.
However with the apparmor_enabled param being 0444 and the
apparmor_enabled_setup() fn handling boot cmdline do with even need
the set parameter fn?
> + /* Create local copy, with arg pointing to bool type. */
> + value = !!*((int *)kp->arg);
> + memcpy(&kp_local, kp, sizeof(kp_local));
> + kp_local.arg = &value;
> +
> + error = param_set_bool(val, &kp_local);
> + if (!error)
> + *((int *)kp->arg) = *((bool *)kp_local.arg);
> + return error;
> +}
> +
> +/*
> + * To avoid changing /sys/module/apparmor/parameters/enabled from Y/N to
> + * 1/0, this converts the "int that is actually bool" back to bool for
> + * display in the /sys filesystem, while keeping it "int" for the LSM
> + * infrastructure.
> + */
> +static int param_get_aaintbool(char *buffer, const struct kernel_param *kp)
> +{
> + struct kernel_param kp_local;
> + bool value;
> +
> + /* Create local copy, with arg pointing to bool type. */
> + value = !!*((int *)kp->arg);
> + memcpy(&kp_local, kp, sizeof(kp_local));
> + kp_local.arg = &value;
> +
> + return param_get_bool(buffer, &kp_local);
> +}
> +
> static int param_get_audit(char *buffer, const struct kernel_param *kp)
> {
> if (!apparmor_enabled)
>
On Mon, Apr 8, 2019 at 9:58 AM John Johansen
<[email protected]> wrote:
> > +/* Can only be set before AppArmor is initialized (i.e. on boot cmdline). */
> > +static int param_set_aaintbool(const char *val, const struct kernel_param *kp)
> > +{
> > + struct kernel_param kp_local;
> > + bool value;
> > + int error;
> > +
> > + if (apparmor_initialized)
> > + return -EPERM;
> > +
> This isn't sufficient/correct. apparmor_initialized is only set after
> apparmor has gone through and completed initialization. However if
> apparmor is not selected as one of the LSMs to enable, then this check
> won't stop apparmor_enabled from being set post boot.
>
> However with the apparmor_enabled param being 0444 and the
> apparmor_enabled_setup() fn handling boot cmdline do with even need
> the set parameter fn?
Yup, that's true. I've gone and tested this, and yes, the 0444 is
sufficient to protect the logic here (even if root chmods the inode).
So the test here is redundant. However, very early in the threads
about LSM boot cmdline enabling it was made clear that
"apparmor.enabled=..." needed to stay working, which means the "set"
op is still needed. (But I'm happy to do whatever you want here -- I
was just trying to keep the functionality as it was.)
Should I send a v2 without the "initialized" check or is this okay to
leave as-is with the redundant check?
--
Kees Cook
On 4/8/19 10:25 AM, Kees Cook wrote:
> On Mon, Apr 8, 2019 at 9:58 AM John Johansen
> <[email protected]> wrote:
>>> +/* Can only be set before AppArmor is initialized (i.e. on boot cmdline). */
>>> +static int param_set_aaintbool(const char *val, const struct kernel_param *kp)
>>> +{
>>> + struct kernel_param kp_local;
>>> + bool value;
>>> + int error;
>>> +
>>> + if (apparmor_initialized)
>>> + return -EPERM;
>>> +
>> This isn't sufficient/correct. apparmor_initialized is only set after
>> apparmor has gone through and completed initialization. However if
>> apparmor is not selected as one of the LSMs to enable, then this check
>> won't stop apparmor_enabled from being set post boot.
>>
>> However with the apparmor_enabled param being 0444 and the
>> apparmor_enabled_setup() fn handling boot cmdline do with even need
>> the set parameter fn?
>
> Yup, that's true. I've gone and tested this, and yes, the 0444 is
> sufficient to protect the logic here (even if root chmods the inode).
> So the test here is redundant. However, very early in the threads
> about LSM boot cmdline enabling it was made clear that
> "apparmor.enabled=..." needed to stay working, which means the "set"
> op is still needed. (But I'm happy to do whatever you want here -- I
> was just trying to keep the functionality as it was.)
>
Right, though the legacy case that most document reference is apparmor=0/1
which is handled by __apparmor_enabled_setup()
still best not to break apparmor.enabled
> Should I send a v2 without the "initialized" check or is this okay to
> leave as-is with the redundant check?
>
redundant is fine, it will help protect against shooting ourselves in
the foot if some one ever tries changing the 0444
you can have my Acked-by: John Johansen <[email protected]>
Hi
On Mon, Apr 8, 2019 at 6:07 PM Kees Cook <[email protected]> wrote:
>
> Before commit c5459b829b71 ("LSM: Plumb visibility into optional "enabled"
> state"), /sys/module/apparmor/parameters/enabled would show "Y" or "N"
> since it was using the "bool" handler. After being changed to "int",
> this switched to "1" or "0", breaking the userspace AppArmor detection
> of dbus-broker. This restores the Y/N output while keeping the LSM
> infrastructure happy.
>
> Before:
> $ cat /sys/module/apparmor/parameters/enabled
> 1
>
> After:
> $ cat /sys/module/apparmor/parameters/enabled
> Y
>
> Reported-by: David Rheinsberg <[email protected]>
> Link: https://lkml.kernel.org/r/CADyDSO6k8vYb1eryT4g6+EHrLCvb68GAbHVWuULkYjcZcYNhhw@mail.gmail.com
> Fixes: c5459b829b71 ("LSM: Plumb visibility into optional "enabled" state")
> Signed-off-by: Kees Cook <[email protected]>
> ---
> This fix, if John is okay with it, is needed in v5.1 to correct the
> userspace regression reported by David.
> ---
> security/apparmor/lsm.c | 49 ++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 48 insertions(+), 1 deletion(-)
This looks good to me. Thanks a lot! If this makes v5.1, I will leave
the apparmor-detection in dbus-broker as it is, unless someone asks me
to parse 0/1 as well?
I cannot judge whether the apparmor_initialized check is correct, but
for the parameter parsing:
Reviewed-by: David Rheinsberg <[email protected]>
Thanks
David
> diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
> index 49d664ddff44..87500bde5a92 100644
> --- a/security/apparmor/lsm.c
> +++ b/security/apparmor/lsm.c
> @@ -1336,9 +1336,16 @@ module_param_named(path_max, aa_g_path_max, aauint, S_IRUSR);
> bool aa_g_paranoid_load = true;
> module_param_named(paranoid_load, aa_g_paranoid_load, aabool, S_IRUGO);
>
> +static int param_get_aaintbool(char *buffer, const struct kernel_param *kp);
> +static int param_set_aaintbool(const char *val, const struct kernel_param *kp);
> +#define param_check_aaintbool param_check_int
> +static const struct kernel_param_ops param_ops_aaintbool = {
> + .set = param_set_aaintbool,
> + .get = param_get_aaintbool
> +};
> /* Boot time disable flag */
> static int apparmor_enabled __lsm_ro_after_init = 1;
> -module_param_named(enabled, apparmor_enabled, int, 0444);
> +module_param_named(enabled, apparmor_enabled, aaintbool, 0444);
>
> static int __init apparmor_enabled_setup(char *str)
> {
> @@ -1413,6 +1420,46 @@ static int param_get_aauint(char *buffer, const struct kernel_param *kp)
> return param_get_uint(buffer, kp);
> }
>
> +/* Can only be set before AppArmor is initialized (i.e. on boot cmdline). */
> +static int param_set_aaintbool(const char *val, const struct kernel_param *kp)
> +{
> + struct kernel_param kp_local;
> + bool value;
> + int error;
> +
> + if (apparmor_initialized)
> + return -EPERM;
> +
> + /* Create local copy, with arg pointing to bool type. */
> + value = !!*((int *)kp->arg);
> + memcpy(&kp_local, kp, sizeof(kp_local));
> + kp_local.arg = &value;
> +
> + error = param_set_bool(val, &kp_local);
> + if (!error)
> + *((int *)kp->arg) = *((bool *)kp_local.arg);
> + return error;
> +}
> +
> +/*
> + * To avoid changing /sys/module/apparmor/parameters/enabled from Y/N to
> + * 1/0, this converts the "int that is actually bool" back to bool for
> + * display in the /sys filesystem, while keeping it "int" for the LSM
> + * infrastructure.
> + */
> +static int param_get_aaintbool(char *buffer, const struct kernel_param *kp)
> +{
> + struct kernel_param kp_local;
> + bool value;
> +
> + /* Create local copy, with arg pointing to bool type. */
> + value = !!*((int *)kp->arg);
> + memcpy(&kp_local, kp, sizeof(kp_local));
> + kp_local.arg = &value;
> +
> + return param_get_bool(buffer, &kp_local);
> +}
> +
> static int param_get_audit(char *buffer, const struct kernel_param *kp)
> {
> if (!apparmor_enabled)
> --
> 2.17.1
>
>
> --
> Kees Cook
On Mon, Apr 8, 2019 at 11:21 PM David Rheinsberg
<[email protected]> wrote:
>
> Hi
>
> On Mon, Apr 8, 2019 at 6:07 PM Kees Cook <[email protected]> wrote:
> >
> > Before commit c5459b829b71 ("LSM: Plumb visibility into optional "enabled"
> > state"), /sys/module/apparmor/parameters/enabled would show "Y" or "N"
> > since it was using the "bool" handler. After being changed to "int",
> > this switched to "1" or "0", breaking the userspace AppArmor detection
> > of dbus-broker. This restores the Y/N output while keeping the LSM
> > infrastructure happy.
> >
> > Before:
> > $ cat /sys/module/apparmor/parameters/enabled
> > 1
> >
> > After:
> > $ cat /sys/module/apparmor/parameters/enabled
> > Y
> >
> > Reported-by: David Rheinsberg <[email protected]>
> > Link: https://lkml.kernel.org/r/CADyDSO6k8vYb1eryT4g6+EHrLCvb68GAbHVWuULkYjcZcYNhhw@mail.gmail.com
> > Fixes: c5459b829b71 ("LSM: Plumb visibility into optional "enabled" state")
> > Signed-off-by: Kees Cook <[email protected]>
> > ---
> > This fix, if John is okay with it, is needed in v5.1 to correct the
> > userspace regression reported by David.
> > ---
> > security/apparmor/lsm.c | 49 ++++++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 48 insertions(+), 1 deletion(-)
>
> This looks good to me. Thanks a lot! If this makes v5.1, I will leave
> the apparmor-detection in dbus-broker as it is, unless someone asks me
> to parse 0/1 as well?
>
> I cannot judge whether the apparmor_initialized check is correct, but
> for the parameter parsing:
>
> Reviewed-by: David Rheinsberg <[email protected]>
Thanks!
James, are you able to take this for v5.1 fixes?
--
Kees Cook
On Tue, 9 Apr 2019, Kees Cook wrote:
> On Mon, Apr 8, 2019 at 11:21 PM David Rheinsberg
> <[email protected]> wrote:
> >
> > Hi
> >
> > On Mon, Apr 8, 2019 at 6:07 PM Kees Cook <[email protected]> wrote:
> > >
> > > Before commit c5459b829b71 ("LSM: Plumb visibility into optional "enabled"
> > > state"), /sys/module/apparmor/parameters/enabled would show "Y" or "N"
> > > since it was using the "bool" handler. After being changed to "int",
> > > this switched to "1" or "0", breaking the userspace AppArmor detection
> > > of dbus-broker. This restores the Y/N output while keeping the LSM
> > > infrastructure happy.
> > >
> > > Before:
> > > $ cat /sys/module/apparmor/parameters/enabled
> > > 1
> > >
> > > After:
> > > $ cat /sys/module/apparmor/parameters/enabled
> > > Y
> > >
> > > Reported-by: David Rheinsberg <[email protected]>
> > > Link: https://lkml.kernel.org/r/CADyDSO6k8vYb1eryT4g6+EHrLCvb68GAbHVWuULkYjcZcYNhhw@mail.gmail.com
> > > Fixes: c5459b829b71 ("LSM: Plumb visibility into optional "enabled" state")
> > > Signed-off-by: Kees Cook <[email protected]>
> > > ---
> > > This fix, if John is okay with it, is needed in v5.1 to correct the
> > > userspace regression reported by David.
> > > ---
> > > security/apparmor/lsm.c | 49 ++++++++++++++++++++++++++++++++++++++++-
> > > 1 file changed, 48 insertions(+), 1 deletion(-)
> >
> > This looks good to me. Thanks a lot! If this makes v5.1, I will leave
> > the apparmor-detection in dbus-broker as it is, unless someone asks me
> > to parse 0/1 as well?
> >
> > I cannot judge whether the apparmor_initialized check is correct, but
> > for the parameter parsing:
> >
> > Reviewed-by: David Rheinsberg <[email protected]>
>
> Thanks!
>
> James, are you able to take this for v5.1 fixes?
Sure.
--
James Morris
<[email protected]>
On Tue, 9 Apr 2019, Kees Cook wrote:
> On Mon, Apr 8, 2019 at 11:21 PM David Rheinsberg
> <[email protected]> wrote:
> >
> > Hi
> >
> > On Mon, Apr 8, 2019 at 6:07 PM Kees Cook <[email protected]> wrote:
> > >
> > > Before commit c5459b829b71 ("LSM: Plumb visibility into optional "enabled"
> > > state"), /sys/module/apparmor/parameters/enabled would show "Y" or "N"
> > > since it was using the "bool" handler. After being changed to "int",
> > > this switched to "1" or "0", breaking the userspace AppArmor detection
> > > of dbus-broker. This restores the Y/N output while keeping the LSM
> > > infrastructure happy.
> > >
> > > Before:
> > > $ cat /sys/module/apparmor/parameters/enabled
> > > 1
> > >
> > > After:
> > > $ cat /sys/module/apparmor/parameters/enabled
> > > Y
> > >
> > > Reported-by: David Rheinsberg <[email protected]>
> > > Link: https://lkml.kernel.org/r/CADyDSO6k8vYb1eryT4g6+EHrLCvb68GAbHVWuULkYjcZcYNhhw@mail.gmail.com
> > > Fixes: c5459b829b71 ("LSM: Plumb visibility into optional "enabled" state")
> > > Signed-off-by: Kees Cook <[email protected]>
> > > ---
> > > This fix, if John is okay with it, is needed in v5.1 to correct the
> > > userspace regression reported by David.
> > > ---
> > > security/apparmor/lsm.c | 49 ++++++++++++++++++++++++++++++++++++++++-
> > > 1 file changed, 48 insertions(+), 1 deletion(-)
> >
> > This looks good to me. Thanks a lot! If this makes v5.1, I will leave
> > the apparmor-detection in dbus-broker as it is, unless someone asks me
> > to parse 0/1 as well?
> >
> > I cannot judge whether the apparmor_initialized check is correct, but
> > for the parameter parsing:
> >
> > Reviewed-by: David Rheinsberg <[email protected]>
>
> Thanks!
>
> James, are you able to take this for v5.1 fixes?
Actually, JJ usually submits directly to Linus.
--
James Morris
<[email protected]>
On Tue, Apr 9, 2019 at 1:12 PM James Morris <[email protected]> wrote:
> Actually, JJ usually submits directly to Linus.
Ah! Right; I forgot. John, can you take and send this?
--
Kees Cook
On 4/9/19 1:11 PM, James Morris wrote:
> On Tue, 9 Apr 2019, Kees Cook wrote:
>
>> On Mon, Apr 8, 2019 at 11:21 PM David Rheinsberg
>> <[email protected]> wrote:
>>>
>>> Hi
>>>
>>> On Mon, Apr 8, 2019 at 6:07 PM Kees Cook <[email protected]> wrote:
>>>>
>>>> Before commit c5459b829b71 ("LSM: Plumb visibility into optional "enabled"
>>>> state"), /sys/module/apparmor/parameters/enabled would show "Y" or "N"
>>>> since it was using the "bool" handler. After being changed to "int",
>>>> this switched to "1" or "0", breaking the userspace AppArmor detection
>>>> of dbus-broker. This restores the Y/N output while keeping the LSM
>>>> infrastructure happy.
>>>>
>>>> Before:
>>>> $ cat /sys/module/apparmor/parameters/enabled
>>>> 1
>>>>
>>>> After:
>>>> $ cat /sys/module/apparmor/parameters/enabled
>>>> Y
>>>>
>>>> Reported-by: David Rheinsberg <[email protected]>
>>>> Link: https://lkml.kernel.org/r/CADyDSO6k8vYb1eryT4g6+EHrLCvb68GAbHVWuULkYjcZcYNhhw@mail.gmail.com
>>>> Fixes: c5459b829b71 ("LSM: Plumb visibility into optional "enabled" state")
>>>> Signed-off-by: Kees Cook <[email protected]>
>>>> ---
>>>> This fix, if John is okay with it, is needed in v5.1 to correct the
>>>> userspace regression reported by David.
>>>> ---
>>>> security/apparmor/lsm.c | 49 ++++++++++++++++++++++++++++++++++++++++-
>>>> 1 file changed, 48 insertions(+), 1 deletion(-)
>>>
>>> This looks good to me. Thanks a lot! If this makes v5.1, I will leave
>>> the apparmor-detection in dbus-broker as it is, unless someone asks me
>>> to parse 0/1 as well?
>>>
>>> I cannot judge whether the apparmor_initialized check is correct, but
>>> for the parameter parsing:
>>>
>>> Reviewed-by: David Rheinsberg <[email protected]>
>>
>> Thanks!
>>
>> James, are you able to take this for v5.1 fixes?
>
> Actually, JJ usually submits directly to Linus.
>
yeah, I can push this up
On 4/9/19 1:55 PM, Kees Cook wrote:
> On Tue, Apr 9, 2019 at 1:12 PM James Morris <[email protected]> wrote:
>> Actually, JJ usually submits directly to Linus.
>
> Ah! Right; I forgot. John, can you take and send this?
>
yep, I'll send it up today