* Re: [PATCH 2/2] net: ipv6: ndisc: Fix warning when CONFIG_SYSCTL=n
From: David Miller @ 2013-11-18 19:49 UTC (permalink / raw)
To: festevam; +Cc: netdev, fabio.estevam
In-Reply-To: <1384570328-21666-2-git-send-email-festevam@gmail.com>
From: Fabio Estevam <festevam@gmail.com>
Date: Sat, 16 Nov 2013 00:52:08 -0200
> From: Fabio Estevam <fabio.estevam@freescale.com>
>
> When CONFIG_SYSCTL=n the following build warning happens:
>
> net/ipv6/ndisc.c:1730:1: warning: label 'out' defined but not used [-Wunused-label]
>
> The 'out' label is only used when CONFIG_SYSCTL=y, so move it inside the
> 'ifdef CONFIG_SYSCTL' block.
>
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
Applied.
^ permalink raw reply
* [PATCH] genetlink: rename shadowed variable
From: Johannes Berg @ 2013-11-18 19:54 UTC (permalink / raw)
To: netdev; +Cc: Johannes Berg
From: Johannes Berg <johannes.berg@intel.com>
Sparse pointed out that the new flags variable I had added
shadowed an existing one, rename the new one to avoid that,
making the code clearer.
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
net/netlink/genetlink.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/net/netlink/genetlink.c b/net/netlink/genetlink.c
index 9bdc581..7dbc4f7 100644
--- a/net/netlink/genetlink.c
+++ b/net/netlink/genetlink.c
@@ -680,21 +680,21 @@ static int ctrl_fill_info(struct genl_family *family, u32 portid, u32 seq,
for (i = 0; i < family->n_ops; i++) {
struct nlattr *nest;
const struct genl_ops *ops = &family->ops[i];
- u32 flags = ops->flags;
+ u32 op_flags = ops->flags;
if (ops->dumpit)
- flags |= GENL_CMD_CAP_DUMP;
+ op_flags |= GENL_CMD_CAP_DUMP;
if (ops->doit)
- flags |= GENL_CMD_CAP_DO;
+ op_flags |= GENL_CMD_CAP_DO;
if (ops->policy)
- flags |= GENL_CMD_CAP_HASPOL;
+ op_flags |= GENL_CMD_CAP_HASPOL;
nest = nla_nest_start(skb, i + 1);
if (nest == NULL)
goto nla_put_failure;
if (nla_put_u32(skb, CTRL_ATTR_OP_ID, ops->cmd) ||
- nla_put_u32(skb, CTRL_ATTR_OP_FLAGS, flags))
+ nla_put_u32(skb, CTRL_ATTR_OP_FLAGS, op_flags))
goto nla_put_failure;
nla_nest_end(skb, nest);
--
1.8.4.rc3
^ permalink raw reply related
* Re: [PATCH v4] inet: prevent leakage of uninitialized memory to user in recv syscalls
From: David Miller @ 2013-11-18 20:14 UTC (permalink / raw)
To: hannes; +Cc: eric.dumazet, mpb.mail, netdev
In-Reply-To: <20131118032045.GJ16541@order.stressinduktion.org>
From: Hannes Frederic Sowa <hannes@stressinduktion.org>
Date: Mon, 18 Nov 2013 04:20:45 +0100
> Only update *addr_len when we actually fill in sockaddr, otherwise we
> can return uninitialized memory from the stack to the caller in the
> recvfrom, recvmmsg and recvmsg syscalls. Drop the the (addr_len == NULL)
> checks because we only get called with a valid addr_len pointer either
> from sock_common_recvmsg or inet_recvmsg.
>
> If a blocking read waits on a socket which is concurrently shut down we
> now return zero and set msg_msgnamelen to 0.
>
> Reported-by: mpb <mpb.mail@gmail.com>
> Suggested-by: Eric Dumazet <eric.dumazet@gmail.com>
> Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
> ---
> This is the first round, fixing the callees of sock_common_recvmsg and
> inet_recvmsg. I guess there are some more problems of this kind with
> recvmsg called directly via proto_ops->recvmsg. I will have a look at
> them in the next days.
>
> We could actually leave out filling in msg_name if the user didn't request
> (src_addr == NULL in recvfrom). I'll put this on my TODO for net-next.
> Also it is possible to just update msg->msg_namelen directly here and simplify
> sock_common_recvmsg and inet_recvmsg, also for net-next.
This looks a lot better than your previous attempts, nice work.
Applied and queued up for -stable.
The handling of msg_name/msg_namelen in the ->recvmsg proto op is decidedly
non-trivial. Can I suggest that we at least add some documentation about
this semantic above "(*recvmsg)" in the definition of proto_ops in
include/linux/net.h?
Thanks.
^ permalink raw reply
* Re: [PATCH net-next 02/13] driver: net: remove unnecessary skb NULL check before calling dev_kfree_skb_irq
From: David Miller @ 2013-11-18 20:17 UTC (permalink / raw)
To: govindarajulu90
Cc: gregkh, linux-usb, linux-kernel, schwidefsky, linville,
linux-wireless, netdev, IvDoorn, sbhatewara, samuel, chas, roland,
isdn, jcliburn, benve, ssujith, jeffrey.t.kirsher,
jesse.brandeburg, shahed.shaikh, joe, apw
In-Reply-To: <alpine.LNX.2.03.1311190055150.10646@gmail.com>
From: Govindarajulu Varadarajan <govindarajulu90@gmail.com>
Date: Tue, 19 Nov 2013 00:56:48 +0530 (IST)
> Did you have a chance to look at this? Let me know how you want me to
> fix this.
I said clearly that I don't want this change to be made, because in many
cases the "= NULL" assignment in the driver is desirable to elide when it
isn't necessary because the value is already NULL. Therefore the NULL
check in dev_kfree_skb_irq() is basically coming for free.
Yes there are other cases, but it really doesn't matter enough to justify
your change.
Thanks.
^ permalink raw reply
* Re: [PATCH] genetlink: rename shadowed variable
From: David Miller @ 2013-11-18 20:34 UTC (permalink / raw)
To: johannes; +Cc: netdev, johannes.berg
In-Reply-To: <1384804498-7482-1-git-send-email-johannes@sipsolutions.net>
From: Johannes Berg <johannes@sipsolutions.net>
Date: Mon, 18 Nov 2013 20:54:58 +0100
> From: Johannes Berg <johannes.berg@intel.com>
>
> Sparse pointed out that the new flags variable I had added
> shadowed an existing one, rename the new one to avoid that,
> making the code clearer.
>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Applied, thanks Johannes.
^ permalink raw reply
* Re: [PATCHv2] ipv6: Fix inet6_init() cleanup order
From: David Miller @ 2013-11-18 20:39 UTC (permalink / raw)
To: hannes; +Cc: vyasevich, netdev, lorenzo, fabio.estevam
In-Reply-To: <20131116202646.GE16541@order.stressinduktion.org>
From: Hannes Frederic Sowa <hannes@stressinduktion.org>
Date: Sat, 16 Nov 2013 21:26:46 +0100
> On Sat, Nov 16, 2013 at 03:17:24PM -0500, Vlad Yasevich wrote:
>> Commit 6d0bfe22611602f36617bc7aa2ffa1bbb2f54c67
>> net: ipv6: Add IPv6 support to the ping socket
>>
>> introduced a change in the cleanup logic of inet6_init and
>> has a bug in that ipv6_packet_cleanup() may not be called.
>> Fix the cleanup ordering.
>>
>> CC: Hannes Frederic Sowa <hannes@stressinduktion.org>
>> CC: Lorenzo Colitti <lorenzo@google.com>
>> CC: Fabio Estevam <fabio.estevam@freescale.com>
>> Signed-off-by: Vlad Yasevich <vyasevich@gmail.com>
>
> Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
Looks great, thanks for sorting this out properly.
I'll queue this up for -stable as well.
Thanks again!
^ permalink raw reply
* Re: [PATCH] net: Do not include padding in TCP GRO checksum
From: Ben Hutchings @ 2013-11-18 20:44 UTC (permalink / raw)
To: Alexander Duyck; +Cc: davem, netdev, edumazet, herbert
In-Reply-To: <20131115011413.7090.9349.stgit@ahduyck-fpga.jf.intel.com>
On Thu, 2013-11-14 at 17:18 -0800, Alexander Duyck wrote:
> In some recent tests where I was generating invalid frames I found that
> the checksum was being treated as valid for certain frames that computed
> the checksum with padding included. On closer inspection I found the
> issue was that GRO was using the skb->len instead of the length recorded in
> the IP/IPv6 header to determine the number of bytes to checksum. As such
> padded frames that actually had invalid checksums generated by adding the
> padding to the checksum were being incorrectly tagged as valid.
>
> This change corrects that by using the tot_len from IPv4 headers and the
> payload_len from IPv6 headers to compute the total number of bytes to be
> included in the checksum.
>
> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> ---
>
> I haven't had a chance to test this much yet and I am not that familiar
> some of this GRO code so any review of this would be greatly appreciated.
> I will try to get this tested by end-of-day tomorrow to verify it resolves
> the issues I saw with invalid padded frames being marked as valid and
> doesn't introduce any new issues.
>
> net/ipv4/tcp_offload.c | 13 ++++++++-----
> net/ipv6/tcpv6_offload.c | 11 +++++++----
> 2 files changed, 15 insertions(+), 9 deletions(-)
>
> diff --git a/net/ipv4/tcp_offload.c b/net/ipv4/tcp_offload.c
> index a2b68a1..3dabb76 100644
> --- a/net/ipv4/tcp_offload.c
> +++ b/net/ipv4/tcp_offload.c
[...]
> @@ -288,11 +291,11 @@ flush:
> return NULL;
>
> case CHECKSUM_NONE:
> - wsum = csum_tcpudp_nofold(iph->saddr, iph->daddr,
> - skb_gro_len(skb), IPPROTO_TCP, 0);
> + wsum = csum_tcpudp_nofold(iph->saddr, iph->daddr, length,
> + IPPROTO_TCP, 0);
> sum = csum_fold(skb_checksum(skb,
> skb_gro_offset(skb),
> - skb_gro_len(skb),
> + length,
length may also be > skb_gro_len() which makes this unsafe.
> wsum));
> if (sum)
> goto flush;
> diff --git a/net/ipv6/tcpv6_offload.c b/net/ipv6/tcpv6_offload.c
> index c1097c7..53fc71d 100644
> --- a/net/ipv6/tcpv6_offload.c
> +++ b/net/ipv6/tcpv6_offload.c
[...]
> @@ -52,11 +56,10 @@ flush:
>
> case CHECKSUM_NONE:
> wsum = ~csum_unfold(csum_ipv6_magic(&iph->saddr, &iph->daddr,
> - skb_gro_len(skb),
> - IPPROTO_TCP, 0));
> + length, IPPROTO_TCP, 0));
> sum = csum_fold(skb_checksum(skb,
> skb_gro_offset(skb),
> - skb_gro_len(skb),
> + length,
> wsum));
> if (sum)
> goto flush;
Same problem here.
inet_gro_receive() and ipv6_gro_receive() already check whether the IP
length field is consistent with skb_gro_len(), and set
NAPI_GRO_CB(skb)->flush if it is not. I wonder whether it would make
sense to skip calling the transport-layer gro_receive function in that
case.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [PATCH net 0/4] bnx2x: Bug fixes patch series
From: David Miller @ 2013-11-18 20:46 UTC (permalink / raw)
To: yuvalmin; +Cc: netdev, ariele, dmitry
In-Reply-To: <1384671569-30105-1-git-send-email-yuvalmin@broadcom.com>
From: "Yuval Mintz" <yuvalmin@broadcom.com>
Date: Sun, 17 Nov 2013 08:59:25 +0200
> This series contains several fixes, relating either to SR-IOV flows
> or to critical sections protected by the rtnl lock.
>
> Please consider applying these patches to `net'.
Series applied, thanks Yuval.
^ permalink raw reply
* Re: [PATCH net RESEND] bonding: don't change to 802.3ad mode while ARP monitoring is running
From: David Miller @ 2013-11-18 20:48 UTC (permalink / raw)
To: dcbw; +Cc: dingtianhong, fubar, andy, nikolay, vfalico, netdev
In-Reply-To: <1384796682.4774.26.camel@dcbw.foobar.com>
From: Dan Williams <dcbw@redhat.com>
Date: Mon, 18 Nov 2013 11:44:42 -0600
> On Sat, 2013-11-16 at 14:30 +0800, Ding Tianhong wrote:
>> Because the ARP monitoring is not support for 802.3ad, but I still
>> could change the mode to 802.3ad from ab mode while ARP monitoring
>> is running, it is incorrect.
>>
>> So add a check for 802.3ad in bonding_store_mode to fix the problem,
>> and make a new macro BOND_NO_USES_ARP() to simplify the code.
>
> Instead of failing, couldn't the code stop ARP monitoring and allow the
> mode change? This is similar to setting miimon, which disables ARP
> monitoring, or setting ARP monitoring, which disables miimon.
>
> if (new_value && bond->params.arp_interval) {
> pr_info("%s: MII monitoring cannot be used with ARP monitoring. Disabling ARP monitoring...\n",
> bond->dev->name);
> bond->params.arp_interval = 0;
> if (bond->params.arp_validate)
> bond->params.arp_validate = BOND_ARP_VALIDATE_NONE;
> }
>
> Bond mode is the most important bond option, so it seems like it should
> override any of the other sub-options. I know the code doesn't do this
> now, but maybe instead of the patch you propose, it would be nicer to
> allow the mode change instead?
I agree with Dan, if other mode changes behave this way (by dropping the
incompatible feature) we should make 802.3ad do so as well at the very
least for consistency.
^ permalink raw reply
* Re: [PATCH 1/2] 8139too: Allow setting MTU larger than 1500
From: David Miller @ 2013-11-18 20:50 UTC (permalink / raw)
To: albeu; +Cc: linux-kernel, netdev, jgarzik
In-Reply-To: <1384693524-20599-1-git-send-email-albeu@free.fr>
Because we are in the middle of the merge window, we are only
accepting bug fixes into the 'net' GIT tree, the 'net-next' tree is
closed.
Please resubmit this feature patch when the 'net-next' is openned
again.
Thank you.
^ permalink raw reply
* Re: [PATCH v2] fix possible NULL pointer Oops in fib(6)_rule_suppress
From: David Miller @ 2013-11-18 20:51 UTC (permalink / raw)
To: stefan.tomanek; +Cc: netdev, daniel.golle, sergei.shtylyov
In-Reply-To: <20131117144501.GC23153@zirkel.wertarbyte.de>
From: Stefan Tomanek <stefan.tomanek@wertarbyte.de>
Date: Sun, 17 Nov 2013 15:45:01 +0100
> Reported-by: Daniel Golle <daniel.golle@gmail.com>
> Tested-by: Daniel Golle <daniel.golle@gmail.com>
> Signed-off-by: Stefan Tomanek <stefan.tomanek@wertarbyte.de>
Please us an appropriate subsystem prefix in your Subject lines, otherwise
the commit message header is less useful for others scanning the commit
log.
"inet: " would work well in this case.
^ permalink raw reply
* Re: [PATCH] ping: prevent NULL pointer dereference on write to msg_name
From: David Miller @ 2013-11-18 21:02 UTC (permalink / raw)
To: hannes; +Cc: netdev
In-Reply-To: <20131118060745.GL16541@order.stressinduktion.org>
From: Hannes Frederic Sowa <hannes@stressinduktion.org>
Date: Mon, 18 Nov 2013 07:07:45 +0100
> A plain read() on a socket does set msg->msg_name to NULL. So check for
> NULL pointer first.
>
> Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
> ---
>
> Addendum to "[v4] inet: prevent leakage of uninitialized memory to user in
> recv syscalls".
Applied and queued up for -stable, thanks Hannes.
Looks like ipv6 got this case right.
^ permalink raw reply
* Re: [PATCH net] bridge: flush br's address entry in fdb when remove the bridge dev
From: David Miller @ 2013-11-18 21:15 UTC (permalink / raw)
To: dingtianhong; +Cc: stephen, netdev, bridge
In-Reply-To: <5289CF44.50400@huawei.com>
From: Ding Tianhong <dingtianhong@huawei.com>
Date: Mon, 18 Nov 2013 16:26:44 +0800
> When the following commands are executed:
>
> brctl addbr br0
> ifconfig br0 hw ether <addr>
> rmmod bridge
>
> The calltrace will occur:
...
> The reason is that if the bridge dev's address is changed, the
> br_fdb_change_mac_address() will add new address in fdb, but when
> the bridge was removed, the address entry in the fdb did not free,
> the bridge_fdb_cache still has objects when destroy the cache, Fix
> this by flushing the bridge address entry when removing the bridge.
>
> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
This patch does not apply at all to the current 'net' tree, please
respin.
Thank you.
^ permalink raw reply
* Re: [PATCH net] neigh: Force garbage collection if an entry is deleted administratively
From: David Miller @ 2013-11-18 21:21 UTC (permalink / raw)
To: steffen.klassert; +Cc: yoshfuji, netdev
In-Reply-To: <20131118100843.GY31491@secunet.com>
From: Steffen Klassert <steffen.klassert@secunet.com>
Date: Mon, 18 Nov 2013 11:08:43 +0100
> Subject: [PATCH RFC] neigh: Fix garbage collection if the cached entries are
> below the threshold
>
> Since git commit 2724680 ("neigh: Keep neighbour cache entries if number
> of them is small enough."), we keep all neighbour cache entries if the
> number is below a threshold. But if we now delete an entry administratively
> and then try to replace this by a permanent one, we get -EEXIST because the
> old entry ist still in the table (in NUD_FAILED state).
>
> So remove the threshold check in neigh_periodic_work() and schedule the
> gc_work only when needed, i.e. if gc_thresh1 is reached or if there is
> an administrative change. We reschedule gc_work either if the number of
> cache entries is still above gc_thresh1 or if there are invalid entries
> with "refcnt != 1" cached.
>
> Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
I think the main issue is that after this patch, the problem is really
still there.
Let's say some device holds onto the neigh for a long time, then during
this time an administrative replacement will still get that -EEXIST
failure.
My conclusion is that the management of the state is the problem.
Specifically, if we invalidate an entry then we should remove it's
visisbility. This means the table should operate by unhashing the
entry unconditionally during such operations.
If some stray references exist, that's fine, the entity holding the
reference will perform the final neigh cleanup at release time.
Does this make sense to you?
^ permalink raw reply
* Re: [PATCH 1/2] vhost: remove the dead branch
From: David Miller @ 2013-11-18 21:22 UTC (permalink / raw)
To: zwu.kernel; +Cc: netdev, mst, rusty, linux-kernel, wuzhy
In-Reply-To: <1384780716-25433-1-git-send-email-zwu.kernel@gmail.com>
These two patches are cleanups, please resubmit them after the merge
window closes and the net-next tree opens back up.
Thank you.
^ permalink raw reply
* Re: [PATCH] net, virtio_net: replace the magic value
From: David Miller @ 2013-11-18 21:23 UTC (permalink / raw)
To: zwu.kernel; +Cc: netdev, mst, rusty, linux-kernel, wuzhy
In-Reply-To: <1384780767-25507-1-git-send-email-zwu.kernel@gmail.com>
From: Zhi Yong Wu <zwu.kernel@gmail.com>
Date: Mon, 18 Nov 2013 21:19:27 +0800
> From: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
>
> It is more appropriate to use # of queue pairs currently used by
> the driver instead of a magic value.
>
> Signed-off-by: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
> Acked-by: Michael S. Tsirkin <mst@redhat.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH net 0/2] be2net: Patch series
From: David Miller @ 2013-11-18 21:25 UTC (permalink / raw)
To: ajit.khaparde; +Cc: netdev
In-Reply-To: <20131118164412.GA3016@emulex.com>
From: Ajit Khaparde <ajit.khaparde@emulex.com>
Date: Mon, 18 Nov 2013 10:44:12 -0600
> Please apply these two patches.
>
> Ajit Khaparde (2):
> be2net: Fix unconditional enabling of Rx interface options
> be2net: Delete secondary unicast MAC addresses during be_close
Both applied, thank you.
^ permalink raw reply
* [PATCH] libertas sdio: claim device before calling sdio_disable_func()
From: Alexey Khoroshilov @ 2013-11-18 21:45 UTC (permalink / raw)
To: John W. Linville, Dan Williams
Cc: Alexey Khoroshilov, libertas-dev, linux-wireless, netdev,
linux-kernel, ldv-project
There is a failure path in if_sdio_power_on(), where
sdio_disable_func() is called without claiming the device.
Found by Linux Driver Verification project (linuxtesting.org).
Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
---
drivers/net/wireless/libertas/if_sdio.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/libertas/if_sdio.c b/drivers/net/wireless/libertas/if_sdio.c
index 991238afd1b6..19c4477eb4c6 100644
--- a/drivers/net/wireless/libertas/if_sdio.c
+++ b/drivers/net/wireless/libertas/if_sdio.c
@@ -907,8 +907,8 @@ static int if_sdio_power_on(struct if_sdio_card *card)
sdio_release_host(func);
ret = if_sdio_prog_firmware(card);
if (ret) {
- sdio_disable_func(func);
- return ret;
+ sdio_claim_host(func);
+ goto disable;
}
return 0;
--
1.8.1.2
^ permalink raw reply related
* RE: [PATCH net v3] vti: fix spd lookup: match plaintext pkt, not ipsec pkt
From: Saurabh Mohan @ 2013-11-18 21:38 UTC (permalink / raw)
To: Christophe Gouault, Steffen Klassert
Cc: David S. Miller, Herbert Xu, netdev@vger.kernel.org,
Sergei Shtylyov, Eric Dumazet
In-Reply-To: <527B8DC5.6080702@6wind.com>
> -----Original Message-----
> From: Christophe Gouault [mailto:christophe.gouault@6wind.com]
> Sent: Thursday, November 07, 2013 4:56 AM
> To: Steffen Klassert
> Cc: David S. Miller; Herbert Xu; netdev@vger.kernel.org; Saurabh Mohan;
> Sergei Shtylyov; Eric Dumazet
> Subject: Re: [PATCH net v3] vti: fix spd lookup: match plaintext pkt, not ipsec
> pkt
>
> Hello Steffen,
>
> I am also interested in knowing Saurabh's intentions regarding the
> behavior of policies bound to vti interfaces.
>
The semantics is to match the policy "src 0.0.0.0/0 dst 0.0.0.0/0 proto any"
That is the only policy that VTI should use. The mark is needed to
distinguish and limit the policy to a specific vti tunnel interface only.
There is no other policy that may be applied to a vti interface.
The fact that traffic is going over the tunnel interface implies that it
must be encrypted/decrypted. Applying the above policy is a way
to achieve that.
> However, please note that setting a policy with a wildcard selector
> works in both cases (before or after this patch), so a common test case
> can be defined.
>
> Actually the *previous* patch on vti (7263a5187f9e vti: get rid of nf
> mark rule in prerouting) introduced significant changes, and implies
> behaviors dependant on the kernel version, but it seemed to meet
> Saurabh's agreement, as the following thread witnesses:
>
> http://www.spinics.net/lists/netdev/msg253134.html
>
Getting rid of the pre-routing mark, which had to be done outside of
the vti tunnel code was prone to misconfiguration.
Though it is unfortunate that it creates a kernel version dependency.
^ permalink raw reply
* Re: [PATCH v4] inet: prevent leakage of uninitialized memory to user in recv syscalls
From: Hannes Frederic Sowa @ 2013-11-18 22:14 UTC (permalink / raw)
To: David Miller; +Cc: eric.dumazet, mpb.mail, netdev
In-Reply-To: <20131118.151430.75221704090778394.davem@davemloft.net>
On Mon, Nov 18, 2013 at 08:13:50AM -0800, Eric Dumazet wrote:
> This definitely looks much better, as this will remove lot of crap,
> thanks !
>
> unix_dgram_recvmsg() and unix_stream_recvmsg() for example clear
> msg_namelen
Most recvfrom calls are fine, but there is appletalk, tun and even macvtap
and a lot other recvmsg calls I still have to check.
On Mon, Nov 18, 2013 at 03:14:30PM -0500, David Miller wrote:
> This looks a lot better than your previous attempts, nice work.
>
> Applied and queued up for -stable.
>
> The handling of msg_name/msg_namelen in the ->recvmsg proto op is decidedly
> non-trivial. Can I suggest that we at least add some documentation about
> this semantic above "(*recvmsg)" in the definition of proto_ops in
> include/linux/net.h?
Thanks!
I even want to come up with a better scheme for that. I do think we
can pass msg_namelen == 0 initially and let the recvmsg handlers update
it. So we have a safe fallback. The possible uninitialized memory leaks
can be pretty big, so I would like to see new recvmsg callers not return
anything if they don't care about msg_namelen. This should get noticed
during testing.
I'll add documentation in future patches.
^ permalink raw reply
* Re: [PATCH] ping: prevent NULL pointer dereference on write to msg_name
From: Hannes Frederic Sowa @ 2013-11-18 22:15 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20131118.160238.2288108703856396394.davem@davemloft.net>
On Mon, Nov 18, 2013 at 04:02:38PM -0500, David Miller wrote:
> From: Hannes Frederic Sowa <hannes@stressinduktion.org>
> Date: Mon, 18 Nov 2013 07:07:45 +0100
>
> > A plain read() on a socket does set msg->msg_name to NULL. So check for
> > NULL pointer first.
> >
> > Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
> > ---
> >
> > Addendum to "[v4] inet: prevent leakage of uninitialized memory to user in
> > recv syscalls".
>
> Applied and queued up for -stable, thanks Hannes.
>
> Looks like ipv6 got this case right.
No, IPv6 reuses the IPv4 recvmsg handler. The patch fixes that as well.
Thanks,
Hannes
^ permalink raw reply
* Re: MLD maturity in kernel version 2.6.32.60
From: Hannes Frederic Sowa @ 2013-11-18 22:18 UTC (permalink / raw)
To: Simon Schneider; +Cc: netdev
In-Reply-To: <trinity-825e48d2-0bab-4020-84f0-77800e55f25a-1384777048114@3capp-gmx-bs16>
On Mon, Nov 18, 2013 at 01:17:28PM +0100, Simon Schneider wrote:
> in a development project, we started evaluating MLD functionality.
>
> Our development is based on kernel version 2.6.32.60.
>
> We noticed some pretty basic issues:
>
> - With force_mld_version set to 0 (we expect "MLDv2 with MLDv1 fallback"), we saw only MLDv2 messages, the fallback to MLDv1 didn't seem to work, when we sent MLDv1 messages to the unit.
> - With force_mld_version set to 1 (we expect "MLDv1 only mode"), we also saw MLDv2 messages being generated.
>
> Can someone give a general statement about the MLD implementation maturity in kernel 2.6.32.60? Are there known bugs that would explain the above observations?
>
> We need to see whether it makes sense to put further effort in investigating/fixing any MLD related issue we see.
>
> If this is the wrong list to ask this kind of question, please point me to the correct one.
It would be great if you could share test results with newer kernel with us.
Last time I checked we did correctly fallback to MDLv1 mode but it has been a
while.
If you come up with a specific patch from Daniel's series which fixes the
issues you are seeing, we can think about including this to stable.
There are still some issues in mld: we are currently a bit too noisy for my
taste (e.g. on re-enabling interfaces). This is a big problem for large
wireless networks e.g.
Thanks,
Hannes
^ permalink raw reply
* Re: oops in pskb_expand_head - 3.11.6
From: Hannes Frederic Sowa @ 2013-11-18 22:25 UTC (permalink / raw)
To: Jiri Pirko; +Cc: Michele Baldessari, netdev
In-Reply-To: <20131118092436.GA3743@minipsycho.orion>
On Mon, Nov 18, 2013 at 10:24:36AM +0100, Jiri Pirko wrote:
> Mon, Nov 18, 2013 at 10:19:22AM CET, michele@acksyn.org wrote:
> >Hi Hannes,
> >
> >On Sun, Nov 17, 2013 at 08:18:10PM +0100, Hannes Frederic Sowa wrote:
> >> On Sun, Nov 17, 2013 at 11:11:50AM +0000, Michele Baldessari wrote:
> >> > Hi Hannes,
> >> >
> >> > On Sun, Nov 17, 2013 at 12:42:23AM +0100, Hannes Frederic Sowa wrote:
> >> > > On Sat, Nov 16, 2013 at 11:16:15PM +0000, Michele Baldessari wrote:
> >> > > > Two oops like the following were reported in Fedora 19 - kernel 3.11.6:
> >> > > > https://bugzilla.redhat.com/show_bug.cgi?id=1015905
> >> > >
> >> > > I have not followed that issue that closely, but could you try linus tree
> >> > > or net-next?
> >> > >
> >> > > Maybe those two patches improve the situation:
> >> > > https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=9037c3579a277f3a23ba476664629fda8c35f7c4
> >> > > https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=6aafeef03b9d9ecf255f3a80ed85ee070260e1ae
> >> > >
> >> > indeed a current kernel seems to fix the crash (according to the
> >> > reporters). I'll see if I manage to bisect exactly.
> >>
> >> Great, if you confirm this we can ask David if he adds the commits to the
> >> -stable queue.
> >
> >I can confirm that 6aafeef03b9d9ecf255f3a80ed85ee070260e1ae "netfilter:
> >push reasm skb through instead of original frag skbs" fixes the oops
> >mentioned in this thread. I've applied it to a 3.11.8 kernel and it
> >fixed the crash. I had to slightly tweak it as it does not apply 100%
> >cleanly due to 795aa6ef6a1aba99050735eadd0c2341b789b53b " netfilter:
> >pass hook ops to hookfn".
> >
> >So definitely -stable material.
> >
> >Not sure if you also want to add 9037c3579a277f3a23ba476664629fda8c35f7c4
> >"ip6_output: fragment outgoing reassembled skb properly". It did not
> >have any impact in this particular scenario.
>
> It goes hand in hand with 6aafeef03b9d9ecf255f3a80ed85ee070260e1ae.
> Without 9037c3579a277f3a23ba476664629fda8c35f7c4 skbs that reasm len is
> < MTU would not get fragmented which is not desired.
Jiri, David, do you see problems adding this to stable? We got a lot of
reports because of exactly those kind of crashes in the past.
Greetings,
Hannes
^ permalink raw reply
* Re: [PATCH net RESEND] bonding: don't change to 802.3ad mode while ARP monitoring is running
From: Dan Williams @ 2013-11-18 22:50 UTC (permalink / raw)
To: David Miller; +Cc: dingtianhong, fubar, andy, nikolay, vfalico, netdev
In-Reply-To: <20131118.154838.776189725378131169.davem@davemloft.net>
On Mon, 2013-11-18 at 15:48 -0500, David Miller wrote:
> From: Dan Williams <dcbw@redhat.com>
> Date: Mon, 18 Nov 2013 11:44:42 -0600
>
> > On Sat, 2013-11-16 at 14:30 +0800, Ding Tianhong wrote:
> >> Because the ARP monitoring is not support for 802.3ad, but I still
> >> could change the mode to 802.3ad from ab mode while ARP monitoring
> >> is running, it is incorrect.
> >>
> >> So add a check for 802.3ad in bonding_store_mode to fix the problem,
> >> and make a new macro BOND_NO_USES_ARP() to simplify the code.
> >
> > Instead of failing, couldn't the code stop ARP monitoring and allow the
> > mode change? This is similar to setting miimon, which disables ARP
> > monitoring, or setting ARP monitoring, which disables miimon.
> >
> > if (new_value && bond->params.arp_interval) {
> > pr_info("%s: MII monitoring cannot be used with ARP monitoring. Disabling ARP monitoring...\n",
> > bond->dev->name);
> > bond->params.arp_interval = 0;
> > if (bond->params.arp_validate)
> > bond->params.arp_validate = BOND_ARP_VALIDATE_NONE;
> > }
> >
> > Bond mode is the most important bond option, so it seems like it should
> > override any of the other sub-options. I know the code doesn't do this
> > now, but maybe instead of the patch you propose, it would be nicer to
> > allow the mode change instead?
>
> I agree with Dan, if other mode changes behave this way (by dropping the
> incompatible feature) we should make 802.3ad do so as well at the very
> least for consistency.
Currently ALB and TLB modes will fail if arp_interval > 0, so Ding's
patch is technically correct.
Instead, I'm proposing that 'mode' trumps all, and if the user changes
the mode, conflicting values should be cleared or reset. Otherwise
userspace has to duplicate a lot of kernel logic/validation. For
example:
1) set mode to ROUNDROBIN
2) set arp_interval
3) set mode to ALB or TLB
4) FAIL - incompatible with arp_interval
5) ok, set arp_interval to zero
6) set mode to ALB or TLB
7) SUCCESS
Wouldn't it be nice if the kernel handled clearing arp_interval for us,
since it knows that arp_interval is incompatible with ALB/TLB...
Could be done separately. I have no objection to Ding's patch other
than "life could be even better".
Dan
^ permalink raw reply
* Re: net/usb/ax88179_178a driver broken in linux-3.12
From: Mark Lord @ 2013-11-18 22:52 UTC (permalink / raw)
To: David Laight, Eric Dumazet, Ming Lei, davem, netdev
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6026B742F@saturn3.aculab.com>
On 13-11-18 08:32 AM, David Laight wrote:
>> From: David Laight
>>> On 13-11-17 01:56 PM, Mark Lord wrote:
>>>> On 13-11-17 01:35 PM, Mark Lord wrote:
>>>>> The USB3 network adapter locks up consistently for me here in 3.12,
>>>>> but was working without issues in 3.11.x
>>
>> The xhci driver is well broken in 3.12.
>
> To correct myself...
>
> The xhci driver has never correctly support scatter-gather requests.
> In 3.12 code was added to usbnet to generate SG transmits, and to the
> ax88179_178a driver to use them.
> TCP segmentation offload was then enabled - with does generate
> SG transfers.
>
> SG transfers for 'disks' almost certainly work because the
> fragment boundaries are 'adequately aligned'.
>
> David
>
Well, that's all very nice and whatnot,
except the ax88179_178a driver still does not work in linux-3.12,
whereas it works fine in all earlier kernels.
That's a regression.
And a simple revert (earlier in this thread) fixes it.
So.. let's revert it for now, until a proper xhci compatible patch is produced.
--
Mark Lord
Real-Time Remedies Inc.
mlord@pobox.com
^ 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