* Re: [RFC PATCH 2/9] ethtool: introduce ethtool netlink interface
From: Jiri Pirko @ 2017-12-11 16:02 UTC (permalink / raw)
To: Michal Kubecek; +Cc: netdev, linux-kernel
In-Reply-To: <37174026f818dd1a61ca1b37bef2f1b114198de2.1513000306.git.mkubecek@suse.cz>
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.
>+};
>+
>+#define ETHNL_HDRLEN NLMSG_ALIGN(sizeof(struct ethnlmsghdr))
>+
>+enum {
>+ ETHTOOL_CMD_NOOP,
>+
>+ __ETHTOOL_CMD_MAX,
>+ ETHTOOL_CMD_MAX = (__ETHTOOL_CMD_MAX - 1),
>+};
>+
>+/* generic netlink info */
>+#define ETHTOOL_GENL_NAME "ethtool"
>+#define ETHTOOL_GENL_VERSION 1
>+
>+#endif /* _UAPI_LINUX_ETHTOOL_NETLINK_H_ */
>diff --git a/net/Kconfig b/net/Kconfig
>index 9dba2715919d..a5e3c89a2495 100644
>--- a/net/Kconfig
>+++ b/net/Kconfig
>@@ -440,6 +440,13 @@ config MAY_USE_DEVLINK
> on MAY_USE_DEVLINK to ensure they do not cause link errors when
> devlink is a loadable module and the driver using it is built-in.
>
>+config ETHTOOL_NETLINK
>+ tristate "Netlink interface for ethtool"
>+ default m
>+ help
>+ New netlink based interface for ethtool which is going to obsolete
>+ the old ioctl based one once it provides all features.
>+
> endif # if NET
>
> # Used by archs to tell that they support BPF JIT compiler plus which flavour.
>diff --git a/net/core/Makefile b/net/core/Makefile
>index 1fd0a9c88b1b..617ab2abecdf 100644
>--- a/net/core/Makefile
>+++ b/net/core/Makefile
>@@ -30,3 +30,4 @@ obj-$(CONFIG_DST_CACHE) += dst_cache.o
> obj-$(CONFIG_HWBM) += hwbm.o
> obj-$(CONFIG_NET_DEVLINK) += devlink.o
> obj-$(CONFIG_GRO_CELLS) += gro_cells.o
>+obj-$(CONFIG_ETHTOOL_NETLINK) += ethtool_netlink.o
>diff --git a/net/core/ethtool_netlink.c b/net/core/ethtool_netlink.c
>new file mode 100644
>index 000000000000..46a226bb9a2c
>--- /dev/null
>+++ b/net/core/ethtool_netlink.c
>@@ -0,0 +1,46 @@
>+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
>+
>+#include <linux/module.h>
>+#include <linux/ethtool_netlink.h>
>+#include <linux/netdevice.h>
>+#include <net/genetlink.h>
>+#include "ethtool_common.h"
>+
>+static struct genl_family ethtool_genl_family;
>+
>+/* genetlink paperwork */
What "paperwork"? :)
>+
>+static const struct genl_ops ethtool_genl_ops[] = {
>+};
>+
>+static struct genl_family ethtool_genl_family = {
>+ .hdrsize = ETHNL_HDRLEN,
>+ .name = ETHTOOL_GENL_NAME,
>+ .version = ETHTOOL_GENL_VERSION,
>+ .netnsok = true,
>+ .ops = ethtool_genl_ops,
>+ .n_ops = ARRAY_SIZE(ethtool_genl_ops),
>+};
>+
>+/* module paperwork */
>+
>+static int __init ethtool_nl_init(void)
>+{
>+ return genl_register_family(ðtool_genl_family);
>+}
>+
>+static void __exit ethtool_nl_exit(void)
>+{
>+ genl_unregister_family(ðtool_genl_family);
>+}
>+
>+module_init(ethtool_nl_init);
>+module_exit(ethtool_nl_exit);
>+
>+/* this alias is for autoload */
>+MODULE_ALIAS("net-pf-" __stringify(PF_NETLINK)
>+ "-proto-" __stringify(NETLINK_GENERIC)
>+ "-family-" ETHTOOL_GENL_NAME);
You can use MODULE_ALIAS_GENL_FAMILY instead.
>+MODULE_AUTHOR("Michal Kubecek <mkubecek@suse.cz>");
>+MODULE_DESCRIPTION("Netlink interface for ethtool");
>+MODULE_LICENSE("GPL");
"GPL v2"?
>--
>2.15.1
>
^ permalink raw reply
* Re: [Patch net-next 0/2] netlink: improve netlink tap code
From: David Miller @ 2017-12-11 15:57 UTC (permalink / raw)
To: xiyou.wangcong; +Cc: netdev
In-Reply-To: <20171206230320.22191-1-xiyou.wangcong@gmail.com>
From: Cong Wang <xiyou.wangcong@gmail.com>
Date: Wed, 6 Dec 2017 15:03:18 -0800
> Cong Wang (2):
> netlink: make netlink tap per netns
> netlink: convert netlink tap spinlock to mutex
Both applied, thanks Cong.
^ permalink raw reply
* Re: Linux 4.14 - regression: broken tun/tap / bridge network with virtio - bisected
From: Andreas Hartmann @ 2017-12-11 15:54 UTC (permalink / raw)
To: Willem de Bruijn, Michal Kubecek
Cc: Jason Wang, David Miller, Network Development
In-Reply-To: <bc075f2c-85d0-cf97-60cd-ee6d6efca12e@01019freenet.de>
[-- Attachment #1: Type: text/plain, Size: 4176 bytes --]
On 12/08/2017 at 09:44 PM Andreas Hartmann wrote:
> On 12/08/2017 at 09:11 PM Andreas Hartmann wrote:
>> On 12/08/2017 at 05:04 PM Willem de Bruijn wrote:
>>> On Fri, Dec 8, 2017 at 6:40 AM, Michal Kubecek <mkubecek@suse.cz> wrote:
>>>> On Fri, Dec 08, 2017 at 11:31:50AM +0100, Andreas Hartmann wrote:
>>>>> On 12/08/2017 at 09:47 AM Michal Kubecek wrote:
>>>>>> On Fri, Dec 08, 2017 at 08:21:16AM +0100, Andreas Hartmann wrote:
>>>>>>>
>>>>>>> All my VMs are using virtio_net. BTW: I couldn't see the problems
>>>>>>> (sometimes, the VM couldn't be stopped at all) if all my VMs are using
>>>>>>> e1000 as interface instead.
>>>>>>>
>>>>>>> This finding now matches pretty much the responsible UDP-package which
>>>>>>> caused the stall. I already mentioned it here [2].
>>>>>>>
>>>>>>> To prove it, I reverted from the patch series "[PATCH v2 RFC 0/13]
>>>>>>> Remove UDP Fragmentation Offload support" [3]
>>>>>>>
>>>>>>> 11/13 [v2,RFC,11/13] net: Remove all references to SKB_GSO_UDP. [4]
>>>>>>> 12/13 [v2,RFC,12/13] inet: Remove software UFO fragmenting code. [5]
>>>>>>> 13/13 [v2,RFC,13/13] net: Kill NETIF_F_UFO and SKB_GSO_UDP. [6]
>>>>>>>
>>>>>>> and applied it to Linux 4.14.4. It compiled fine and is running fine.
>>>>>>> The vnet doesn't die anymore. Yet, I can't say if the qemu stop hangs
>>>>>>> are gone, too.
>>>>>>>
>>>>>>> Obviously, there is something broken with the new UDP handling. Could
>>>>>>> you please analyze this problem? I could test some more patches ... .
>>>>>>
>>>>>> Any chance your VMs were live migrated from pre-4.14 host kernel?
>>>>>
>>>>> No - the VMs are not live migrated. They are always running on the same
>>>>> host - either with kernel < 4.14 or with kernel 4.14.x.
>>>>
>>>> This is disturbing... unless I'm mistaken, it shouldn't be possible to
>>>> have UFO enabled on a virtio device in a VM booted on a host with 4.14
>>>> kernel.
>>>
>>> Indeed. When working on that revert patch I verified that UFO in
>>> the guest virtio_net was off before the revert patch, on after.
>>>
>>> Qemu should check host support with tap_probe_has_ufo
>>> before advertising support to the guest. Indeed, this is exactly
>>> what broke live migration in virtio_net_load_device at
>>>
>>> if (qemu_get_byte(f) && !peer_has_ufo(n)) {
>>> error_report("virtio-net: saved image requires TUN_F_UFO support");
>>> return -1;
>>> }
>>>
>>> Which follows
>>>
>>> peer_has_ufo
>>> qemu_has_ufo
>>> tap_has_ufo
>>> s->has_ufo
>>>
>>> where s->has_ufo was set by tap_probe_has_ufo in net_tap_fd_init.
>>>
>>> Now, checking my qemu git branch, I ran pretty old 2.7.0-rc3. But this
>>> codepath does not seem to have changed between then and 2.10.1.
>>>
>>> I cherry-picked the revert onto 4.14.3. It did not apply cleanly, but the
>>> fix-up wasn't too hard. Compiled and booted, but untested otherwise. At
>>>
>>> https://github.com/wdebruij/linux/commits/v4.14.3-aargh-ufo
>>
>> I'm just running it at the moment. I didn't face any network hang until
>> now - although the critical UDP packages have been gone through.
>> Therefore: looks nice.
>
> Well, the patch does not fix hanging VMs, which have been shutdown and
> can't be killed any more.
> Because of the stack trace
>
> [<ffffffffc0d0e3c5>] vhost_net_ubuf_put_and_wait+0x35/0x60 [vhost_net]
> [<ffffffffc0d0f264>] vhost_net_ioctl+0x304/0x870 [vhost_net]
> [<ffffffff9b25460f>] do_vfs_ioctl+0x8f/0x5c0
> [<ffffffff9b254bb4>] SyS_ioctl+0x74/0x80
> [<ffffffff9b00365b>] do_syscall_64+0x5b/0x100
> [<ffffffff9b78e7ab>] entry_SYSCALL64_slow_path+0x25/0x25
> [<ffffffffffffffff>] 0xffffffffffffffff
>
> I was hoping, that the problems could be related - but that seems not to
> be true.
However, it turned out, that reverting the complete patchset "Remove UDP
Fragmentation Offload support" prevent hanging qemu processes. I tested
today and the whole weekend - I didn't face any problem. Before that, I
could see nearly immediately hanging qemu processes after shutdown w/
libvirt.
Tested w/ 4.14.4, qemu 2.6.2 and libvirt 2.0.0 and 4 VMs.
I'll be back if the problem comes up again while the patchset is reverted.
Thanks,
Andreas
[-- Attachment #2: Revert - Remove UDP Fragmentation Offload support.tar.xz --]
[-- Type: application/x-xz, Size: 9152 bytes --]
^ permalink raw reply
* [RFC][PATCH] new byteorder primitives - ..._{replace,get}_bits()
From: Al Viro @ 2017-12-11 15:54 UTC (permalink / raw)
To: Linus Torvalds; +Cc: netdev, linux-kernel
In-Reply-To: <20171211053803.GW21978@ZenIV.linux.org.uk>
A lot of drivers are open-coding the "replace these bits in __be32 with
the following value" kind of primitives. Let's add them to byteorder.h.
Primitives:
{be,le}{16,32,64}_replace_bits(old, v, bit, nbits)
{be,le}{16,32,64}_get_bits(val, bit, nbits)
Essentially, it gives helpers for work with bitfields in fixed-endian.
Suppose we have e.g. a little-endian 32bit value with fixed layout;
expressing that as a bitfield would go like
struct foo {
unsigned foo:4; /* bits 0..3 */
unsigned :2;
unsigned bar:12; /* bits 6..17 */
unsigned baz:14; /* bits 18..31 */
}
Even for host-endian it doesn't work all that well - you end up with
ifdefs in structure definition and generated code stinks. For fixed-endian
it gets really painful, and people tend to use explicit shift-and-mask
kind of macros for accessing the fields (and often enough get the
endianness conversions wrong, at that). With these primitives
struct foo v <=> __le32 v
v.foo = i ? 1 : 2 <=> v = le32_replace_bits(v, i ? 1 : 2, 0, 4)
f(4 + v.baz) <=> f(4 + le32_get_bits(v, 18, 14))
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
diff --git a/include/linux/byteorder/generic.h b/include/linux/byteorder/generic.h
index 451aaa0786ae..d8f169a7104a 100644
--- a/include/linux/byteorder/generic.h
+++ b/include/linux/byteorder/generic.h
@@ -187,4 +187,26 @@ static inline void be32_to_cpu_array(u32 *dst, const __be32 *src, size_t len)
dst[i] = be32_to_cpu(src[i]);
}
+#define ____MASK(bit, nbits) ((((1ULL << ((nbits) - 1)) << 1) - 1) << (bit))
+#define ____MAKE_OP(type,base) \
+static inline __##type type##_replace_bits(__##type old, \
+ base val, int bit, int nbits) \
+{ \
+ __##type mask = cpu_to_##type(____MASK(bit, nbits)); \
+ return (old & ~mask) | (cpu_to_##type(val << bit) & mask); \
+} \
+static inline base type##_get_bits(__##type val, int bit, int nbits) \
+{ \
+ return (type##_to_cpu(val) >> bit) & ____MASK(0, nbits); \
+}
+
+____MAKE_OP(le16,u16)
+____MAKE_OP(le32,u32)
+____MAKE_OP(le64,u64)
+____MAKE_OP(be16,u16)
+____MAKE_OP(be32,u32)
+____MAKE_OP(be64,u64)
+#undef ____MAKE_OP
+#undef ____MASK
+
#endif /* _LINUX_BYTEORDER_GENERIC_H */
^ permalink raw reply related
* Re: [PATCH] ptr_ring: add barriers
From: David Miller @ 2017-12-11 15:53 UTC (permalink / raw)
To: mst
Cc: linux-kernel, george.cherian, jasowang, edumazet, netdev,
virtualization
In-Reply-To: <1512501990-30029-1-git-send-email-mst@redhat.com>
From: "Michael S. Tsirkin" <mst@redhat.com>
Date: Tue, 5 Dec 2017 21:29:37 +0200
> Users of ptr_ring expect that it's safe to give the
> data structure a pointer and have it be available
> to consumers, but that actually requires an smb_wmb
> or a stronger barrier.
>
> In absence of such barriers and on architectures that reorder writes,
> consumer might read an un=initialized value from an skb pointer stored
> in the skb array. This was observed causing crashes.
>
> To fix, add memory barriers. The barrier we use is a wmb, the
> assumption being that producers do not need to read the value so we do
> not need to order these reads.
>
> Reported-by: George Cherian <george.cherian@cavium.com>
> Suggested-by: Jason Wang <jasowang@redhat.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
I'm asked for asking for testing feedback and did not get it in a
reasonable amount of time.
So I'm applying this as-is, and queueing it up for -stable.
Thank you.
^ permalink raw reply
* Re: [PATCH net v2 7/9] net: aquantia: Reset driver level statistics to zero on initialization
From: Andrew Lunn @ 2017-12-11 15:51 UTC (permalink / raw)
To: Igor Russkikh
Cc: David S . Miller, netdev, David Arcari, Pavel Belous,
Nadezhda Krupnina, Simon Edelhaus
In-Reply-To: <99425adad58a8ed133edc1a2b5ec8fec70e56e36.1512994559.git.igor.russkikh@aquantia.com>
> --- a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_a0.c
> +++ b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_a0.c
> @@ -344,6 +344,13 @@ static int hw_atl_a0_hw_init(struct aq_hw_s *self,
> hw_atl_a0_hw_rss_set(self, &aq_nic_cfg->aq_rss);
> hw_atl_a0_hw_rss_hash_set(self, &aq_nic_cfg->aq_rss);
>
> + /* Read initial hardware counters
> + * and reset current in-driver statistics
> + */
> + hw_atl_utils_update_stats(self);
> + memset(&PHAL_ATLANTIC_A0->curr_stats, 0,
> + sizeof(PHAL_ATLANTIC_A0->curr_stats));
> +
> err = aq_hw_err_from_flags(self);
> if (err < 0)
> goto err_exit;
Hi Igor
It looks like this is called on pm state change? Does this mean that
resume will zero the statistics? That is probably not what you want.
You should probably only clear the statics in the probe function.
Andrew
^ permalink raw reply
* Re: [PATCH net v2 8/9] net: aquantia: Fix typo in ethtool statistics names
From: David Miller @ 2017-12-11 15:51 UTC (permalink / raw)
To: andrew
Cc: igor.russkikh, netdev, darcari, pavel.belous, Nadezhda.Krupnina,
simon.edelhaus
In-Reply-To: <20171211154506.GG28672@lunn.ch>
From: Andrew Lunn <andrew@lunn.ch>
Date: Mon, 11 Dec 2017 16:45:06 +0100
> I know it is not nice, but maybe we need to consider this is part of
> the kernel ABI? You could be breaking applications by making this
> change.
>
> What do others think?
Not really.
The whole reason we have a string array exposed to userspace is so that
tools absolutely cannot hard code the driver specific ethtool stats
in any way shape or form, and must always query the number, order,
and names every single time.
^ permalink raw reply
* Re: [PATCH net v2 4/9] net: aquantia: Fill ndev stat couters from hardware
From: Andrew Lunn @ 2017-12-11 15:47 UTC (permalink / raw)
To: Igor Russkikh
Cc: David S . Miller, netdev, David Arcari, Pavel Belous,
Nadezhda Krupnina, Simon Edelhaus
In-Reply-To: <2a99c143ef60ccd48c2720c1a9ad57222f37952e.1512994559.git.igor.russkikh@aquantia.com>
On Mon, Dec 11, 2017 at 03:16:21PM +0300, Igor Russkikh wrote:
> Originally they were filled from ring sw counters.
> These sometimes incorrectly calculate byte and packet amounts
> when using LRO/LSO and jumboframes. Filling ndev counters from
> hardware makes them precise.
>
> Signed-off-by: Igor Russkikh <igor.russkikh@aquantia.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH] iptables: ip6t_MASQUERADE: add dependency on conntrack module
From: Pablo Neira Ayuso @ 2017-12-11 15:47 UTC (permalink / raw)
To: Konstantin Khlebnikov
Cc: netdev, David S. Miller, Eric Dumazet, Florian Westphal
In-Reply-To: <151300557384.454702.4055341451761100641.stgit@buzz>
On Mon, Dec 11, 2017 at 06:19:33PM +0300, Konstantin Khlebnikov wrote:
> After commit 4d3a57f23dec ("netfilter: conntrack: do not enable connection
> tracking unless needed") conntrack is disabled by default unless some
> module explicitly declares dependency in particular network namespace.
Applied, thanks.
^ permalink raw reply
* Re: [PATCH net v2 5/9] net: aquantia: Fill in multicast counter in ndev stats from hardware
From: Andrew Lunn @ 2017-12-11 15:46 UTC (permalink / raw)
To: Igor Russkikh
Cc: David S . Miller, netdev, David Arcari, Pavel Belous,
Nadezhda Krupnina, Simon Edelhaus
In-Reply-To: <6386236d1b55525e9f25de48070f96818f4f7a67.1512994559.git.igor.russkikh@aquantia.com>
On Mon, Dec 11, 2017 at 03:16:22PM +0300, Igor Russkikh wrote:
> Signed-off-by: Igor Russkikh <igor.russkikh@aquantia.com>
A one link comment for the change log would be good.
Otherwise,
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH net v2 0/9] net: aquantia: Atlantic driver 12/2017 updates
From: Andrew Lunn @ 2017-12-11 15:45 UTC (permalink / raw)
To: Igor Russkikh
Cc: David S . Miller, netdev, David Arcari, Pavel Belous,
Nadezhda Krupnina, Simon Edelhaus
In-Reply-To: <cover.1512994559.git.igor.russkikh@aquantia.com>
On Mon, Dec 11, 2017 at 03:16:17PM +0300, Igor Russkikh wrote:
> The patchset contains important hardware fix for machines with large MRRS
> and couple of improvement in stats and capabilities reporting
>
> patch v2:
> - split into more detailed commits
Hi Igor
Thanks for breaking up the patches. They are much easier to review.
Andrew
^ permalink raw reply
* Re: [PATCH net v2 8/9] net: aquantia: Fix typo in ethtool statistics names
From: Andrew Lunn @ 2017-12-11 15:45 UTC (permalink / raw)
To: Igor Russkikh
Cc: David S . Miller, netdev, David Arcari, Pavel Belous,
Nadezhda Krupnina, Simon Edelhaus
In-Reply-To: <77ee1884039b2c73fb7542144931e30bdcb2de7f.1512994559.git.igor.russkikh@aquantia.com>
On Mon, Dec 11, 2017 at 03:16:25PM +0300, Igor Russkikh wrote:
> Signed-off-by: Igor Russkikh <igor.russkikh@aquantia.com>
> ---
> drivers/net/ethernet/aquantia/atlantic/aq_ethtool.c | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_ethtool.c b/drivers/net/ethernet/aquantia/atlantic/aq_ethtool.c
> index 70efb74..f2d8063 100644
> --- a/drivers/net/ethernet/aquantia/atlantic/aq_ethtool.c
> +++ b/drivers/net/ethernet/aquantia/atlantic/aq_ethtool.c
> @@ -66,14 +66,14 @@ static const char aq_ethtool_stat_names[][ETH_GSTRING_LEN] = {
> "OutUCast",
> "OutMCast",
> "OutBCast",
> - "InUCastOctects",
> - "OutUCastOctects",
> - "InMCastOctects",
> - "OutMCastOctects",
> - "InBCastOctects",
> - "OutBCastOctects",
> - "InOctects",
> - "OutOctects",
> + "InUCastOctets",
> + "OutUCastOctets",
> + "InMCastOctets",
> + "OutMCastOctets",
> + "InBCastOctets",
> + "OutBCastOctets",
> + "InOctets",
> + "OutOctets",
> "InPacketsDma",
> "OutPacketsDma",
> "InOctetsDma",
Hi Igor
I know it is not nice, but maybe we need to consider this is part of
the kernel ABI? You could be breaking applications by making this
change.
What do others think?
Andrew
^ permalink raw reply
* Re: [PATCH net] sctp: make sure stream nums can match optlen in sctp_setsockopt_reset_streams
From: Marcelo Ricardo Leitner @ 2017-12-11 15:36 UTC (permalink / raw)
To: Neil Horman; +Cc: Xin Long, network dev, linux-sctp, davem, syzkaller
In-Reply-To: <20171211145434.GB18284@hmswarspite.think-freely.org>
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
> 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
>
^ permalink raw reply
* [PATCH] iptables: ip6t_MASQUERADE: add dependency on conntrack module
From: Konstantin Khlebnikov @ 2017-12-11 15:19 UTC (permalink / raw)
To: netdev, David S. Miller; +Cc: Eric Dumazet, Florian Westphal, Pablo Neira Ayuso
After commit 4d3a57f23dec ("netfilter: conntrack: do not enable connection
tracking unless needed") conntrack is disabled by default unless some
module explicitly declares dependency in particular network namespace.
Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
Fixes: a357b3f80bc8 ("netfilter: nat: add dependencies on conntrack module")
---
net/ipv6/netfilter/ip6t_MASQUERADE.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/net/ipv6/netfilter/ip6t_MASQUERADE.c b/net/ipv6/netfilter/ip6t_MASQUERADE.c
index 2b1a15846f9a..92c0047e7e33 100644
--- a/net/ipv6/netfilter/ip6t_MASQUERADE.c
+++ b/net/ipv6/netfilter/ip6t_MASQUERADE.c
@@ -33,13 +33,19 @@ static int masquerade_tg6_checkentry(const struct xt_tgchk_param *par)
if (range->flags & NF_NAT_RANGE_MAP_IPS)
return -EINVAL;
- return 0;
+ return nf_ct_netns_get(par->net, par->family);
+}
+
+static void masquerade_tg6_destroy(const struct xt_tgdtor_param *par)
+{
+ nf_ct_netns_put(par->net, par->family);
}
static struct xt_target masquerade_tg6_reg __read_mostly = {
.name = "MASQUERADE",
.family = NFPROTO_IPV6,
.checkentry = masquerade_tg6_checkentry,
+ .destroy = masquerade_tg6_destroy,
.target = masquerade_tg6,
.targetsize = sizeof(struct nf_nat_range),
.table = "nat",
^ permalink raw reply related
* [PATCH net] ipv4: igmp: guard against silly MTU values
From: Eric Dumazet @ 2017-12-11 15:17 UTC (permalink / raw)
To: David Miller; +Cc: netdev
From: Eric Dumazet <edumazet@google.com>
IPv4 stack reacts to changes to small MTU, by disabling itself under
RTNL.
But there is a window where threads not using RTNL can see a wrong
device mtu. This can lead to surprises, in igmp code where it is
assumed the mtu is suitable.
Fix this by reading device mtu once and checking IPv4 minimal MTU.
This patch adds missing IPV4_MIN_MTU define, to not abuse
ETH_MIN_MTU anymore.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/net/ip.h | 1 +
net/ipv4/devinet.c | 2 +-
net/ipv4/igmp.c | 24 +++++++++++++++---------
net/ipv4/ip_tunnel.c | 4 ++--
4 files changed, 19 insertions(+), 12 deletions(-)
diff --git a/include/net/ip.h b/include/net/ip.h
index 9896f46cbbf11235395d75a5ec18a14736ee099d..af8addbaa3c188a896b74ff9646b6fdd692d1c8e 100644
--- a/include/net/ip.h
+++ b/include/net/ip.h
@@ -34,6 +34,7 @@
#include <net/flow_dissector.h>
#define IPV4_MAX_PMTU 65535U /* RFC 2675, Section 5.1 */
+#define IPV4_MIN_MTU 68 /* RFC 791 */
struct sock;
diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index a4573bccd6da7b6763016d4f07d3032d44483d99..7a93359fbc7229389fc7bec67889ca1115f47a69 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -1428,7 +1428,7 @@ static void inetdev_changename(struct net_device *dev, struct in_device *in_dev)
static bool inetdev_valid_mtu(unsigned int mtu)
{
- return mtu >= 68;
+ return mtu >= IPV4_MIN_MTU;
}
static void inetdev_send_gratuitous_arp(struct net_device *dev,
diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
index d1f8f302dbf3ed5a079f27efa6eeaf802de40243..50448a220a1f26e29715bffc1b86f1f749e5a61d 100644
--- a/net/ipv4/igmp.c
+++ b/net/ipv4/igmp.c
@@ -404,16 +404,17 @@ static int grec_size(struct ip_mc_list *pmc, int type, int gdel, int sdel)
}
static struct sk_buff *add_grhead(struct sk_buff *skb, struct ip_mc_list *pmc,
- int type, struct igmpv3_grec **ppgr)
+ int type, struct igmpv3_grec **ppgr, unsigned int mtu)
{
struct net_device *dev = pmc->interface->dev;
struct igmpv3_report *pih;
struct igmpv3_grec *pgr;
- if (!skb)
- skb = igmpv3_newpack(dev, dev->mtu);
- if (!skb)
- return NULL;
+ if (!skb) {
+ skb = igmpv3_newpack(dev, mtu);
+ if (!skb)
+ return NULL;
+ }
pgr = skb_put(skb, sizeof(struct igmpv3_grec));
pgr->grec_type = type;
pgr->grec_auxwords = 0;
@@ -436,12 +437,17 @@ static struct sk_buff *add_grec(struct sk_buff *skb, struct ip_mc_list *pmc,
struct igmpv3_grec *pgr = NULL;
struct ip_sf_list *psf, *psf_next, *psf_prev, **psf_list;
int scount, stotal, first, isquery, truncate;
+ unsigned int mtu;
if (pmc->multiaddr == IGMP_ALL_HOSTS)
return skb;
if (ipv4_is_local_multicast(pmc->multiaddr) && !net->ipv4.sysctl_igmp_llm_reports)
return skb;
+ mtu = READ_ONCE(dev->mtu);
+ if (mtu < IPV4_MIN_MTU)
+ return skb;
+
isquery = type == IGMPV3_MODE_IS_INCLUDE ||
type == IGMPV3_MODE_IS_EXCLUDE;
truncate = type == IGMPV3_MODE_IS_EXCLUDE ||
@@ -462,7 +468,7 @@ static struct sk_buff *add_grec(struct sk_buff *skb, struct ip_mc_list *pmc,
AVAILABLE(skb) < grec_size(pmc, type, gdeleted, sdeleted)) {
if (skb)
igmpv3_sendpack(skb);
- skb = igmpv3_newpack(dev, dev->mtu);
+ skb = igmpv3_newpack(dev, mtu);
}
}
first = 1;
@@ -498,12 +504,12 @@ static struct sk_buff *add_grec(struct sk_buff *skb, struct ip_mc_list *pmc,
pgr->grec_nsrcs = htons(scount);
if (skb)
igmpv3_sendpack(skb);
- skb = igmpv3_newpack(dev, dev->mtu);
+ skb = igmpv3_newpack(dev, mtu);
first = 1;
scount = 0;
}
if (first) {
- skb = add_grhead(skb, pmc, type, &pgr);
+ skb = add_grhead(skb, pmc, type, &pgr, mtu);
first = 0;
}
if (!skb)
@@ -538,7 +544,7 @@ static struct sk_buff *add_grec(struct sk_buff *skb, struct ip_mc_list *pmc,
igmpv3_sendpack(skb);
skb = NULL; /* add_grhead will get a new one */
}
- skb = add_grhead(skb, pmc, type, &pgr);
+ skb = add_grhead(skb, pmc, type, &pgr, mtu);
}
}
if (pgr)
diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c
index fe6fee728ce49d01b55aa478698e1a3bcf9a3bdb..5ddb1cb52bd405ed10cce43195a25607d136efbf 100644
--- a/net/ipv4/ip_tunnel.c
+++ b/net/ipv4/ip_tunnel.c
@@ -349,8 +349,8 @@ static int ip_tunnel_bind_dev(struct net_device *dev)
dev->needed_headroom = t_hlen + hlen;
mtu -= (dev->hard_header_len + t_hlen);
- if (mtu < 68)
- mtu = 68;
+ if (mtu < IPV4_MIN_MTU)
+ mtu = IPV4_MIN_MTU;
return mtu;
}
^ permalink raw reply related
* Re: RFC(v2): Audit Kernel Container IDs
From: Richard Guy Briggs @ 2017-12-11 15:10 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Casey Schaufler, cgroups-u79uwXL29TY76Z2rM5mHXA, Linux Containers,
Linux API, Linux Audit, Linux FS Devel, Linux Kernel,
Linux Network Development, mszeredi-H+wXaHxf7aLQT0dZR+AlfA,
Eric W. Biederman, Simo Sorce, jlayton-H+wXaHxf7aLQT0dZR+AlfA,
Carlos O'Donell, David Howells, Al Viro, Andy Lutomirski,
Eric Paris, trondmy-7I+n7zu2hftEKMMhf/gKZA, Michael Kerrisk
In-Reply-To: <7ebca85a-425c-2b95-9a5f-59d81707339e-WFhQfpSGs3bR7s880joybQ@public.gmane.org>
On 2017-12-09 11:20, Mickaël Salaün wrote:
>
> On 12/10/2017 18:33, Casey Schaufler wrote:
> > On 10/12/2017 7:14 AM, Richard Guy Briggs wrote:
> >> Containers are a userspace concept. The kernel knows nothing of them.
> >>
> >> The Linux audit system needs a way to be able to track the container
> >> provenance of events and actions. Audit needs the kernel's help to do
> >> this.
> >>
> >> Since the concept of a container is entirely a userspace concept, a
> >> registration from the userspace container orchestration system initiates
> >> this. This will define a point in time and a set of resources
> >> associated with a particular container with an audit container ID.
> >>
> >> The registration is a pseudo filesystem (proc, since PID tree already
> >> exists) write of a u8[16] UUID representing the container ID to a file
> >> representing a process that will become the first process in a new
> >> container. This write might place restrictions on mount namespaces
> >> required to define a container, or at least careful checking of
> >> namespaces in the kernel to verify permissions of the orchestrator so it
> >> can't change its own container ID. A bind mount of nsfs may be
> >> necessary in the container orchestrator's mntNS.
> >> Note: Use a 128-bit scalar rather than a string to make compares faster
> >> and simpler.
> >>
> >> Require a new CAP_CONTAINER_ADMIN to be able to carry out the
> >> registration.
> >
> > Hang on. If containers are a user space concept, how can
> > you want CAP_CONTAINER_ANYTHING? If there's not such thing as
> > a container, how can you be asking for a capability to manage
> > them?
> >
> >> At that time, record the target container's user-supplied
> >> container identifier along with the target container's first process
> >> (which may become the target container's "init" process) process ID
> >> (referenced from the initial PID namespace), all namespace IDs (in the
> >> form of a nsfs device number and inode number tuple) in a new auxilliary
> >> record AUDIT_CONTAINER with a qualifying op=$action field.
>
> Here is an idea to avoid privilege problems or the need for a new
> capability: make it automatic. What makes a container a container seems
> to be the use of at least a namespace. What about automatically create
> and assign an ID to a process when it enters a namespace different than
> one of its parent process? This delegates the (permission)
> responsibility to the use of namespaces (e.g. /proc/sys/user/max_* limit).
A container doesn't imply a namespace and vice versa.
> One interesting side effect of this approach would be to be able to
> identify which processes are in the same set of namespaces, even if not
> spawn from the container but entered after its creation (i.e. using
> setns), by creating container IDs as a (deterministic) checksum from the
> /proc/self/ns/* IDs.
This would be really helpful, but it isn't the case.
> Since the concern is to identify a container, I think the ability to
> audit the switch from one container ID to another is enough. I don't
> think we need nested IDs.
Since container namespace membership is arbitrary between container
orchestrators, this needs a registration process and a way for the
container orchestrator to know the ID.
I completely agree with Casey here.
> As a side note, you may want to take a look at the Linux-VServer's XID.
>
> Regards,
> Mickaël
- RGB
--
Richard Guy Briggs <rgb-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635
^ permalink raw reply
* Re: [PATCH v2 net-next 0/5] rhashtable: New features in walk and bucket
From: David Miller @ 2017-12-11 15:10 UTC (permalink / raw)
To: herbert; +Cc: tom, netdev, rohit
In-Reply-To: <20171211120324.GP12014@gondor.apana.org.au>
From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Mon, 11 Dec 2017 23:03:24 +1100
> On Tue, Dec 05, 2017 at 02:47:58PM -0500, David Miller wrote:
>>
>> I'll allow Herbert time to think about this some more as he requested
>> in comments against the first version of this series.
>
> Sorry for the late response. Tom's changes look good to me.
>
> We should also fix up all existing rhashtable users that dump
> through netlink to use the new peek interface.
Thanks for reviewing Herbert, series applied.
Thanks everyone.
^ permalink raw reply
* [PATCH net] ipv6: mcast: better catch silly mtu values
From: Eric Dumazet @ 2017-12-11 15:03 UTC (permalink / raw)
To: Xin Long, David Miller; +Cc: network dev
In-Reply-To: <1512838770.25033.33.camel@gmail.com>
From: Eric Dumazet <edumazet@google.com>
syzkaller reported crashes in IPv6 stack [1]
Xin Long found that lo MTU was set to silly values.
IPv6 stack reacts to changes to small MTU, by disabling itself under
RTNL.
But there is a window where threads not using RTNL can see a wrong
device mtu. This can lead to surprises, in mld code where it is assumed
the mtu is suitable.
Fix this by reading device mtu once and checking IPv6 minimal MTU.
[1]
skbuff: skb_over_panic: text:0000000010b86b8d len:196 put:20
head:000000003b477e60 data:000000000e85441e tail:0xd4 end:0xc0 dev:lo
------------[ cut here ]------------
kernel BUG at net/core/skbuff.c:104!
invalid opcode: 0000 [#1] SMP KASAN
Dumping ftrace buffer:
(ftrace buffer empty)
Modules linked in:
CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.15.0-rc2-mm1+ #39
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
RIP: 0010:skb_panic+0x15c/0x1f0 net/core/skbuff.c:100
RSP: 0018:ffff8801db307508 EFLAGS: 00010286
RAX: 0000000000000082 RBX: ffff8801c517e840 RCX: 0000000000000000
RDX: 0000000000000082 RSI: 1ffff1003b660e61 RDI: ffffed003b660e95
RBP: ffff8801db307570 R08: 1ffff1003b660e23 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000000 R12: ffffffff85bd4020
R13: ffffffff84754ed2 R14: 0000000000000014 R15: ffff8801c4e26540
FS: 0000000000000000(0000) GS:ffff8801db300000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000463610 CR3: 00000001c6698000 CR4: 00000000001406e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
<IRQ>
skb_over_panic net/core/skbuff.c:109 [inline]
skb_put+0x181/0x1c0 net/core/skbuff.c:1694
add_grhead.isra.24+0x42/0x3b0 net/ipv6/mcast.c:1695
add_grec+0xa55/0x1060 net/ipv6/mcast.c:1817
mld_send_cr net/ipv6/mcast.c:1903 [inline]
mld_ifc_timer_expire+0x4d2/0x770 net/ipv6/mcast.c:2448
call_timer_fn+0x23b/0x840 kernel/time/timer.c:1320
expire_timers kernel/time/timer.c:1357 [inline]
__run_timers+0x7e1/0xb60 kernel/time/timer.c:1660
run_timer_softirq+0x4c/0xb0 kernel/time/timer.c:1686
__do_softirq+0x29d/0xbb2 kernel/softirq.c:285
invoke_softirq kernel/softirq.c:365 [inline]
irq_exit+0x1d3/0x210 kernel/softirq.c:405
exiting_irq arch/x86/include/asm/apic.h:540 [inline]
smp_apic_timer_interrupt+0x16b/0x700 arch/x86/kernel/apic/apic.c:1052
apic_timer_interrupt+0xa9/0xb0 arch/x86/entry/entry_64.S:920
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: syzbot <syzkaller@googlegroups.com>
---
Another fix will be sent separately for ipv4 igmp
net/ipv6/mcast.c | 25 +++++++++++++++----------
1 file changed, 15 insertions(+), 10 deletions(-)
diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
index fc6d7d143f2c29aab9a3f56eae02e5337e65a97b..844642682b8363c4c32d329ed92474f834a59618 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c
@@ -1682,16 +1682,16 @@ static int grec_size(struct ifmcaddr6 *pmc, int type, int gdel, int sdel)
}
static struct sk_buff *add_grhead(struct sk_buff *skb, struct ifmcaddr6 *pmc,
- int type, struct mld2_grec **ppgr)
+ int type, struct mld2_grec **ppgr, unsigned int mtu)
{
- struct net_device *dev = pmc->idev->dev;
struct mld2_report *pmr;
struct mld2_grec *pgr;
- if (!skb)
- skb = mld_newpack(pmc->idev, dev->mtu);
- if (!skb)
- return NULL;
+ if (!skb) {
+ skb = mld_newpack(pmc->idev, mtu);
+ if (!skb)
+ return NULL;
+ }
pgr = skb_put(skb, sizeof(struct mld2_grec));
pgr->grec_type = type;
pgr->grec_auxwords = 0;
@@ -1714,10 +1714,15 @@ static struct sk_buff *add_grec(struct sk_buff *skb, struct ifmcaddr6 *pmc,
struct mld2_grec *pgr = NULL;
struct ip6_sf_list *psf, *psf_next, *psf_prev, **psf_list;
int scount, stotal, first, isquery, truncate;
+ unsigned int mtu;
if (pmc->mca_flags & MAF_NOREPORT)
return skb;
+ mtu = READ_ONCE(dev->mtu);
+ if (mtu < IPV6_MIN_MTU)
+ return skb;
+
isquery = type == MLD2_MODE_IS_INCLUDE ||
type == MLD2_MODE_IS_EXCLUDE;
truncate = type == MLD2_MODE_IS_EXCLUDE ||
@@ -1738,7 +1743,7 @@ static struct sk_buff *add_grec(struct sk_buff *skb, struct ifmcaddr6 *pmc,
AVAILABLE(skb) < grec_size(pmc, type, gdeleted, sdeleted)) {
if (skb)
mld_sendpack(skb);
- skb = mld_newpack(idev, dev->mtu);
+ skb = mld_newpack(idev, mtu);
}
}
first = 1;
@@ -1774,12 +1779,12 @@ static struct sk_buff *add_grec(struct sk_buff *skb, struct ifmcaddr6 *pmc,
pgr->grec_nsrcs = htons(scount);
if (skb)
mld_sendpack(skb);
- skb = mld_newpack(idev, dev->mtu);
+ skb = mld_newpack(idev, mtu);
first = 1;
scount = 0;
}
if (first) {
- skb = add_grhead(skb, pmc, type, &pgr);
+ skb = add_grhead(skb, pmc, type, &pgr, mtu);
first = 0;
}
if (!skb)
@@ -1814,7 +1819,7 @@ static struct sk_buff *add_grec(struct sk_buff *skb, struct ifmcaddr6 *pmc,
mld_sendpack(skb);
skb = NULL; /* add_grhead will get a new one */
}
- skb = add_grhead(skb, pmc, type, &pgr);
+ skb = add_grhead(skb, pmc, type, &pgr, mtu);
}
}
if (pgr)
^ permalink raw reply related
* Re: [PATCH net] sctp: make sure stream nums can match optlen in sctp_setsockopt_reset_streams
From: Neil Horman @ 2017-12-11 14:54 UTC (permalink / raw)
To: Xin Long; +Cc: network dev, linux-sctp, davem, Marcelo Ricardo Leitner,
syzkaller
In-Reply-To: <a6c86b9cb13c2b6e2ed2539c028557a6b1a713ef.1512891651.git.lucien.xin@gmail.com>
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 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
>
^ permalink raw reply
* Re: Waiting for the PHY to complete auto-negotiation
From: Mason @ 2017-12-11 14:47 UTC (permalink / raw)
To: Mans Rullgard; +Cc: Florian Fainelli, Andrew Lunn, netdev, David Miller
In-Reply-To: <yw1xy3m9tjp5.fsf@mansr.com>
On 11/12/2017 15:36, Måns Rullgård wrote:
> Mason writes:
>
>> I suppose I should test forcefully setting the enable bit to 0 in
>> the driver, and see if hell breaks loose.
>
> You can't. When the enable bit is 1, writes to that register are
> ignored. It goes back to 0 automatically when the hw runs out of
> descriptors.
I don't think they are ignored, because toggling the control flow
bit actually breaks RX.
Regards.
^ permalink raw reply
* Re: [PATCH] rtnetlink: fix typo in GSO max segments
From: David Miller @ 2017-12-11 14:46 UTC (permalink / raw)
To: stephen; +Cc: netdev, sthemmin
In-Reply-To: <20171208233413.18677-1-sthemmin@microsoft.com>
From: Stephen Hemminger <stephen@networkplumber.org>
Date: Fri, 8 Dec 2017 15:34:13 -0800
> Fixes: 46e6b992c250 ("rtnetlink: allow GSO maximums to be set on device creation")
> Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
Applied, thanks you.
^ permalink raw reply
* Re: pull-request: mac80211 2017-12-11
From: David Miller @ 2017-12-11 14:39 UTC (permalink / raw)
To: johannes-cdvu00un1VgdHxzADdlk8Q
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
linux-wireless-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20171211095236.10172-1-johannes-cdvu00un1VgdHxzADdlk8Q@public.gmane.org>
From: Johannes Berg <johannes-cdvu00un1VgdHxzADdlk8Q@public.gmane.org>
Date: Mon, 11 Dec 2017 10:52:35 +0100
> Three fixes, two related to build issues with the new regdb stuff,
> and one for some patch overlap problem that caused locking to be
> missing which in turn caused lots of warnings.
>
> Please pull and let me know if there's any problem.
Pulled, thanks.
^ permalink raw reply
* Re: Waiting for the PHY to complete auto-negotiation
From: Måns Rullgård @ 2017-12-11 14:36 UTC (permalink / raw)
To: Mason; +Cc: Florian Fainelli, Andrew Lunn, netdev, David Miller
In-Reply-To: <eebca60e-8a40-3e1d-c7ed-4b3c0f339587@free.fr>
Mason <slash.tmp@free.fr> writes:
> + Mans
>
> On 09/12/2017 19:49, Florian Fainelli wrote:
>
>> Having any HW state machine requiring X number of clock cycles to
>> guarantee a full transition to a stopped state is not unusual, however,
>> the fact that you need to send 5 packets to guarantee an EOC descriptor
>> is hit is completely unusual. Ideally there is a single bit that tells
>> the DMA engine: you are enabled, do your thing, or you are now disabled,
>> and you must stop all accesses to DRAM *now*.
That's how sane hardware works. This hardware is not sane.
>> So what would be the correct way to quiesce that controller according to
>> your HW folks?
>
> He (it's a single person) offered to run some RTL-level simulations,
> but then moved on to more important tasks. At some point he wrote:
>
>> If you reset the DMA enable in the middle of a DMA, the hardware
>> state machine doesn't return to the IDLE state if it has more
>> descriptors to process. It should be noted that the RX DMA has been
>> designed by our IP vendor with no intention of stopping the DMA in
>> the middle of its operation.
>
> While the documentation for the IP states:
>
>> Receive DMA Channel Disabling
>>
>> When the entire receive frame has been read from the Receive FIFO and
>> sent over the AMBA bus, the DMA operation ends, and the Receive DMA
>> Channel is automatically disabled. To do this, hardware resets the
>> Enable bit in the Receive Channel Control Register to "0" after the
>> last data has been read from the Receive FIFO and sent over the AMBA
>> bus.
>>
>> When operating in descriptor mode, upon completion of a receive frame
>> DMA operation, if the descriptor chain has not ended when a receive
>> frame DMA operation completes, the next receive frame DMA operation
>> begins. The last descriptor in a descriptor chain is indicated by
>> having its End Of Chain- EOC, flag set to "1". If this EOC flag is
>> "0", to begin the next receive frame DMA operation, the next
>> descriptor is automatically retrieved and used to configure the
>> Receive DMA Channel. The Receive DMA Channel is then automatically
>> re-enabled and the next receive frame DMA operation begins.
>>
>> In descriptor mode, an AMBA bus error can occur when reading receive
>> descriptor data. If this happens, receive descriptor processing ends
>> and the Receive DMA Channel is turned off. The Descriptor Error bit
>> in the Receive Status Register is set to "1".
>
> I don't see how sending "fake" packets through the loop back would be
> considered "resetting the DMA enable in the middle of a DMA".
> (I'm afraid the HW dev didn't grasp what the driver is doing.)
>
> I suppose I should test forcefully setting the enable bit to 0 in
> the driver, and see if hell breaks loose.
You can't. When the enable bit is 1, writes to that register are
ignored. It goes back to 0 automatically when the hw runs out of
descriptors.
--
Måns Rullgård
^ permalink raw reply
* Re: [PATCH net-next v5 2/2] net: ethernet: socionext: add AVE ethernet driver
From: David Miller @ 2017-12-11 14:35 UTC (permalink / raw)
To: hayashi.kunihiko
Cc: netdev, andrew, f.fainelli, robh+dt, mark.rutland,
linux-arm-kernel, linux-kernel, devicetree, yamada.masahiro,
masami.hiramatsu, jaswinder.singh
In-Reply-To: <1512979049-15930-3-git-send-email-hayashi.kunihiko@socionext.com>
From: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
Date: Mon, 11 Dec 2017 16:57:29 +0900
> +static inline u32 ave_desc_read(struct net_device *ndev, enum desc_id id,
> + int entry, int offset)
Do not use inline functions in foo.c files, let the compiler decide what
to do.
Thank you.
^ permalink raw reply
* Re: [PATCH net-next v5 0/2] net: thunderx: add support for PTP clock
From: Philippe Ombredanne @ 2017-12-11 14:33 UTC (permalink / raw)
To: Aleksey Makarov
Cc: netdev, moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
LKML, Goutham, Sunil, Radoslaw Biernacki, Robert Richter,
David Daney, Richard Cochran
In-Reply-To: <20171211141435.2915-1-aleksey.makarov@cavium.com>
Aleksey,
On Mon, Dec 11, 2017 at 3:14 PM, Aleksey Makarov
<aleksey.makarov@cavium.com> wrote:
> This series adds support for IEEE 1588 Precision Time Protocol
> to Cavium ethernet driver.
>
> The first patch adds support for the Precision Time Protocol Clocks and
> Timestamping coprocessor (PTP) found on Cavium processors.
> It registers a new PTP clock in the PTP core and provides functions
> to use the counter in BGX, TNS, GTI, and NIC blocks.
>
> The second patch introduces support for the PTP protocol to the
> Cavium ThunderX ethernet driver.
>
> v5:
> - fix the file headers (add SPDX tags, remove advertisment) (Philippe Ombredanne)
Thank you.
Acked-by: Philippe Ombredanne <pombredanne@nexb.com>
--
Cordially
Philippe Ombredanne
^ 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