* [PATCH] generic arch_futex_atomic_op_inuser() cleanup
@ 2019-07-15 10:27 Vasily Averin
2019-07-15 10:27 ` Vasily Averin
` (2 more replies)
0 siblings, 3 replies; 14+ messages in thread
From: Vasily Averin @ 2019-07-15 10:27 UTC (permalink / raw)
To: linux-kernel, linux-arch, Thomas Gleixner, Ingo Molnar,
Arnd Bergmann
Access to 'op' variable does not require pagefault_disable(),
'ret' variable should be initialized before using,
'oldval' variable can be replaced by constant.
Signed-off-by: Vasily Averin <vvs@virtuozzo.com>
---
include/asm-generic/futex.h | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/include/asm-generic/futex.h b/include/asm-generic/futex.h
index 8666fe7f35d7..e9a9655d786d 100644
--- a/include/asm-generic/futex.h
+++ b/include/asm-generic/futex.h
@@ -118,9 +118,7 @@ futex_atomic_cmpxchg_inatomic(u32 *uval, u32 __user *uaddr,
static inline int
arch_futex_atomic_op_inuser(int op, u32 oparg, int *oval, u32 __user *uaddr)
{
- int oldval = 0, ret;
-
- pagefault_disable();
+ int ret = 0;
switch (op) {
case FUTEX_OP_SET:
@@ -132,10 +130,8 @@ arch_futex_atomic_op_inuser(int op, u32 oparg, int *oval, u32 __user *uaddr)
ret = -ENOSYS;
}
- pagefault_enable();
-
if (!ret)
- *oval = oldval;
+ *oval = 0;
return ret;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH] generic arch_futex_atomic_op_inuser() cleanup
2019-07-15 10:27 [PATCH] generic arch_futex_atomic_op_inuser() cleanup Vasily Averin
@ 2019-07-15 10:27 ` Vasily Averin
2019-07-15 10:29 ` Vasily Averin
2019-07-15 13:13 ` [PATCH] " Thomas Gleixner
2 siblings, 0 replies; 14+ messages in thread
From: Vasily Averin @ 2019-07-15 10:27 UTC (permalink / raw)
To: linux-kernel, linux-arch, Thomas Gleixner, Ingo Molnar,
Arnd Bergmann
Access to 'op' variable does not require pagefault_disable(),
'ret' variable should be initialized before using,
'oldval' variable can be replaced by constant.
Signed-off-by: Vasily Averin <vvs@virtuozzo.com>
---
include/asm-generic/futex.h | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/include/asm-generic/futex.h b/include/asm-generic/futex.h
index 8666fe7f35d7..e9a9655d786d 100644
--- a/include/asm-generic/futex.h
+++ b/include/asm-generic/futex.h
@@ -118,9 +118,7 @@ futex_atomic_cmpxchg_inatomic(u32 *uval, u32 __user *uaddr,
static inline int
arch_futex_atomic_op_inuser(int op, u32 oparg, int *oval, u32 __user *uaddr)
{
- int oldval = 0, ret;
-
- pagefault_disable();
+ int ret = 0;
switch (op) {
case FUTEX_OP_SET:
@@ -132,10 +130,8 @@ arch_futex_atomic_op_inuser(int op, u32 oparg, int *oval, u32 __user *uaddr)
ret = -ENOSYS;
}
- pagefault_enable();
-
if (!ret)
- *oval = oldval;
+ *oval = 0;
return ret;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH] generic arch_futex_atomic_op_inuser() cleanup
2019-07-15 10:27 [PATCH] generic arch_futex_atomic_op_inuser() cleanup Vasily Averin
2019-07-15 10:27 ` Vasily Averin
@ 2019-07-15 10:29 ` Vasily Averin
2019-07-15 10:29 ` Vasily Averin
2019-07-15 12:06 ` Arnd Bergmann
2019-07-15 13:13 ` [PATCH] " Thomas Gleixner
2 siblings, 2 replies; 14+ messages in thread
From: Vasily Averin @ 2019-07-15 10:29 UTC (permalink / raw)
To: linux-kernel, linux-arch, Thomas Gleixner, Ingo Molnar,
Arnd Bergmann
Looks like this code is dead and therefore looks strange.
I've found it during manual code review and decided to send patch
to pay your attention to this problem.
Probably it's better to remove this code at all?
On 7/15/19 1:27 PM, Vasily Averin wrote:
> Access to 'op' variable does not require pagefault_disable(),
> 'ret' variable should be initialized before using,
> 'oldval' variable can be replaced by constant.
>
> Signed-off-by: Vasily Averin <vvs@virtuozzo.com>
> ---
> include/asm-generic/futex.h | 8 ++------
> 1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/include/asm-generic/futex.h b/include/asm-generic/futex.h
> index 8666fe7f35d7..e9a9655d786d 100644
> --- a/include/asm-generic/futex.h
> +++ b/include/asm-generic/futex.h
> @@ -118,9 +118,7 @@ futex_atomic_cmpxchg_inatomic(u32 *uval, u32 __user *uaddr,
> static inline int
> arch_futex_atomic_op_inuser(int op, u32 oparg, int *oval, u32 __user *uaddr)
> {
> - int oldval = 0, ret;
> -
> - pagefault_disable();
> + int ret = 0;
>
> switch (op) {
> case FUTEX_OP_SET:
> @@ -132,10 +130,8 @@ arch_futex_atomic_op_inuser(int op, u32 oparg, int *oval, u32 __user *uaddr)
> ret = -ENOSYS;
> }
>
> - pagefault_enable();
> -
> if (!ret)
> - *oval = oldval;
> + *oval = 0;
>
> return ret;
> }
>
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] generic arch_futex_atomic_op_inuser() cleanup
2019-07-15 10:29 ` Vasily Averin
@ 2019-07-15 10:29 ` Vasily Averin
2019-07-15 12:06 ` Arnd Bergmann
1 sibling, 0 replies; 14+ messages in thread
From: Vasily Averin @ 2019-07-15 10:29 UTC (permalink / raw)
To: linux-kernel, linux-arch, Thomas Gleixner, Ingo Molnar,
Arnd Bergmann
Looks like this code is dead and therefore looks strange.
I've found it during manual code review and decided to send patch
to pay your attention to this problem.
Probably it's better to remove this code at all?
On 7/15/19 1:27 PM, Vasily Averin wrote:
> Access to 'op' variable does not require pagefault_disable(),
> 'ret' variable should be initialized before using,
> 'oldval' variable can be replaced by constant.
>
> Signed-off-by: Vasily Averin <vvs@virtuozzo.com>
> ---
> include/asm-generic/futex.h | 8 ++------
> 1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/include/asm-generic/futex.h b/include/asm-generic/futex.h
> index 8666fe7f35d7..e9a9655d786d 100644
> --- a/include/asm-generic/futex.h
> +++ b/include/asm-generic/futex.h
> @@ -118,9 +118,7 @@ futex_atomic_cmpxchg_inatomic(u32 *uval, u32 __user *uaddr,
> static inline int
> arch_futex_atomic_op_inuser(int op, u32 oparg, int *oval, u32 __user *uaddr)
> {
> - int oldval = 0, ret;
> -
> - pagefault_disable();
> + int ret = 0;
>
> switch (op) {
> case FUTEX_OP_SET:
> @@ -132,10 +130,8 @@ arch_futex_atomic_op_inuser(int op, u32 oparg, int *oval, u32 __user *uaddr)
> ret = -ENOSYS;
> }
>
> - pagefault_enable();
> -
> if (!ret)
> - *oval = oldval;
> + *oval = 0;
>
> return ret;
> }
>
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] generic arch_futex_atomic_op_inuser() cleanup
2019-07-15 10:29 ` Vasily Averin
2019-07-15 10:29 ` Vasily Averin
@ 2019-07-15 12:06 ` Arnd Bergmann
2019-07-15 12:06 ` Arnd Bergmann
2019-07-15 13:15 ` Thomas Gleixner
1 sibling, 2 replies; 14+ messages in thread
From: Arnd Bergmann @ 2019-07-15 12:06 UTC (permalink / raw)
To: Vasily Averin
Cc: Linux Kernel Mailing List, linux-arch, Thomas Gleixner,
Ingo Molnar
On Mon, Jul 15, 2019 at 12:29 PM Vasily Averin <vvs@virtuozzo.com> wrote:
>
> Looks like this code is dead and therefore looks strange.
> I've found it during manual code review and decided to send patch
> to pay your attention to this problem.
> Probably it's better to remove this code at all?
>
> On 7/15/19 1:27 PM, Vasily Averin wrote:
> > Access to 'op' variable does not require pagefault_disable(),
> > 'ret' variable should be initialized before using,
> > 'oldval' variable can be replaced by constant.
> >
> > Signed-off-by: Vasily Averin <vvs@virtuozzo.com>
I'm not following the reasoning for any of the changes here. Why do you
think we don't need the pagefault_disable() around get_user()/put_user(),
and which part of the funtion is dead code?
Arnd
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] generic arch_futex_atomic_op_inuser() cleanup
2019-07-15 12:06 ` Arnd Bergmann
@ 2019-07-15 12:06 ` Arnd Bergmann
2019-07-15 13:15 ` Thomas Gleixner
1 sibling, 0 replies; 14+ messages in thread
From: Arnd Bergmann @ 2019-07-15 12:06 UTC (permalink / raw)
To: Vasily Averin
Cc: Linux Kernel Mailing List, linux-arch, Thomas Gleixner,
Ingo Molnar
On Mon, Jul 15, 2019 at 12:29 PM Vasily Averin <vvs@virtuozzo.com> wrote:
>
> Looks like this code is dead and therefore looks strange.
> I've found it during manual code review and decided to send patch
> to pay your attention to this problem.
> Probably it's better to remove this code at all?
>
> On 7/15/19 1:27 PM, Vasily Averin wrote:
> > Access to 'op' variable does not require pagefault_disable(),
> > 'ret' variable should be initialized before using,
> > 'oldval' variable can be replaced by constant.
> >
> > Signed-off-by: Vasily Averin <vvs@virtuozzo.com>
I'm not following the reasoning for any of the changes here. Why do you
think we don't need the pagefault_disable() around get_user()/put_user(),
and which part of the funtion is dead code?
Arnd
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] generic arch_futex_atomic_op_inuser() cleanup
2019-07-15 12:06 ` Arnd Bergmann
2019-07-15 12:06 ` Arnd Bergmann
@ 2019-07-15 13:15 ` Thomas Gleixner
2019-07-15 13:15 ` Thomas Gleixner
2019-07-16 6:22 ` [PATCH v2] futex: " Vasily Averin
1 sibling, 2 replies; 14+ messages in thread
From: Thomas Gleixner @ 2019-07-15 13:15 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Vasily Averin, Linux Kernel Mailing List, linux-arch, Ingo Molnar
On Mon, 15 Jul 2019, Arnd Bergmann wrote:
> On Mon, Jul 15, 2019 at 12:29 PM Vasily Averin <vvs@virtuozzo.com> wrote:
> >
> > Looks like this code is dead and therefore looks strange.
> > I've found it during manual code review and decided to send patch
> > to pay your attention to this problem.
> > Probably it's better to remove this code at all?
> >
> > On 7/15/19 1:27 PM, Vasily Averin wrote:
> > > Access to 'op' variable does not require pagefault_disable(),
> > > 'ret' variable should be initialized before using,
> > > 'oldval' variable can be replaced by constant.
> > >
> > > Signed-off-by: Vasily Averin <vvs@virtuozzo.com>
>
> I'm not following the reasoning for any of the changes here. Why do you
> think we don't need the pagefault_disable() around get_user()/put_user(),
> and which part of the funtion is dead code?
All of it. If you change the function to
{
return -ENOSYS;
}
then it is equivalent (except for the pointless pagefault_disable/enable()
pair which protects absolutely nothing).
Thanks,
tglx
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] generic arch_futex_atomic_op_inuser() cleanup
2019-07-15 13:15 ` Thomas Gleixner
@ 2019-07-15 13:15 ` Thomas Gleixner
2019-07-16 6:22 ` [PATCH v2] futex: " Vasily Averin
1 sibling, 0 replies; 14+ messages in thread
From: Thomas Gleixner @ 2019-07-15 13:15 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Vasily Averin, Linux Kernel Mailing List, linux-arch, Ingo Molnar
On Mon, 15 Jul 2019, Arnd Bergmann wrote:
> On Mon, Jul 15, 2019 at 12:29 PM Vasily Averin <vvs@virtuozzo.com> wrote:
> >
> > Looks like this code is dead and therefore looks strange.
> > I've found it during manual code review and decided to send patch
> > to pay your attention to this problem.
> > Probably it's better to remove this code at all?
> >
> > On 7/15/19 1:27 PM, Vasily Averin wrote:
> > > Access to 'op' variable does not require pagefault_disable(),
> > > 'ret' variable should be initialized before using,
> > > 'oldval' variable can be replaced by constant.
> > >
> > > Signed-off-by: Vasily Averin <vvs@virtuozzo.com>
>
> I'm not following the reasoning for any of the changes here. Why do you
> think we don't need the pagefault_disable() around get_user()/put_user(),
> and which part of the funtion is dead code?
All of it. If you change the function to
{
return -ENOSYS;
}
then it is equivalent (except for the pointless pagefault_disable/enable()
pair which protects absolutely nothing).
Thanks,
tglx
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH v2] futex: generic arch_futex_atomic_op_inuser() cleanup
2019-07-15 13:15 ` Thomas Gleixner
2019-07-15 13:15 ` Thomas Gleixner
@ 2019-07-16 6:22 ` Vasily Averin
2019-07-16 6:22 ` Vasily Averin
2019-07-16 12:46 ` Arnd Bergmann
1 sibling, 2 replies; 14+ messages in thread
From: Vasily Averin @ 2019-07-16 6:22 UTC (permalink / raw)
To: Thomas Gleixner, linux-arch, Arnd Bergmann, linux-kernel
generic smp version of arch_futex_atomic_op_inuser() always returns -ENOSYS
Signed-off-by: Vasily Averin <vvs@virtuozzo.com>
---
include/asm-generic/futex.h | 21 +--------------------
1 file changed, 1 insertion(+), 20 deletions(-)
diff --git a/include/asm-generic/futex.h b/include/asm-generic/futex.h
index 8666fe7f35d7..02970b11f71f 100644
--- a/include/asm-generic/futex.h
+++ b/include/asm-generic/futex.h
@@ -118,26 +118,7 @@ futex_atomic_cmpxchg_inatomic(u32 *uval, u32 __user *uaddr,
static inline int
arch_futex_atomic_op_inuser(int op, u32 oparg, int *oval, u32 __user *uaddr)
{
- int oldval = 0, ret;
-
- pagefault_disable();
-
- switch (op) {
- case FUTEX_OP_SET:
- case FUTEX_OP_ADD:
- case FUTEX_OP_OR:
- case FUTEX_OP_ANDN:
- case FUTEX_OP_XOR:
- default:
- ret = -ENOSYS;
- }
-
- pagefault_enable();
-
- if (!ret)
- *oval = oldval;
-
- return ret;
+ return -ENOSYS;
}
static inline int
--
2.17.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v2] futex: generic arch_futex_atomic_op_inuser() cleanup
2019-07-16 6:22 ` [PATCH v2] futex: " Vasily Averin
@ 2019-07-16 6:22 ` Vasily Averin
2019-07-16 12:46 ` Arnd Bergmann
1 sibling, 0 replies; 14+ messages in thread
From: Vasily Averin @ 2019-07-16 6:22 UTC (permalink / raw)
To: Thomas Gleixner, linux-arch, Arnd Bergmann, linux-kernel
generic smp version of arch_futex_atomic_op_inuser() always returns -ENOSYS
Signed-off-by: Vasily Averin <vvs@virtuozzo.com>
---
include/asm-generic/futex.h | 21 +--------------------
1 file changed, 1 insertion(+), 20 deletions(-)
diff --git a/include/asm-generic/futex.h b/include/asm-generic/futex.h
index 8666fe7f35d7..02970b11f71f 100644
--- a/include/asm-generic/futex.h
+++ b/include/asm-generic/futex.h
@@ -118,26 +118,7 @@ futex_atomic_cmpxchg_inatomic(u32 *uval, u32 __user *uaddr,
static inline int
arch_futex_atomic_op_inuser(int op, u32 oparg, int *oval, u32 __user *uaddr)
{
- int oldval = 0, ret;
-
- pagefault_disable();
-
- switch (op) {
- case FUTEX_OP_SET:
- case FUTEX_OP_ADD:
- case FUTEX_OP_OR:
- case FUTEX_OP_ANDN:
- case FUTEX_OP_XOR:
- default:
- ret = -ENOSYS;
- }
-
- pagefault_enable();
-
- if (!ret)
- *oval = oldval;
-
- return ret;
+ return -ENOSYS;
}
static inline int
--
2.17.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v2] futex: generic arch_futex_atomic_op_inuser() cleanup
2019-07-16 6:22 ` [PATCH v2] futex: " Vasily Averin
2019-07-16 6:22 ` Vasily Averin
@ 2019-07-16 12:46 ` Arnd Bergmann
2019-07-16 12:46 ` Arnd Bergmann
1 sibling, 1 reply; 14+ messages in thread
From: Arnd Bergmann @ 2019-07-16 12:46 UTC (permalink / raw)
To: Vasily Averin; +Cc: Thomas Gleixner, linux-arch, Linux Kernel Mailing List
On Tue, Jul 16, 2019 at 8:22 AM Vasily Averin <vvs@virtuozzo.com> wrote:
>
> generic smp version of arch_futex_atomic_op_inuser() always returns -ENOSYS
>
> Signed-off-by: Vasily Averin <vvs@virtuozzo.com>
Acked-by: Arnd Bergmann <arnd@arndb.de>
I see what you mean now, I got confused earlier when I looked at the
non-SMP version of the same function.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] futex: generic arch_futex_atomic_op_inuser() cleanup
2019-07-16 12:46 ` Arnd Bergmann
@ 2019-07-16 12:46 ` Arnd Bergmann
0 siblings, 0 replies; 14+ messages in thread
From: Arnd Bergmann @ 2019-07-16 12:46 UTC (permalink / raw)
To: Vasily Averin; +Cc: Thomas Gleixner, linux-arch, Linux Kernel Mailing List
On Tue, Jul 16, 2019 at 8:22 AM Vasily Averin <vvs@virtuozzo.com> wrote:
>
> generic smp version of arch_futex_atomic_op_inuser() always returns -ENOSYS
>
> Signed-off-by: Vasily Averin <vvs@virtuozzo.com>
Acked-by: Arnd Bergmann <arnd@arndb.de>
I see what you mean now, I got confused earlier when I looked at the
non-SMP version of the same function.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] generic arch_futex_atomic_op_inuser() cleanup
2019-07-15 10:27 [PATCH] generic arch_futex_atomic_op_inuser() cleanup Vasily Averin
2019-07-15 10:27 ` Vasily Averin
2019-07-15 10:29 ` Vasily Averin
@ 2019-07-15 13:13 ` Thomas Gleixner
2019-07-15 13:13 ` Thomas Gleixner
2 siblings, 1 reply; 14+ messages in thread
From: Thomas Gleixner @ 2019-07-15 13:13 UTC (permalink / raw)
To: Vasily Averin; +Cc: linux-kernel, linux-arch, Ingo Molnar, Arnd Bergmann
On Mon, 15 Jul 2019, Vasily Averin wrote:
> Access to 'op' variable does not require pagefault_disable(),
> 'ret' variable should be initialized before using,
> 'oldval' variable can be replaced by constant.
>
> Signed-off-by: Vasily Averin <vvs@virtuozzo.com>
> ---
> include/asm-generic/futex.h | 8 ++------
> 1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/include/asm-generic/futex.h b/include/asm-generic/futex.h
> index 8666fe7f35d7..e9a9655d786d 100644
> --- a/include/asm-generic/futex.h
> +++ b/include/asm-generic/futex.h
> @@ -118,9 +118,7 @@ futex_atomic_cmpxchg_inatomic(u32 *uval, u32 __user *uaddr,
> static inline int
> arch_futex_atomic_op_inuser(int op, u32 oparg, int *oval, u32 __user *uaddr)
> {
> - int oldval = 0, ret;
> -
> - pagefault_disable();
> + int ret = 0;
The variable is clearly initialized before using in 'default:', but the
whole function is pretty useless. It's guaranteed to return -ENOSYS and
does nothing else than disabling/enabling pagefaults for no reason.
Thanks,
tglx
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] generic arch_futex_atomic_op_inuser() cleanup
2019-07-15 13:13 ` [PATCH] " Thomas Gleixner
@ 2019-07-15 13:13 ` Thomas Gleixner
0 siblings, 0 replies; 14+ messages in thread
From: Thomas Gleixner @ 2019-07-15 13:13 UTC (permalink / raw)
To: Vasily Averin; +Cc: linux-kernel, linux-arch, Ingo Molnar, Arnd Bergmann
On Mon, 15 Jul 2019, Vasily Averin wrote:
> Access to 'op' variable does not require pagefault_disable(),
> 'ret' variable should be initialized before using,
> 'oldval' variable can be replaced by constant.
>
> Signed-off-by: Vasily Averin <vvs@virtuozzo.com>
> ---
> include/asm-generic/futex.h | 8 ++------
> 1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/include/asm-generic/futex.h b/include/asm-generic/futex.h
> index 8666fe7f35d7..e9a9655d786d 100644
> --- a/include/asm-generic/futex.h
> +++ b/include/asm-generic/futex.h
> @@ -118,9 +118,7 @@ futex_atomic_cmpxchg_inatomic(u32 *uval, u32 __user *uaddr,
> static inline int
> arch_futex_atomic_op_inuser(int op, u32 oparg, int *oval, u32 __user *uaddr)
> {
> - int oldval = 0, ret;
> -
> - pagefault_disable();
> + int ret = 0;
The variable is clearly initialized before using in 'default:', but the
whole function is pretty useless. It's guaranteed to return -ENOSYS and
does nothing else than disabling/enabling pagefaults for no reason.
Thanks,
tglx
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2019-07-16 12:46 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-07-15 10:27 [PATCH] generic arch_futex_atomic_op_inuser() cleanup Vasily Averin
2019-07-15 10:27 ` Vasily Averin
2019-07-15 10:29 ` Vasily Averin
2019-07-15 10:29 ` Vasily Averin
2019-07-15 12:06 ` Arnd Bergmann
2019-07-15 12:06 ` Arnd Bergmann
2019-07-15 13:15 ` Thomas Gleixner
2019-07-15 13:15 ` Thomas Gleixner
2019-07-16 6:22 ` [PATCH v2] futex: " Vasily Averin
2019-07-16 6:22 ` Vasily Averin
2019-07-16 12:46 ` Arnd Bergmann
2019-07-16 12:46 ` Arnd Bergmann
2019-07-15 13:13 ` [PATCH] " Thomas Gleixner
2019-07-15 13:13 ` Thomas Gleixner
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox