* [PATCH ipsec net] xfrm: put policies when reusing pcpu xdst entry
From: Florian Westphal @ 2017-12-11 17:23 UTC (permalink / raw)
To: netdev; +Cc: Florian Westphal
We need to put the policies when re-using the pcpu xdst entry, else
this leaks the reference.
Fixes: ec30d78c14a813db39a647b6a348b428 ("xfrm: add xdst pcpu cache")
Signed-off-by: Florian Westphal <fw@strlen.de>
---
net/xfrm/xfrm_policy.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 9542975eb2f9..88143a480ec9 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -1833,6 +1833,7 @@ xfrm_resolve_and_create_bundle(struct xfrm_policy **pols, int num_pols,
sizeof(struct xfrm_policy *) * num_pols) == 0 &&
xfrm_xdst_can_reuse(xdst, xfrm, err)) {
dst_hold(&xdst->u.dst);
+ xfrm_pols_put(pols, num_pols);
while (err > 0)
xfrm_state_put(xfrm[--err]);
return xdst;
--
2.13.6
^ permalink raw reply related
* Re: Linux 4.15-rc3 (uml + bpf_perf_event.h)
From: Randy Dunlap @ 2017-12-11 17:27 UTC (permalink / raw)
To: Daniel Borkmann, Richard Weinberger, netdev@vger.kernel.org
Cc: Linus Torvalds, Linux Kernel Mailing List, alexei.starovoitov,
brueckner
In-Reply-To: <ea01fc20-a122-e6d3-e147-2de7f9fa7e47@iogearbox.net>
On 12/11/2017 02:19 AM, Daniel Borkmann wrote:
> Hi Randy, hi Richard, [ +Hendrik for c895f6f703ad7dd2f ]
>
> On 12/11/2017 09:32 AM, Richard Weinberger wrote:
>> Randy,
>>
>> Am Montag, 11. Dezember 2017, 03:42:12 CET schrieb Randy Dunlap:
>>> On 12/10/2017 06:08 PM, Linus Torvalds wrote:
>>>> Another week, another rc.
>>>
>>> um (uml) won't build on i386 or x86_64:
>>>
>>> CC init/main.o
>>> In file included from ../include/linux/perf_event.h:18:0,
>>> from ../include/linux/trace_events.h:10,
>>> from ../include/trace/syscall.h:7,
>>> from ../include/linux/syscalls.h:82,
>>> from ../init/main.c:20:
>>> ../include/uapi/linux/bpf_perf_event.h:11:32: fatal error:
>>> asm/bpf_perf_event.h: No such file or directory #include
>>> <asm/bpf_perf_event.h>
>>> ^
>>> compilation terminated.
>>> ../scripts/Makefile.build:310: recipe for target 'init/main.o' failed
>>
>> How do you trigger that build failure?
>> Can you share your .config?
Richard, it's just defconfig on both i386 and x86_64.
> Hmm, too bad kbuild bot doesn't catch issues on uml. I'm not too familiar
> with uml, but looks like it's the only special case where there's no
> arch/um/include/uapi/asm/. What is the usual convention to pull in such
> headers in this case? Something like the below, would that fix it for you?
>
> Thanks for your help,
> Daniel
Yes, that patch works. Thanks.
> arch/um/include/asm/bpf_perf_event.h | 1 +
> include/asm-generic/bpf_perf_event.h | 1 +
> 2 files changed, 2 insertions(+)
> create mode 100644 arch/um/include/asm/bpf_perf_event.h
> create mode 100644 include/asm-generic/bpf_perf_event.h
>
> diff --git a/arch/um/include/asm/bpf_perf_event.h b/arch/um/include/asm/bpf_perf_event.h
> new file mode 100644
> index 0000000..3097758
> --- /dev/null
> +++ b/arch/um/include/asm/bpf_perf_event.h
> @@ -0,0 +1 @@
> +#include <asm-generic/bpf_perf_event.h>
> diff --git a/include/asm-generic/bpf_perf_event.h b/include/asm-generic/bpf_perf_event.h
> new file mode 100644
> index 0000000..67112e5
> --- /dev/null
> +++ b/include/asm-generic/bpf_perf_event.h
> @@ -0,0 +1 @@
> +#include <uapi/asm-generic/bpf_perf_event.h>
>
--
~Randy
^ permalink raw reply
* Re: Linux 4.15-rc3 (uml + bpf_perf_event.h)
From: Daniel Borkmann @ 2017-12-11 17:32 UTC (permalink / raw)
To: Randy Dunlap, Richard Weinberger, netdev@vger.kernel.org
Cc: Linus Torvalds, Linux Kernel Mailing List, alexei.starovoitov,
brueckner
In-Reply-To: <96fb23e5-881d-46d5-b3ed-0a9269d4b831@infradead.org>
On 12/11/2017 06:27 PM, Randy Dunlap wrote:
> On 12/11/2017 02:19 AM, Daniel Borkmann wrote:
>> Hi Randy, hi Richard, [ +Hendrik for c895f6f703ad7dd2f ]
>>
>> On 12/11/2017 09:32 AM, Richard Weinberger wrote:
>>> Randy,
>>>
>>> Am Montag, 11. Dezember 2017, 03:42:12 CET schrieb Randy Dunlap:
>>>> On 12/10/2017 06:08 PM, Linus Torvalds wrote:
>>>>> Another week, another rc.
>>>>
>>>> um (uml) won't build on i386 or x86_64:
>>>>
>>>> CC init/main.o
>>>> In file included from ../include/linux/perf_event.h:18:0,
>>>> from ../include/linux/trace_events.h:10,
>>>> from ../include/trace/syscall.h:7,
>>>> from ../include/linux/syscalls.h:82,
>>>> from ../init/main.c:20:
>>>> ../include/uapi/linux/bpf_perf_event.h:11:32: fatal error:
>>>> asm/bpf_perf_event.h: No such file or directory #include
>>>> <asm/bpf_perf_event.h>
>>>> ^
>>>> compilation terminated.
>>>> ../scripts/Makefile.build:310: recipe for target 'init/main.o' failed
>>>
>>> How do you trigger that build failure?
>>> Can you share your .config?
>
> Richard, it's just defconfig on both i386 and x86_64.
>
>> Hmm, too bad kbuild bot doesn't catch issues on uml. I'm not too familiar
>> with uml, but looks like it's the only special case where there's no
>> arch/um/include/uapi/asm/. What is the usual convention to pull in such
>> headers in this case? Something like the below, would that fix it for you?
>>
>> Thanks for your help,
>> Daniel
>
> Yes, that patch works. Thanks.
Thanks, I'd get it out later today, and route it via bpf tree.
^ permalink raw reply
* Re: [PATCH 1/1] rtlwifi: always initialize variables given to RT_TRACE()
From: Larry Finger @ 2017-12-11 17:34 UTC (permalink / raw)
To: Nicolas Iooss, Chaoming Li, Kalle Valo,
linux-wireless-u79uwXL29TY76Z2rM5mHXA
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20171210195159.7454-1-nicolas.iooss_linux-oWGTIYur0i8@public.gmane.org>
On 12/10/2017 01:51 PM, Nicolas Iooss wrote:
> In rtl_rx_ampdu_apply(), when rtlpriv->cfg->ops->get_btc_status()
> returns false, RT_TRACE() is called with the values of variables
> reject_agg and agg_size, which have not been initialized.
>
> Always initialize these variables in order to prevent using
> uninitialized values.
>
> This issue has been found with clang. The compiler reported:
>
> drivers/net/wireless/realtek/rtlwifi/base.c:1665:6: error: variable
> 'agg_size' is used uninitialized whenever 'if' condition is false
> [-Werror,-Wsometimes-uninitialized]
> if (rtlpriv->cfg->ops->get_btc_status())
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/net/wireless/realtek/rtlwifi/base.c:1671:31: note:
> uninitialized use occurs here
> reject_agg, ctrl_agg_size, agg_size);
> ^~~~~~~~
>
> drivers/net/wireless/realtek/rtlwifi/base.c:1665:6: error: variable
> 'reject_agg' is used uninitialized whenever 'if' condition
> is false [-Werror,-Wsometimes-uninitialized]
> if (rtlpriv->cfg->ops->get_btc_status())
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/net/wireless/realtek/rtlwifi/base.c:1671:4: note:
> uninitialized use occurs here
> reject_agg, ctrl_agg_size, agg_size);
> ^~~~~~~~~~
>
> Fixes: 2635664e6e4a ("rtlwifi: Add rx ampdu cfg for btcoexist.")
> Signed-off-by: Nicolas Iooss <nicolas.iooss_linux-oWGTIYur0i8@public.gmane.org>
Looks good. Acked-by: Larry Finger <Larry.Finger-tQ5ms3gMjBLk1uMJSBkQmQ@public.gmane.org>
Thanks,
Larry
> ---
> drivers/net/wireless/realtek/rtlwifi/base.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/realtek/rtlwifi/base.c b/drivers/net/wireless/realtek/rtlwifi/base.c
> index cad2272ae21b..704741d6f495 100644
> --- a/drivers/net/wireless/realtek/rtlwifi/base.c
> +++ b/drivers/net/wireless/realtek/rtlwifi/base.c
> @@ -1726,7 +1726,7 @@ int rtl_tx_agg_oper(struct ieee80211_hw *hw,
> void rtl_rx_ampdu_apply(struct rtl_priv *rtlpriv)
> {
> struct rtl_btc_ops *btc_ops = rtlpriv->btcoexist.btc_ops;
> - u8 reject_agg, ctrl_agg_size = 0, agg_size;
> + u8 reject_agg = 0, ctrl_agg_size = 0, agg_size = 0;
>
> if (rtlpriv->cfg->ops->get_btc_status())
> btc_ops->btc_get_ampdu_cfg(rtlpriv, &reject_agg,
>
^ permalink raw reply
* Re: [PATCH net-next 1/3] tcp: do not overshoot window_clamp in tcp_rcv_space_adjust()
From: Neal Cardwell @ 2017-12-11 17:47 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S . Miller, Yuchung Cheng, Soheil Hassas Yeganeh, Wei Wang,
Priyaranjan Jha, netdev, Eric Dumazet
In-Reply-To: <20171211015504.26551-2-edumazet@google.com>
On Sun, Dec 10, 2017 at 8:55 PM, Eric Dumazet <edumazet@google.com> wrote:
>
> While rcvbuf is properly clamped by tcp_rmem[2], rcvwin
> is left to a potentially too big value.
>
> It has no serious effect, since :
> 1) tcp_grow_window() has very strict checks.
> 2) window_clamp can be mangled by user space to any value anyway.
>
> tcp_init_buffer_space() and companions use tcp_full_space(),
> we use tcp_win_from_space() to avoid reloading sk->sk_rcvbuf
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Acked-by: Soheil Hassas Yeganeh <soheil@google.com>
> Acked-by: Wei Wang <weiwan@google.com>
> ---
Acked-by: Neal Cardwell <ncardwell@google.com>
Thanks, Eric!
neal
^ permalink raw reply
* Re: [PATCH net-next 2/3] tcp: avoid integer overflows in tcp_rcv_space_adjust()
From: Neal Cardwell @ 2017-12-11 17:47 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S . Miller, Yuchung Cheng, Soheil Hassas Yeganeh, Wei Wang,
Priyaranjan Jha, netdev, Eric Dumazet
In-Reply-To: <20171211015504.26551-3-edumazet@google.com>
On Sun, Dec 10, 2017 at 8:55 PM, Eric Dumazet <edumazet@google.com> wrote:
> When using large tcp_rmem[2] values (I did tests with 500 MB),
> I noticed overflows while computing rcvwin.
>
> Lets fix this before the following patch.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Acked-by: Soheil Hassas Yeganeh <soheil@google.com>
> Acked-by: Wei Wang <weiwan@google.com>
> ---
Acked-by: Neal Cardwell <ncardwell@google.com>
Thanks, Eric!
neal
^ permalink raw reply
* Re: [RFC PATCH 0/9] ethtool netlink interface (WiP)
From: Jiri Pirko @ 2017-12-11 17:59 UTC (permalink / raw)
To: David Miller; +Cc: mkubecek, netdev, linux-kernel
In-Reply-To: <20171211.120144.1060832843526341781.davem@davemloft.net>
Mon, Dec 11, 2017 at 06:01:44PM CET, davem@davemloft.net wrote:
>From: Jiri Pirko <jiri@resnulli.us>
>Date: Mon, 11 Dec 2017 17:32:46 +0100
>
>> I think that it does not make sense to convert ethtool->netlink_ethtool
>> 1:1 feature wise. Now we have devlink, ritch switch representation
>> model, tc offload and many others. Lot of things that are in
>> ethtool, should be done in devlink. Also, there are couple of things
>> that should just die - nice example is ethtool --config-ntuple - we
>> should use tc for that.
>
>Whilst I do agree that devlink is probably a good place for this stuff
>(we want to be able to do ethetool things on objects that lack a netdev)
>I do not agree with the tc angle.
>
>It is entirely appropriate to set the ntuple settings of a driver
>without being required to use TC or similar.
>
>All you are going to do with your suggestion is make people keep using
>the existing ethtool ioctl, because they'll say "screw this, I'm not
>using TC I have something which works just fine already". And that's
>not the goal of putting this stuff into netlink, we want people to
>use the new facilities and move off of the ioctl.
Sure, but this is a great opportunity to avoid copying old mistakes.
That is why I suggested to do it not 1:1 but rather introduce brand new
netlink-based interface that would not carry old baggage.
^ permalink raw reply
* Re: [PATCH net-next 3/3] tcp: smoother receiver autotuning
From: Neal Cardwell @ 2017-12-11 17:59 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S . Miller, Yuchung Cheng, Soheil Hassas Yeganeh, Wei Wang,
Priyaranjan Jha, netdev, Eric Dumazet
In-Reply-To: <20171211015504.26551-4-edumazet@google.com>
On Sun, Dec 10, 2017 at 8:55 PM, Eric Dumazet <edumazet@google.com> wrote:
> Back in linux-3.13 (commit b0983d3c9b13 ("tcp: fix dynamic right sizing"))
> I addressed the pressing issues we had with receiver autotuning.
>
> But DRS suffers from extra latencies caused by rcv_rtt_est.rtt_us
> drifts. One common problem happens during slow start, since the
> apparent RTT measured by the receiver can be inflated by ~50%,
> at the end of one packet train.
>
> Also, a single drop can delay read() calls by one RTT, meaning
> tcp_rcv_space_adjust() can be called one RTT too late.
>
> By replacing the tri-modal heuristic with a continuous function,
> we can offset the effects of not growing 'at the optimal time'.
>
> The curve of the function matches prior behavior if the space
> increased by 25% and 50% exactly.
>
> Cost of added multiply/divide is small, considering a TCP flow
> typically would run this part of the code few times in its life.
>
> I tested this patch with 100 ms RTT / 1% loss link, 100 runs
> of (netperf -l 5), and got an average throughput of 4600 Mbit
> instead of 1700 Mbit.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Acked-by: Soheil Hassas Yeganeh <soheil@google.com>
> Acked-by: Wei Wang <weiwan@google.com>
> ---
Acked-by: Neal Cardwell <ncardwell@google.com>
Thanks, Eric!
neal
^ permalink raw reply
* Re: [RFC PATCH 2/9] ethtool: introduce ethtool netlink interface
From: Jiri Pirko @ 2017-12-11 18:02 UTC (permalink / raw)
To: David Miller; +Cc: mkubecek, netdev, linux-kernel
In-Reply-To: <20171211.115651.1046181633998981619.davem@davemloft.net>
Mon, Dec 11, 2017 at 05:56:51PM CET, davem@davemloft.net wrote:
>From: Jiri Pirko <jiri@resnulli.us>
>Date: Mon, 11 Dec 2017 17:02:21 +0100
>
>> Mon, Dec 11, 2017 at 02:53:31PM CET, mkubecek@suse.cz wrote:
>>>No function implemented yet, only genetlink and module infrastructure.
>>>Register/unregister genetlink family "ethtool" and allow the module to be
>>>autoloaded by genetlink code (if built as a module, distributions would
>>>probably prefer "y").
>>>
>>>Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
>>
>> [...]
>>
>>
>>>+
>>>+/* identifies the device to query/set
>>>+ * - use either ifindex or ifname, not both
>>>+ * - for dumps and messages not related to a particular devices, fill neither
>>>+ * - info_mask is a bitfield, interpretation depends on the command
>>>+ */
>>>+struct ethnlmsghdr {
>>>+ __u32 ifindex; /* device ifindex */
>>>+ __u16 flags; /* request/response flags */
>>>+ __u16 info_mask; /* request/response info mask */
>>>+ char ifname[IFNAMSIZ]; /* device name */
>>
>> Why do you need this header? You can have 2 attrs:
>> ETHTOOL_ATTR_IFINDEX and
>> ETHTOOL_ATTR_IFNAME
>>
>> Why do you need per-command flags and info_mask? Could be bitfield
>> attr if needed by specific command.
>
>Jiri, we've had this discussion before :-)
>
>For elements which are common to most, if not all, requests it makes
>sense to define a base netlink message.
>
>My opinion on this matter has not changed at all since the last time
>we discussed this.
The discussion we had before was about flag bitfield that was there
*always*. In this case, that is not true. It is either ifindex or
ifname. Even rtnetlink has ifname as attribute.
The flags and info_mask is just big mystery. If it is per-command,
seems natural to have it as attributes.
>
>So unless you have new information to provide to me on this issue
>which might change my mind, please accept the result of the previous
>discussion which is that a base netlink message is not only
>appropriate but desirable.
>
>Thank you.
^ permalink raw reply
* [net 1/1] tipc: eliminate potential memory leak
From: Jon Maloy @ 2017-12-11 18:11 UTC (permalink / raw)
To: davem, netdev
Cc: tipc-discussion, hoang.h.le, mohan.krishna.ghanta.krishnamurthy
In the function tipc_sk_mcast_rcv() we call refcount_dec(&skb->users)
on received sk_buffers. Since the reference counter might hit zero at
this point, we have a potential memory leak.
We fix this by replacing refcount_dec() with kfree_skb().
Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
---
net/tipc/socket.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index 5d18c0c..41127d0 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -1140,7 +1140,7 @@ void tipc_sk_mcast_rcv(struct net *net, struct sk_buff_head *arrvq,
__skb_dequeue(arrvq);
__skb_queue_tail(inputq, skb);
}
- refcount_dec(&skb->users);
+ kfree_skb(skb);
spin_unlock_bh(&inputq->lock);
continue;
}
--
2.1.4
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
^ permalink raw reply related
* Re: Linux 4.15-rc3 (uml + bpf_perf_event.h)
From: Richard Weinberger @ 2017-12-11 18:19 UTC (permalink / raw)
To: Randy Dunlap
Cc: Daniel Borkmann, netdev@vger.kernel.org, Linus Torvalds,
Linux Kernel Mailing List, alexei.starovoitov, brueckner
In-Reply-To: <96fb23e5-881d-46d5-b3ed-0a9269d4b831@infradead.org>
Am Montag, 11. Dezember 2017, 18:27:40 CET schrieb Randy Dunlap:
> On 12/11/2017 02:19 AM, Daniel Borkmann wrote:
> > Hi Randy, hi Richard, [ +Hendrik for c895f6f703ad7dd2f ]
> >
> > On 12/11/2017 09:32 AM, Richard Weinberger wrote:
> >> Randy,
> >>
> >> Am Montag, 11. Dezember 2017, 03:42:12 CET schrieb Randy Dunlap:
> >>> On 12/10/2017 06:08 PM, Linus Torvalds wrote:
> >>>> Another week, another rc.
> >>>
> >>> um (uml) won't build on i386 or x86_64:
> >>> CC init/main.o
> >>>
> >>> In file included from ../include/linux/perf_event.h:18:0,
> >>>
> >>> from ../include/linux/trace_events.h:10,
> >>> from ../include/trace/syscall.h:7,
> >>> from ../include/linux/syscalls.h:82,
> >>>
> >>> from ../init/main.c:20:
> >>> ../include/uapi/linux/bpf_perf_event.h:11:32: fatal error:
> >>> asm/bpf_perf_event.h: No such file or directory #include
> >>> <asm/bpf_perf_event.h>
> >>>
> >>> ^
> >>>
> >>> compilation terminated.
> >>> ../scripts/Makefile.build:310: recipe for target 'init/main.o' failed
> >>
> >> How do you trigger that build failure?
> >> Can you share your .config?
>
> Richard, it's just defconfig on both i386 and x86_64.
How odd, here it used to build.
...until I did a make mrproper, and tried again. ;-\
Thanks,
//richard
^ permalink raw reply
* Re: [PATCH net] sctp: make sure stream nums can match optlen in sctp_setsockopt_reset_streams
From: Neil Horman @ 2017-12-11 18:28 UTC (permalink / raw)
To: Marcelo Ricardo Leitner
Cc: Xin Long, network dev, linux-sctp, davem, syzkaller
In-Reply-To: <20171211153639.GA3532@localhost.localdomain>
On Mon, Dec 11, 2017 at 01:36:39PM -0200, Marcelo Ricardo Leitner wrote:
> Hi,
>
> On Mon, Dec 11, 2017 at 09:54:34AM -0500, Neil Horman wrote:
> > On Sun, Dec 10, 2017 at 03:40:51PM +0800, Xin Long wrote:
> > > Now in sctp_setsockopt_reset_streams, it only does the check
> > > optlen < sizeof(*params) for optlen. But it's not enough, as
> > > params->srs_number_streams should also match optlen.
> > >
> > > If the streams in params->srs_stream_list are less than stream
> > > nums in params->srs_number_streams, later when dereferencing
> > > the stream list, it could cause a slab-out-of-bounds crash, as
> > > reported by syzbot.
> > >
> > > This patch is to fix it by also checking the stream numbers in
> > > sctp_setsockopt_reset_streams to make sure at least it's not
> > > greater than the streams in the list.
> > >
> > > Fixes: 7f9d68ac944e ("sctp: implement sender-side procedures for SSN Reset Request Parameter")
> > > Reported-by: Dmitry Vyukov <dvyukov@google.com>
> > > Signed-off-by: Xin Long <lucien.xin@gmail.com>
> > > ---
> > > net/sctp/socket.c | 6 +++++-
> > > 1 file changed, 5 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> > > index 014847e..dbf140d 100644
> > > --- a/net/sctp/socket.c
> > > +++ b/net/sctp/socket.c
> > > @@ -3891,13 +3891,17 @@ static int sctp_setsockopt_reset_streams(struct sock *sk,
> > > struct sctp_association *asoc;
> > > int retval = -EINVAL;
> > >
> > > - if (optlen < sizeof(struct sctp_reset_streams))
> > > + if (optlen < sizeof(*params))
> > > return -EINVAL;
> > >
> > Is this going to work in all corner cases? IIRC struct
> > sctp_reset_stream has variable length array at the end of it, and so
> > sizeof(struct sctp_reset_streams) returns just the size of the
> > struct, while sizeof(*params) returns the size of the entire object
> > (including the array elements). If a user space task allocates a
>
> I don't think it can include the array elements as such information
> can't be passed from the application to the kernel other than via
> optlen parameter. There is no metadata around the struct that could
> allow that, and sizeof() is a constant, it can't be assuming different
> values in runtime.
>
> Cheers,
> Marcelo
>%
Yup, you're right, its fine.
Acked-by: Neil Horman <nhorman@tuxdriver.com>
> > static memory block to hold this struct and the array, and passes it
> > in with a shorter optlen (if for example, they do not intend to use
> > all the array elements), this will cause a failure, where no failure
> > is truly warranted. It seems the correct check to me should be the
> > origional sizeof(struct sctp_reset_streams) check, and the below
> > check to ensure that there are at least the same number of array
> > elements available as indicated in srs_nuber_streams.
> >
> > Regards
> > Neil
> >
> > > params = memdup_user(optval, optlen);
> > > if (IS_ERR(params))
> > > return PTR_ERR(params);
> > >
> > > + if (params->srs_number_streams * sizeof(__u16) >
> > > + optlen - sizeof(*params))
> > > + goto out;
> > > +
> > > asoc = sctp_id2assoc(sk, params->srs_assoc_id);
> > > if (!asoc)
> > > goto out;
> > > --
> > > 2.1.0
> > >
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> > > the body of a message to majordomo@vger.kernel.org
> > > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [PATCH net-next] netdevsim: correctly check return value of debugfs_create_dir
From: Jakub Kicinski @ 2017-12-11 18:40 UTC (permalink / raw)
To: Prashant Bhole; +Cc: 'David Miller', netdev
In-Reply-To: <002201d3723b$0e2b9310$2a82b930$@lab.ntt.co.jp>
On Mon, 11 Dec 2017 13:46:48 +0900, Prashant Bhole wrote:
> > From: David Miller [mailto:davem@davemloft.net]
> >
> > From: Prashant Bhole <bhole_prashant_q7@lab.ntt.co.jp>
> > Date: Fri, 8 Dec 2017 09:52:50 +0900
> >
> > > Return value is now checked with IS_ERROR_OR_NULL because
> > > debugfs_create_dir doesn't return error value. It either returns NULL
> > > or a valid pointer.
> > >
> > > Signed-off-by: Prashant Bhole <bhole_prashant_q7@lab.ntt.co.jp>
> > > ---
> > > drivers/net/netdevsim/netdev.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/net/netdevsim/netdev.c
> > > b/drivers/net/netdevsim/netdev.c index eb8c679fca9f..88d8ee2c89da
> > > 100644
> > > --- a/drivers/net/netdevsim/netdev.c
> > > +++ b/drivers/net/netdevsim/netdev.c
> > > @@ -469,7 +469,7 @@ static int __init nsim_module_init(void)
> > > int err;
> > >
> > > nsim_ddir = debugfs_create_dir(DRV_NAME, NULL);
> > > - if (IS_ERR(nsim_ddir))
> > > + if (IS_ERR_OR_NULL(nsim_ddir))
> > > return PTR_ERR(nsim_ddir);
> >
> > debugfs_create_dir() should really be fixed, either it uses error pointers
> > consistently and therefore always provides a suitable error code to return
> or it
> > always uses NULL.
> >
> > This in-between behavior makes using it as an interface painful because no
> clear
> > meaning is given to NULL.
> >
> > So please do the work necessary to make debugfs_create_dir()'s return
> > semantics clearer and more useful.
> >
> > Thank you.
>
> Dave,
> Thanks for comments. I will try to fix error handling in netdevsim first.
>
> Jakub,
> Let's decide with an example. The typical directory structure for netdevsim
> interface is as below:
> /sys/kernel/debug/netdevsim/sim0/bpf_bound_progs/
> Please let me know if you are ok with following:
>
> 1) If debugfs_create_dir() fails in module_init, let's keep it fatal error
> with corrected condition:
> + if (IS_ERR_OR_NULL(nsim_ddir))
> + return -ENOMEM;
>
> 2) In case sim0 or bpf_bound_progs are fail to create, we need to add
> checks before creating any file in them.
Fine with me, although if you fix DebugFS first you could use the real
error from the start here.
^ permalink raw reply
* Re: [RFC PATCH 2/9] ethtool: introduce ethtool netlink interface
From: David Miller @ 2017-12-11 18:45 UTC (permalink / raw)
To: jiri; +Cc: mkubecek, netdev, linux-kernel
In-Reply-To: <20171211180219.GB2047@nanopsycho>
From: Jiri Pirko <jiri@resnulli.us>
Date: Mon, 11 Dec 2017 19:02:19 +0100
> The discussion we had before was about flag bitfield that was there
> *always*. In this case, that is not true. It is either ifindex or
> ifname. Even rtnetlink has ifname as attribute.
>
> The flags and info_mask is just big mystery. If it is per-command,
> seems natural to have it as attributes.
I think flags and info_mask indeed can be moved out of this struct.
I guess, in this case, I can see your point of view especially if we
allow ethtool operations on non-netdev entities.
So, ok, let's move forward without a base command struct and just
use attributes.
Thanks :)
^ permalink raw reply
* Re: [PATCH 1/3] PCI: introduce a device-managed version of pci_set_mwi
From: David Miller @ 2017-12-11 18:49 UTC (permalink / raw)
To: hkallweit1; +Cc: nic_swsd, bhelgaas, netdev, linux-pci
In-Reply-To: <3e560e2b-7f11-74e2-6db5-60c8479dc59b@gmail.com>
From: Heiner Kallweit <hkallweit1@gmail.com>
Date: Sun, 10 Dec 2017 00:43:48 +0100
> Introduce a device-managed version of pci_set_mwi. First user is the
> Realtek r8169 driver.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Bjorn, can I get an ACK on this? And would you be OK with this going
through my tree as the patches later in this series give an example of
usage?
Thanks.
^ permalink raw reply
* Re: Linux 4.15-rc3 (uml + bpf_perf_event.h)
From: Richard Weinberger @ 2017-12-11 18:49 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Randy Dunlap, netdev@vger.kernel.org, Linus Torvalds,
Linux Kernel Mailing List, alexei.starovoitov, brueckner
In-Reply-To: <ea01fc20-a122-e6d3-e147-2de7f9fa7e47@iogearbox.net>
Am Montag, 11. Dezember 2017, 11:19:54 CET schrieb Daniel Borkmann:
> Hi Randy, hi Richard, [ +Hendrik for c895f6f703ad7dd2f ]
>
> On 12/11/2017 09:32 AM, Richard Weinberger wrote:
> > Randy,
> >
> > Am Montag, 11. Dezember 2017, 03:42:12 CET schrieb Randy Dunlap:
> >> On 12/10/2017 06:08 PM, Linus Torvalds wrote:
> >>> Another week, another rc.
> >>
> >> um (uml) won't build on i386 or x86_64:
> >> CC init/main.o
> >>
> >> In file included from ../include/linux/perf_event.h:18:0,
> >>
> >> from ../include/linux/trace_events.h:10,
> >> from ../include/trace/syscall.h:7,
> >> from ../include/linux/syscalls.h:82,
> >>
> >> from ../init/main.c:20:
> >> ../include/uapi/linux/bpf_perf_event.h:11:32: fatal error:
> >> asm/bpf_perf_event.h: No such file or directory #include
> >> <asm/bpf_perf_event.h>
> >>
> >> ^
> >>
> >> compilation terminated.
> >> ../scripts/Makefile.build:310: recipe for target 'init/main.o' failed
> >
> > How do you trigger that build failure?
> > Can you share your .config?
>
> Hmm, too bad kbuild bot doesn't catch issues on uml. I'm not too familiar
> with uml, but looks like it's the only special case where there's no
> arch/um/include/uapi/asm/. What is the usual convention to pull in such
> headers in this case? Something like the below, would that fix it for you?
>
> Thanks for your help,
> Daniel
>
> arch/um/include/asm/bpf_perf_event.h | 1 +
> include/asm-generic/bpf_perf_event.h | 1 +
> 2 files changed, 2 insertions(+)
> create mode 100644 arch/um/include/asm/bpf_perf_event.h
> create mode 100644 include/asm-generic/bpf_perf_event.h
>
> diff --git a/arch/um/include/asm/bpf_perf_event.h
> b/arch/um/include/asm/bpf_perf_event.h new file mode 100644
> index 0000000..3097758
> --- /dev/null
> +++ b/arch/um/include/asm/bpf_perf_event.h
> @@ -0,0 +1 @@
> +#include <asm-generic/bpf_perf_event.h>
> diff --git a/include/asm-generic/bpf_perf_event.h
> b/include/asm-generic/bpf_perf_event.h new file mode 100644
> index 0000000..67112e5
> --- /dev/null
> +++ b/include/asm-generic/bpf_perf_event.h
> @@ -0,0 +1 @@
> +#include <uapi/asm-generic/bpf_perf_event.h>
Hmm, what about this?
diff --git a/arch/um/include/asm/Kbuild b/arch/um/include/asm/Kbuild
index 50a32c33d729..fb35ec000433 100644
--- a/arch/um/include/asm/Kbuild
+++ b/arch/um/include/asm/Kbuild
@@ -27,3 +27,4 @@ generic-y += trace_clock.h
generic-y += word-at-a-time.h
generic-y += xor.h
generic-y += kprobes.h
+generic-y += bpf_perf_event.h
--
sigma star gmbh - Eduard-Bodem-Gasse 6 - 6020 Innsbruck - Austria
ATU66964118 - FN 374287y
^ permalink raw reply related
* Re: [PATCH] net: dsa: allow XAUI phy interface mode
From: David Miller @ 2017-12-11 18:54 UTC (permalink / raw)
To: rmk+kernel; +Cc: andrew, vivien.didelot, f.fainelli, netdev
In-Reply-To: <E1eNL91-0007uX-Ig@rmk-PC.armlinux.org.uk>
From: Russell King <rmk+kernel@armlinux.org.uk>
Date: Fri, 08 Dec 2017 16:04:59 +0000
> XGMII is a 32-bit bus plus two clock signals per direction. XAUI is
> four serial lanes per direction. The 88e6190 supports XAUI but not
> XGMII as it doesn't have enough pins. The same is true of 88e6176.
>
> Match on PHY_INTERFACE_MODE_XAUI for the XAUI port type, but keep
> accepting XGMII for backwards compatibility.
>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
Based upon the discussion of this patch I am expecting to receive
a DT patch that replaces this one, and some time in the future the
XGMII case statement providing backwards compatibility will be
removed.
Please let me know if this is not the case.
THanks.
^ permalink raw reply
* Re: Linux 4.15-rc3 (uml + bpf_perf_event.h)
From: Randy Dunlap @ 2017-12-11 18:56 UTC (permalink / raw)
To: Richard Weinberger, Daniel Borkmann
Cc: netdev@vger.kernel.org, Linus Torvalds, Linux Kernel Mailing List,
alexei.starovoitov, brueckner
In-Reply-To: <1879970.PcO3yMVeYC@blindfold>
On 12/11/2017 10:49 AM, Richard Weinberger wrote:
> Am Montag, 11. Dezember 2017, 11:19:54 CET schrieb Daniel Borkmann:
>> Hi Randy, hi Richard, [ +Hendrik for c895f6f703ad7dd2f ]
>>
>> On 12/11/2017 09:32 AM, Richard Weinberger wrote:
>>> Randy,
>>>
>>> Am Montag, 11. Dezember 2017, 03:42:12 CET schrieb Randy Dunlap:
>>>> On 12/10/2017 06:08 PM, Linus Torvalds wrote:
>>>>> Another week, another rc.
>>>>
>>>> um (uml) won't build on i386 or x86_64:
>>>> CC init/main.o
>>>>
>>>> In file included from ../include/linux/perf_event.h:18:0,
>>>>
>>>> from ../include/linux/trace_events.h:10,
>>>> from ../include/trace/syscall.h:7,
>>>> from ../include/linux/syscalls.h:82,
>>>>
>>>> from ../init/main.c:20:
>>>> ../include/uapi/linux/bpf_perf_event.h:11:32: fatal error:
>>>> asm/bpf_perf_event.h: No such file or directory #include
>>>> <asm/bpf_perf_event.h>
>>>>
>>>> ^
>>>>
>>>> compilation terminated.
>>>> ../scripts/Makefile.build:310: recipe for target 'init/main.o' failed
>>>
>>> How do you trigger that build failure?
>>> Can you share your .config?
>>
>> Hmm, too bad kbuild bot doesn't catch issues on uml. I'm not too familiar
>> with uml, but looks like it's the only special case where there's no
>> arch/um/include/uapi/asm/. What is the usual convention to pull in such
>> headers in this case? Something like the below, would that fix it for you?
>>
>> Thanks for your help,
>> Daniel
>>
>> arch/um/include/asm/bpf_perf_event.h | 1 +
>> include/asm-generic/bpf_perf_event.h | 1 +
>> 2 files changed, 2 insertions(+)
>> create mode 100644 arch/um/include/asm/bpf_perf_event.h
>> create mode 100644 include/asm-generic/bpf_perf_event.h
>>
>> diff --git a/arch/um/include/asm/bpf_perf_event.h
>> b/arch/um/include/asm/bpf_perf_event.h new file mode 100644
>> index 0000000..3097758
>> --- /dev/null
>> +++ b/arch/um/include/asm/bpf_perf_event.h
>> @@ -0,0 +1 @@
>> +#include <asm-generic/bpf_perf_event.h>
>> diff --git a/include/asm-generic/bpf_perf_event.h
>> b/include/asm-generic/bpf_perf_event.h new file mode 100644
>> index 0000000..67112e5
>> --- /dev/null
>> +++ b/include/asm-generic/bpf_perf_event.h
>> @@ -0,0 +1 @@
>> +#include <uapi/asm-generic/bpf_perf_event.h>
>
> Hmm, what about this?
>
> diff --git a/arch/um/include/asm/Kbuild b/arch/um/include/asm/Kbuild
> index 50a32c33d729..fb35ec000433 100644
> --- a/arch/um/include/asm/Kbuild
> +++ b/arch/um/include/asm/Kbuild
> @@ -27,3 +27,4 @@ generic-y += trace_clock.h
> generic-y += word-at-a-time.h
> generic-y += xor.h
> generic-y += kprobes.h
> +generic-y += bpf_perf_event.h
>
That also works for arch/um.
thanks,
--
~Randy
^ permalink raw reply
* Re: [PATCH] net: dsa: allow XAUI phy interface mode
From: Russell King - ARM Linux @ 2017-12-11 19:02 UTC (permalink / raw)
To: David Miller; +Cc: andrew, vivien.didelot, f.fainelli, netdev
In-Reply-To: <20171211.135403.1074292653549923685.davem@davemloft.net>
On Mon, Dec 11, 2017 at 01:54:03PM -0500, David Miller wrote:
> From: Russell King <rmk+kernel@armlinux.org.uk>
> Date: Fri, 08 Dec 2017 16:04:59 +0000
>
> > XGMII is a 32-bit bus plus two clock signals per direction. XAUI is
> > four serial lanes per direction. The 88e6190 supports XAUI but not
> > XGMII as it doesn't have enough pins. The same is true of 88e6176.
> >
> > Match on PHY_INTERFACE_MODE_XAUI for the XAUI port type, but keep
> > accepting XGMII for backwards compatibility.
> >
> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
>
> Based upon the discussion of this patch I am expecting to receive
> a DT patch that replaces this one, and some time in the future the
> XGMII case statement providing backwards compatibility will be
> removed.
>
> Please let me know if this is not the case.
That's something like it - asked Andrew via IRC on Saturday:
Do we want to do the XAUI thing as one patch to avoid any breakage, and
if so via whom?
and haven't had a response. Normally arm-soc folk want patches
touching arch/arm/boot/dts to go via their tree.
So, until I know what to do, I can't proceed with this... and I've
soo many outstanding net patches now that I'm losing track of where
each patch I've sent out is. Sorry, I'm failing.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
^ permalink raw reply
* Re: [RFC PATCH 0/9] ethtool netlink interface (WiP)
From: Florian Fainelli @ 2017-12-11 19:03 UTC (permalink / raw)
To: David Miller, jiri; +Cc: mkubecek, netdev, linux-kernel
In-Reply-To: <20171211.120144.1060832843526341781.davem@davemloft.net>
On 12/11/2017 09:01 AM, David Miller wrote:
> From: Jiri Pirko <jiri@resnulli.us>
> Date: Mon, 11 Dec 2017 17:32:46 +0100
>
>> I think that it does not make sense to convert ethtool->netlink_ethtool
>> 1:1 feature wise. Now we have devlink, ritch switch representation
>> model, tc offload and many others. Lot of things that are in
>> ethtool, should be done in devlink. Also, there are couple of things
>> that should just die - nice example is ethtool --config-ntuple - we
>> should use tc for that.
>
> Whilst I do agree that devlink is probably a good place for this stuff
> (we want to be able to do ethetool things on objects that lack a netdev)
> I do not agree with the tc angle.
>
> It is entirely appropriate to set the ntuple settings of a driver
> without being required to use TC or similar.
>
> All you are going to do with your suggestion is make people keep using
> the existing ethtool ioctl, because they'll say "screw this, I'm not
> using TC I have something which works just fine already". And that's
> not the goal of putting this stuff into netlink, we want people to
> use the new facilities and move off of the ioctl.
I agree, we can't walk away from that feature today, there are many more
drivers implementing ethtool::ntuple (counting 22) than there are
implementing cls_flower (counting 6), also they are not strictly
equivalent feature wise, one thing critically missing in cls_flower
(last I looked) is the ability to either auto-place (RX_CLS_LOC_ANY) a
matching rule, or select a particular location. For specifying what to
match in a packet and how, cls_flower is superior.
We can probably advise people not to use that feature and request their
driver provider to switch over to cls_flower, but considering how many
more LOCs are needed in the driver to implement that (as opposed to
ethtool ntuple), I just don't see it being something people will be
thrilled to do.
Writing a shim that converts from ethtool ntuple to the equivalent in
kernel space representation of the same call through cls_flower would be
reasonably easy, but same thing, that still requires migrating your
kernel driver over to the cls_flower interface, which is the harder part.
That being said, we should probably discourage implementing both for new
drivers, since they are likely to use the same centralized HW resources
towards the same goals: match + actions, with no visibility into one
another (can't see rules set-up in ethtool via cls_flower and vice
versa) and that just sucks.
--
Florian
^ permalink raw reply
* Re: [PATCHv2 net-next 1/1] forcedeth: remove unnecessary structure member
From: David Miller @ 2017-12-11 19:04 UTC (permalink / raw)
To: yanjun.zhu; +Cc: netdev, keescook
In-Reply-To: <1512875246-28258-1-git-send-email-yanjun.zhu@oracle.com>
From: Zhu Yanjun <yanjun.zhu@oracle.com>
Date: Sat, 9 Dec 2017 22:07:26 -0500
> Since both tx_ring and first_tx are the head of tx ring, it not
> necessary to use two structure members to statically indicate
> the head of tx ring. So first_tx is removed.
>
> CC: Srinivas Eeda <srinivas.eeda@oracle.com>
> CC: Joe Jin <joe.jin@oracle.com>
> CC: Junxiao Bi <junxiao.bi@oracle.com>
> Signed-off-by: Zhu Yanjun <yanjun.zhu@oracle.com>
I suspect this extraneous member was meant to be used for multiqueue
support or something like that.
But, it never got really used, and your change is correct.
Applied, thanks.
^ permalink raw reply
* Re: [PATCH 27/45] net: remove duplicate includes
From: Pablo Neira Ayuso @ 2017-12-11 19:04 UTC (permalink / raw)
To: Pravin Shedge
Cc: netdev, netfilter-devel, coreteam, davem, andrew, vivien.didelot,
f.fainelli, kadlec, fw, jhs, xiyou.wangcong, mingo, rmk+kernel,
linux-kernel
In-Reply-To: <1513010386-3394-1-git-send-email-pravin.shedge4linux@gmail.com>
On Mon, Dec 11, 2017 at 10:09:46PM +0530, Pravin Shedge wrote:
> These duplicate includes have been found with scripts/checkincludes.pl but
> they have been removed manually to avoid removing false positives.
>
> Signed-off-by: Pravin Shedge <pravin.shedge4linux@gmail.com>
For the Netfilter chunk.
Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
Thanks.
^ permalink raw reply
* Re: [PATCH] net: ipv4: fix for a race condition in raw_sendmsg
From: David Miller @ 2017-12-11 19:05 UTC (permalink / raw)
To: simo.ghannam; +Cc: netdev
In-Reply-To: <5a2caf2e.4ce61c0a.5017a.575f@mx.google.com>
From: simo.ghannam@gmail.com
Date: Sun, 10 Dec 2017 03:50:58 +0000
> From: Mohamed Ghannam <simo.ghannam@gmail.com>
>
> inet->hdrincl is racy, and could lead to uninitialized stack pointer
> usage, so its value should be read only once.
>
> Signed-off-by: Mohamed Ghannam <simo.ghannam@gmail.com>
Applied and queued up for -stable with Fixes: tag from Eric Dumazet added.
Thanks.
^ permalink raw reply
* Re: [PATCH net] sctp: make sure stream nums can match optlen in sctp_setsockopt_reset_streams
From: David Miller @ 2017-12-11 19:09 UTC (permalink / raw)
To: lucien.xin; +Cc: netdev, linux-sctp, marcelo.leitner, nhorman, syzkaller
In-Reply-To: <a6c86b9cb13c2b6e2ed2539c028557a6b1a713ef.1512891651.git.lucien.xin@gmail.com>
From: Xin Long <lucien.xin@gmail.com>
Date: Sun, 10 Dec 2017 15:40:51 +0800
> Now in sctp_setsockopt_reset_streams, it only does the check
> optlen < sizeof(*params) for optlen. But it's not enough, as
> params->srs_number_streams should also match optlen.
>
> If the streams in params->srs_stream_list are less than stream
> nums in params->srs_number_streams, later when dereferencing
> the stream list, it could cause a slab-out-of-bounds crash, as
> reported by syzbot.
>
> This patch is to fix it by also checking the stream numbers in
> sctp_setsockopt_reset_streams to make sure at least it's not
> greater than the streams in the list.
>
> Fixes: 7f9d68ac944e ("sctp: implement sender-side procedures for SSN Reset Request Parameter")
> Reported-by: Dmitry Vyukov <dvyukov@google.com>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
Applied and queued up for -stable, thanks.
^ permalink raw reply
* Re: [PATCH net] fou: fix some member types in guehdr
From: David Miller @ 2017-12-11 19:10 UTC (permalink / raw)
To: lucien.xin; +Cc: netdev, therbert
In-Reply-To: <5e022d6a50c6a9f8de5e12bbd2018217475ba6c3.1512896160.git.lucien.xin@gmail.com>
From: Xin Long <lucien.xin@gmail.com>
Date: Sun, 10 Dec 2017 16:56:00 +0800
> guehdr struct is used to build or parse gue packets, which
> are always in big endian. It's better to define all guehdr
> members as __beXX types.
>
> Also, in validate_gue_flags it's not good to use a __be32
> variable for both Standard flags(__be16) and Private flags
> (__be32), and pass it to other funcions.
>
> This patch could fix a bunch of sparse warnings from fou.
>
> Fixes: 5024c33ac354 ("gue: Add infrastructure for flags and options")
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
Looks good, applied, thanks.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox