[PATCH] efi/capsule: Make efi_capsule_pending() lockless

[PATCH] efi/capsule: Make efi_capsule_pending() lockless

[Matt Fleming]

Taking a mutex in the reboot path is bogus because we cannot sleep
with interrupts disabled, such as when rebooting due to panic(),

 [   18.069005] BUG: sleeping function called from invalid context at kernel/locking/mutex.c:97
 [   18.071639] in_atomic(): 0, irqs_disabled(): 1, pid: 7, name: rcu_sched
 [   18.085940] Call Trace:
 [   18.086911]  [<ffffffff8142e53a>] dump_stack+0x63/0x89
 [   18.088260]  [<ffffffff810a1048>] ___might_sleep+0xd8/0x120
 [   18.089860]  [<ffffffff810a10d9>] __might_sleep+0x49/0x80
 [   18.091272]  [<ffffffff818f5110>] mutex_lock+0x20/0x50
 [   18.092636]  [<ffffffff81771edd>] efi_capsule_pending+0x1d/0x60
 [   18.094272]  [<ffffffff8104e749>] native_machine_emergency_restart+0x59/0x280
 [   18.095975]  [<ffffffff8104e5d9>] machine_emergency_restart+0x19/0x20
 [   18.097685]  [<ffffffff8109d4b8>] emergency_restart+0x18/0x20
 [   18.099303]  [<ffffffff81172d6d>] panic+0x1ba/0x217

In this case all other CPUs will have been stopped by the time we
execute the platform reboot code, so 'capsule_pending' cannot change
under our feet. We wouldn't care even if it could since we cannot wait
for it complete.

Also, instead of relying on the external 'system_state' variable just
use a reboot notifier, so we can set 'stop_capsules' while holding
'capsule_mutex', thereby avoiding a race where system_state is updated
while we're in the middle of efi_capsule_update_locked() (since CPUs
won't have been stopped at that point).

Cc: Borislav Petkov <bp-Gina5bIWoIWzQB+pC5nmwQ@public.gmane.org>
Cc: Kweh Hock Leong <hock.leong.kweh-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Cc: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: Bryan O'Donoghue <pure.logic-SyKdqv6vbfZdzvEItQ6vdLNAH6kLmebB@public.gmane.org>
Cc: joeyli <jlee-IBi9RG/b67k@public.gmane.org>
Signed-off-by: Matt Fleming <matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org>
---
 drivers/firmware/efi/capsule.c | 36 ++++++++++++++++++++++++++----------
 1 file changed, 26 insertions(+), 10 deletions(-)

diff --git a/drivers/firmware/efi/capsule.c b/drivers/firmware/efi/capsule.c
index 0de55944ac0b..4703dc9b8fbd 100644
--- a/drivers/firmware/efi/capsule.c
+++ b/drivers/firmware/efi/capsule.c
@@ -22,11 +22,12 @@ typedef struct {
 } efi_capsule_block_desc_t;
 
 static bool capsule_pending;
+static bool stop_capsules;
 static int efi_reset_type = -1;
 
 /*
  * capsule_mutex serialises access to both capsule_pending and
- * efi_reset_type.
+ * efi_reset_type and stop_capsules.
  */
 static DEFINE_MUTEX(capsule_mutex);
 
@@ -50,18 +51,13 @@ static DEFINE_MUTEX(capsule_mutex);
  */
 bool efi_capsule_pending(int *reset_type)
 {
-	bool rv = false;
-
-	mutex_lock(&capsule_mutex);
 	if (!capsule_pending)
-		goto out;
+		return false;
 
 	if (reset_type)
 		*reset_type = efi_reset_type;
-	rv = true;
-out:
-	mutex_unlock(&capsule_mutex);
-	return rv;
+
+	return true;
 }
 
 /*
@@ -176,7 +172,7 @@ efi_capsule_update_locked(efi_capsule_header_t *capsule,
 	 * whether to force an EFI reboot), and we're racing against
 	 * that call. Abort in that case.
 	 */
-	if (unlikely(system_state == SYSTEM_RESTART)) {
+	if (unlikely(stop_capsules)) {
 		pr_warn("Capsule update raced with reboot, aborting.\n");
 		return -EINVAL;
 	}
@@ -298,3 +294,23 @@ out:
 	return rv;
 }
 EXPORT_SYMBOL_GPL(efi_capsule_update);
+
+static int capsule_reboot_notify(struct notifier_block *nb,
+				 unsigned long event, void *cmd)
+{
+	mutex_lock(&capsule_mutex);
+	stop_capsules = true;
+	mutex_unlock(&capsule_mutex);
+
+	return NOTIFY_DONE;
+}
+
+static struct notifier_block capsule_reboot_nb = {
+	.notifier_call = capsule_reboot_notify,
+};
+
+static int __init capsule_reboot_register(void)
+{
+	return register_reboot_notifier(&capsule_reboot_nb);
+}
+core_initcall(capsule_reboot_register);
-- 
2.7.3

Re: [PATCH] efi/capsule: Make efi_capsule_pending() lockless

[Borislav Petkov]

On Sat, Apr 30, 2016 at 11:13:27PM +0100, Matt Fleming wrote:
> Taking a mutex in the reboot path is bogus because we cannot sleep
> with interrupts disabled, such as when rebooting due to panic(),
> 
>  [   18.069005] BUG: sleeping function called from invalid context at kernel/locking/mutex.c:97
>  [   18.071639] in_atomic(): 0, irqs_disabled(): 1, pid: 7, name: rcu_sched
>  [   18.085940] Call Trace:
>  [   18.086911]  [<ffffffff8142e53a>] dump_stack+0x63/0x89
>  [   18.088260]  [<ffffffff810a1048>] ___might_sleep+0xd8/0x120
>  [   18.089860]  [<ffffffff810a10d9>] __might_sleep+0x49/0x80
>  [   18.091272]  [<ffffffff818f5110>] mutex_lock+0x20/0x50
>  [   18.092636]  [<ffffffff81771edd>] efi_capsule_pending+0x1d/0x60
>  [   18.094272]  [<ffffffff8104e749>] native_machine_emergency_restart+0x59/0x280
>  [   18.095975]  [<ffffffff8104e5d9>] machine_emergency_restart+0x19/0x20
>  [   18.097685]  [<ffffffff8109d4b8>] emergency_restart+0x18/0x20
>  [   18.099303]  [<ffffffff81172d6d>] panic+0x1ba/0x217

Please remove all stuff in [] and the offsets and leave only the call
trace with the function names. The numbers are useless and only get in
the way.

> In this case all other CPUs will have been stopped by the time we
> execute the platform reboot code, so 'capsule_pending' cannot change
> under our feet. We wouldn't care even if it could since we cannot wait
> for it complete.
> 
> Also, instead of relying on the external 'system_state' variable just
> use a reboot notifier, so we can set 'stop_capsules' while holding
> 'capsule_mutex', thereby avoiding a race where system_state is updated
> while we're in the middle of efi_capsule_update_locked() (since CPUs
> won't have been stopped at that point).

So I'm wondering: why can't we push all that logic higher up the API
chain, say in efi_capsule_open() or efi_capsule_write() or so?

I mean, userspace can either reboot *or* update capsules but both? Both
would be kinda nasty, like, 2 admins doing that stuff in parallel...

> 
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: Kweh Hock Leong <hock.leong.kweh@intel.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Bryan O'Donoghue <pure.logic@nexus-software.ie>
> Cc: joeyli <jlee@suse.com>
> Signed-off-by: Matt Fleming <matt@codeblueprint.co.uk>
> ---
>  drivers/firmware/efi/capsule.c | 36 ++++++++++++++++++++++++++----------
>  1 file changed, 26 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/firmware/efi/capsule.c b/drivers/firmware/efi/capsule.c
> index 0de55944ac0b..4703dc9b8fbd 100644
> --- a/drivers/firmware/efi/capsule.c
> +++ b/drivers/firmware/efi/capsule.c
> @@ -22,11 +22,12 @@ typedef struct {
>  } efi_capsule_block_desc_t;
>  
>  static bool capsule_pending;
> +static bool stop_capsules;
>  static int efi_reset_type = -1;
>  
>  /*
>   * capsule_mutex serialises access to both capsule_pending and
> - * efi_reset_type.
> + * efi_reset_type and stop_capsules.
>   */
>  static DEFINE_MUTEX(capsule_mutex);
>  
> @@ -50,18 +51,13 @@ static DEFINE_MUTEX(capsule_mutex);
>   */
>  bool efi_capsule_pending(int *reset_type)
>  {
> -	bool rv = false;
> -
> -	mutex_lock(&capsule_mutex);
>  	if (!capsule_pending)
> -		goto out;
> +		return false;
>  
>  	if (reset_type)
>  		*reset_type = efi_reset_type;
> -	rv = true;
> -out:
> -	mutex_unlock(&capsule_mutex);
> -	return rv;
> +
> +	return true;
>  }
>  
>  /*
> @@ -176,7 +172,7 @@ efi_capsule_update_locked(efi_capsule_header_t *capsule,
>  	 * whether to force an EFI reboot), and we're racing against
>  	 * that call. Abort in that case.
>  	 */
> -	if (unlikely(system_state == SYSTEM_RESTART)) {
> +	if (unlikely(stop_capsules)) {
>  		pr_warn("Capsule update raced with reboot, aborting.\n");
>  		return -EINVAL;
>  	}

... also, what happens if stop_capsules gets set here

<---

after the check? We will continue trying to update but the box is
already rebooting too. I think to solve this here properly, you'd need
to be able to hold off reboot temporarily until you either update the
capsule successfully or fail.

But that sounds nasty.

So the first case where the capsule update is started after reboot is
easy: the reboot notifier disables capsules update, done.

The second one where a reboot comes *after* the capsule update has
started is a bit tricky. My current thinking is to maybe disable
virt_efi_update_capsule() with the reboot notifier. But we'd still need
some sort of a synchronization with reboot there too, if we want to be
absolutely clean.

Something like make both the reboot notifier and
virt_efi_update_capsule() grab efi_runtime_lock or so which
virt_efi_update_capsule() already does...

I could very well be missing something though...

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

Re: [PATCH] efi/capsule: Make efi_capsule_pending() lockless

[Matt Fleming]

On Tue, 03 May, at 11:02:29AM, Borislav Petkov wrote:
> On Sat, Apr 30, 2016 at 11:13:27PM +0100, Matt Fleming wrote:
> > Taking a mutex in the reboot path is bogus because we cannot sleep
> > with interrupts disabled, such as when rebooting due to panic(),
> > 
> >  [   18.069005] BUG: sleeping function called from invalid context at kernel/locking/mutex.c:97
> >  [   18.071639] in_atomic(): 0, irqs_disabled(): 1, pid: 7, name: rcu_sched
> >  [   18.085940] Call Trace:
> >  [   18.086911]  [<ffffffff8142e53a>] dump_stack+0x63/0x89
> >  [   18.088260]  [<ffffffff810a1048>] ___might_sleep+0xd8/0x120
> >  [   18.089860]  [<ffffffff810a10d9>] __might_sleep+0x49/0x80
> >  [   18.091272]  [<ffffffff818f5110>] mutex_lock+0x20/0x50
> >  [   18.092636]  [<ffffffff81771edd>] efi_capsule_pending+0x1d/0x60
> >  [   18.094272]  [<ffffffff8104e749>] native_machine_emergency_restart+0x59/0x280
> >  [   18.095975]  [<ffffffff8104e5d9>] machine_emergency_restart+0x19/0x20
> >  [   18.097685]  [<ffffffff8109d4b8>] emergency_restart+0x18/0x20
> >  [   18.099303]  [<ffffffff81172d6d>] panic+0x1ba/0x217
> 
> Please remove all stuff in [] and the offsets and leave only the call
> trace with the function names. The numbers are useless and only get in
> the way.
 
OK.

> > In this case all other CPUs will have been stopped by the time we
> > execute the platform reboot code, so 'capsule_pending' cannot change
> > under our feet. We wouldn't care even if it could since we cannot wait
> > for it complete.
> > 
> > Also, instead of relying on the external 'system_state' variable just
> > use a reboot notifier, so we can set 'stop_capsules' while holding
> > 'capsule_mutex', thereby avoiding a race where system_state is updated
> > while we're in the middle of efi_capsule_update_locked() (since CPUs
> > won't have been stopped at that point).
> 
> So I'm wondering: why can't we push all that logic higher up the API
> chain, say in efi_capsule_open() or efi_capsule_write() or so?
> 
> I mean, userspace can either reboot *or* update capsules but both? Both
> would be kinda nasty, like, 2 admins doing that stuff in parallel...
 
We can find ourselves in the reboot code even if the admin has not
executed the reboot command. The trace above shows we entered because
the kernel panic()'d and it was booted with panic=-1.

Additionally, if you grep for kernel_restart() and you'll see other
ways we can be rebooted outside of the admin's control.
 
> > 
> > Cc: Borislav Petkov <bp-Gina5bIWoIWzQB+pC5nmwQ@public.gmane.org>
> > Cc: Kweh Hock Leong <hock.leong.kweh-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > Cc: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> > Cc: Bryan O'Donoghue <pure.logic-SyKdqv6vbfZdzvEItQ6vdLNAH6kLmebB@public.gmane.org>
> > Cc: joeyli <jlee-IBi9RG/b67k@public.gmane.org>
> > Signed-off-by: Matt Fleming <matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org>
> > ---
> >  drivers/firmware/efi/capsule.c | 36 ++++++++++++++++++++++++++----------
> >  1 file changed, 26 insertions(+), 10 deletions(-)
> > 
> > diff --git a/drivers/firmware/efi/capsule.c b/drivers/firmware/efi/capsule.c
> > index 0de55944ac0b..4703dc9b8fbd 100644
> > --- a/drivers/firmware/efi/capsule.c
> > +++ b/drivers/firmware/efi/capsule.c
> > @@ -22,11 +22,12 @@ typedef struct {
> >  } efi_capsule_block_desc_t;
> >  
> >  static bool capsule_pending;
> > +static bool stop_capsules;
> >  static int efi_reset_type = -1;
> >  
> >  /*
> >   * capsule_mutex serialises access to both capsule_pending and
> > - * efi_reset_type.
> > + * efi_reset_type and stop_capsules.
> >   */
> >  static DEFINE_MUTEX(capsule_mutex);
> >  
> > @@ -50,18 +51,13 @@ static DEFINE_MUTEX(capsule_mutex);
> >   */
> >  bool efi_capsule_pending(int *reset_type)
> >  {
> > -	bool rv = false;
> > -
> > -	mutex_lock(&capsule_mutex);
> >  	if (!capsule_pending)
> > -		goto out;
> > +		return false;
> >  
> >  	if (reset_type)
> >  		*reset_type = efi_reset_type;
> > -	rv = true;
> > -out:
> > -	mutex_unlock(&capsule_mutex);
> > -	return rv;
> > +
> > +	return true;
> >  }
> >  
> >  /*
> > @@ -176,7 +172,7 @@ efi_capsule_update_locked(efi_capsule_header_t *capsule,
> >  	 * whether to force an EFI reboot), and we're racing against
> >  	 * that call. Abort in that case.
> >  	 */
> > -	if (unlikely(system_state == SYSTEM_RESTART)) {
> > +	if (unlikely(stop_capsules)) {
> >  		pr_warn("Capsule update raced with reboot, aborting.\n");
> >  		return -EINVAL;
> >  	}
> 
> ... also, what happens if stop_capsules gets set here
> 

It can't, capsule_mutex prevents that.

> <---
> 
> after the check? We will continue trying to update but the box is
> already rebooting too. I think to solve this here properly, you'd need
> to be able to hold off reboot temporarily until you either update the
> capsule successfully or fail.
> 
> But that sounds nasty.
 
Right. You could find yourself in this situation if you're in the
middle of a capsule update and the box panics and reboots. Like you
said, there's really not much you can do there to ensure the update
completes. Your best option is to just not block and hang the machine.

> So the first case where the capsule update is started after reboot is
> easy: the reboot notifier disables capsules update, done.
 
Yep.

> The second one where a reboot comes *after* the capsule update has
> started is a bit tricky. My current thinking is to maybe disable
> virt_efi_update_capsule() with the reboot notifier. But we'd still need
> some sort of a synchronization with reboot there too, if we want to be
> absolutely clean.
> 
> Something like make both the reboot notifier and
> virt_efi_update_capsule() grab efi_runtime_lock or so which
> virt_efi_update_capsule() already does...
> 
> I could very well be missing something though...

Note that in the panic() -> emergency_restart() case the reboot
notifiers are not called at all.

Re: [PATCH] efi/capsule: Make efi_capsule_pending() lockless

[Borislav Petkov]

On Tue, May 03, 2016 at 03:12:01PM +0100, Matt Fleming wrote:
> We can find ourselves in the reboot code even if the admin has not
> executed the reboot command. The trace above shows we entered because
> the kernel panic()'d and it was booted with panic=-1.

I knew you were gonna say something like that...

> Right. You could find yourself in this situation if you're in the
> middle of a capsule update and the box panics and reboots. Like you
> said, there's really not much you can do there to ensure the update
> completes. Your best option is to just not block and hang the machine.

Hmmm, so panic() does bust_spinlocks() and efi_capsule_pending() could
look at oops_in_progress which is set by bust_spinlocks() and that would
probably solve the panic case but maybe the normal reboot case would
still hang...

> Note that in the panic() -> emergency_restart() case the reboot
> notifiers are not called at all.

And then there's that.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

Re: [PATCH] efi/capsule: Make efi_capsule_pending() lockless

[Matt Fleming]

On Wed, 04 May, at 11:30:31AM, Borislav Petkov wrote:
> 
> Hmmm, so panic() does bust_spinlocks() and efi_capsule_pending() could
> look at oops_in_progress which is set by bust_spinlocks() and that would
> probably solve the panic case but maybe the normal reboot case would
> still hang...
 
But emergency_restart() is called after bust_spinlocks(0) which can
reset oops_in_progress, so can't even use that to solve the panic
case.

Re: [PATCH] efi/capsule: Make efi_capsule_pending() lockless

[Borislav Petkov]

On Wed, May 04, 2016 at 12:46:05PM +0100, Matt Fleming wrote:
> But emergency_restart() is called after bust_spinlocks(0) which can
> reset oops_in_progress, so can't even use that to solve the panic
> case.

Blergh.

So I guess you need an explicit call efi_stop_capsules() somewhere in
the reboot path and be done with it. No reboot notifiers, no bla bla.
Just one big hammer which STFU the EFI.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

Re: [PATCH] efi/capsule: Make efi_capsule_pending() lockless

[Matt Fleming]

On Wed, 04 May, at 02:20:42PM, Borislav Petkov wrote:
> On Wed, May 04, 2016 at 12:46:05PM +0100, Matt Fleming wrote:
> > But emergency_restart() is called after bust_spinlocks(0) which can
> > reset oops_in_progress, so can't even use that to solve the panic
> > case.
> 
> Blergh.
> 
> So I guess you need an explicit call efi_stop_capsules() somewhere in
> the reboot path and be done with it. No reboot notifiers, no bla bla.
> Just one big hammer which STFU the EFI.

But you can't block in the reboot path and you can't guarantee you'll
interrupt an in-progress EFI capsule update call that started before
you entered the reboot path.

Re: [PATCH] efi/capsule: Make efi_capsule_pending() lockless

[Matt Fleming]

On Wed, 04 May, at 02:20:42PM, Borislav Petkov wrote:
> 
> Blergh.

Wilson, Bryan, what kind of rollback support does the Intel Quark have
if its firmware update is interrupted?

The interruption could be for a number of reasons including power
loss, or the example in this case, rebooting due to panic().

RE: [PATCH] efi/capsule: Make efi_capsule_pending() lockless

[Kweh, Hock Leong]

> -----Original Message-----
> From: Matt Fleming [mailto:matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org]
> Sent: Wednesday, May 04, 2016 10:36 PM
> To: Kweh, Hock Leong; Bryan O'Donoghue
> Cc: linux-efi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Ard Biesheuvel;
> joeyli; Borislav Petkov
> Subject: Re: [PATCH] efi/capsule: Make efi_capsule_pending() lockless
> 
> On Wed, 04 May, at 02:20:42PM, Borislav Petkov wrote:
> >
> > Blergh.
> 
> Wilson, Bryan, what kind of rollback support does the Intel Quark have if its
> firmware update is interrupted?
> 
> The interruption could be for a number of reasons including power loss, or
> the example in this case, rebooting due to panic().

If not mistaken, the EFI firmware will not update a partially uploaded binary due to checksum error.
User is required to re-update the efi capsule again on the next boot up.


Regards,
Wilson

Re: [PATCH] efi/capsule: Make efi_capsule_pending() lockless

[Matt Fleming]

On Thu, 05 May, at 02:27:16PM, Kweh, Hock Leong wrote:
> 
> If not mistaken, the EFI firmware will not update a partially uploaded binary due to checksum error.
> User is required to re-update the efi capsule again on the next boot up.

Ah, so the capsule is only processed by the firmware after rebooting?
That makes sense and alleviates my concerns about rebooting while in
the middle of efi_capsule_update().

Thanks Wilson!

Re: [PATCH] efi/capsule: Make efi_capsule_pending() lockless

[One Thousand Gnomes]

On Thu, 5 May 2016 15:36:43 +0100
Matt Fleming <matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org> wrote:

> On Thu, 05 May, at 02:27:16PM, Kweh, Hock Leong wrote:
> > 
> > If not mistaken, the EFI firmware will not update a partially uploaded binary due to checksum error.
> > User is required to re-update the efi capsule again on the next boot up.  
> 
> Ah, so the capsule is only processed by the firmware after rebooting?
> That makes sense and alleviates my concerns about rebooting while in
> the middle of efi_capsule_update().

Yes - and in many cases the actions the firmware capsule update does are
done in a manner which once you exit the firmware cannot be done by the
OS. It acts as a way to communicate a block of (typically signed)
firmware to more hardware priviliged boot firmware.

Alan

Re: [PATCH] efi/capsule: Make efi_capsule_pending() lockless

[Bryan O'Donoghue]

On Thu, 2016-05-05 at 14:27 +0000, Kweh, Hock Leong wrote:
> > -----Original Message-----
> > From: Matt Fleming [mailto:matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org]
> > Sent: Wednesday, May 04, 2016 10:36 PM
> > To: Kweh, Hock Leong; Bryan O'Donoghue
> > Cc: linux-efi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Ard
> > Biesheuvel;
> > joeyli; Borislav Petkov
> > Subject: Re: [PATCH] efi/capsule: Make efi_capsule_pending()
> > lockless
> > 
> > On Wed, 04 May, at 02:20:42PM, Borislav Petkov wrote:
> > > 
> > > Blergh.
> > 
> > Wilson, Bryan, what kind of rollback support does the Intel Quark
> > have if its
> > firmware update is interrupted?
> > 
> > The interruption could be for a number of reasons including power
> > loss, or
> > the example in this case, rebooting due to panic().
> 
> If not mistaken, the EFI firmware will not update a partially
> uploaded binary due to checksum error.
> User is required to re-update the efi capsule again on the next boot
> up.
> 

If the checksum fails then you're fine since you won't update flash.

OTOH if you pull the plug we actually have a backup image - so even a
partially flashed update shouldn't brick the system.

How well that actually works i.e. is it tested in anger ? Meh - YMMV
there fore sure.

---
bod