* Re: [PATCH net-next v2 0/7] net: dsa: mv88e6xxx: ops cosmetics
From: David Miller @ 2017-06-09 16:11 UTC (permalink / raw)
To: vivien.didelot; +Cc: netdev, linux-kernel, kernel, f.fainelli, andrew
In-Reply-To: <20170608223414.594-1-vivien.didelot@savoirfairelinux.com>
From: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
Date: Thu, 8 Jun 2017 18:34:07 -0400
> This patchset brings no functional changes. It is a first step in a
> bigger cosmetics change to the driver. It simplifies print messages and
> polishes data types and chip operations.
>
> The next patchs will only prefix and document the port registers macros.
>
> Changes in v2:
> - KISS and simply use dev_* since chip->ds may not be initialized
> - add reviewers tags
Series applied, thank you.
^ permalink raw reply
* Re: [PATCH net-next 0/6] netvsc: small cleanups
From: David Miller @ 2017-06-09 16:16 UTC (permalink / raw)
To: stephen; +Cc: kys, haiyangz, sthemmin, devel, netdev
In-Reply-To: <20170608232123.28366-1-sthemmin@microsoft.com>
From: Stephen Hemminger <stephen@networkplumber.org>
Date: Thu, 8 Jun 2017 16:21:17 -0700
> These are all small optimizations found during development of later features.
Applied with the spelling error pointed out by Sergei fixed.
^ permalink raw reply
* Re: [PATCH 06/11] net: ehea: convert to use DRIVER_ATTR_RO
From: David Miller @ 2017-06-09 16:20 UTC (permalink / raw)
To: gregkh; +Cc: linux-kernel, dougmill, netdev
In-Reply-To: <20170609090314.13991-6-gregkh@linuxfoundation.org>
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Date: Fri, 9 Jun 2017 11:03:09 +0200
> We are trying to get rid of DRIVER_ATTR(), and the ehea driver's
> attribute can be trivially changed to use DRIVER_ATTR_RO().
>
> Cc: Douglas Miller <dougmill@linux.vnet.ibm.com>
> Cc: <netdev@vger.kernel.org>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Acked-by: David S. Miller <davem@davemloft.net>
^ permalink raw reply
* Re: [PATCH 05/11] net: caif: convert to use DRIVER_ATTR_RO
From: David Miller @ 2017-06-09 16:20 UTC (permalink / raw)
To: gregkh; +Cc: linux-kernel, dmitry.tarnyagin, netdev
In-Reply-To: <20170609090314.13991-5-gregkh@linuxfoundation.org>
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Date: Fri, 9 Jun 2017 11:03:08 +0200
> We are trying to get rid of DRIVER_ATTR(), and the caif driver's
> attributes can be trivially changed to use DRIVER_ATTR_RO().
>
> Cc: Dmitry Tarnyagin <dmitry.tarnyagin@lockless.no>
> Cc: <netdev@vger.kernel.org>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Acked-by: David S. Miller <davem@davemloft.net>
^ permalink raw reply
* Re: [PATCH v2 7/8] net: mvmdio: add xmdio support
From: Antoine Tenart @ 2017-06-09 16:22 UTC (permalink / raw)
To: Andrew Lunn
Cc: Antoine Tenart, Florian Fainelli, davem, jason, gregory.clement,
sebastian.hesselbarth, thomas.petazzoni, mw, linux, netdev,
linux-arm-kernel
In-Reply-To: <20170609150340.GH20756@lunn.ch>
[-- Attachment #1: Type: text/plain, Size: 649 bytes --]
On Fri, Jun 09, 2017 at 05:03:40PM +0200, Andrew Lunn wrote:
> > There are two busses, one generating c22 transactions and one generating
> > c45 transactions. Each bus has its own MDC/MDIO pins.
>
> O.K. That is what i wanted to know. So we want two completely separate
> device tree bindings, busses registered with Linux, etc.
>
> Thanks for clarification.
So in the end I need one change in v3: to bind the xSMI usage to
marvell,xmdio and the SMI one to marvell,orion-mdio. (Plus the GENMASK
and offset comments).
Antoine
--
Antoine Ténart, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH v2 net-next] Ipvlan should return an error when an address is already in use.
From: David Miller @ 2017-06-09 16:26 UTC (permalink / raw)
To: kjlx; +Cc: maheshb, netdev
In-Reply-To: <20170608201214.GB2553@templeofstupid.com>
From: Krister Johansen <kjlx@templeofstupid.com>
Date: Thu, 8 Jun 2017 13:12:14 -0700
> The ipvlan code already knows how to detect when a duplicate address is
> about to be assigned to an ipvlan device. However, that failure is not
> propogated outward and leads to a silent failure.
>
> Introduce a validation step at ip address creation time and allow device
> drivers to register to validate the incoming ip addresses. The ipvlan
> code is the first consumer. If it detects an address in use, we can
> return an error to the user before beginning to commit the new ifa in
> the networking code.
>
> This can be especially useful if it is necessary to provision many
> ipvlans in containers. The provisioning software (or operator) can use
> this to detect situations where an ip address is unexpectedly in use.
>
> Signed-off-by: Krister Johansen <kjlx@templeofstupid.com>
Ok, applied, thank you.
^ permalink raw reply
* Re: [PATCH net] Fix an intermittent pr_emerg warning about lo becoming free.
From: David Miller @ 2017-06-09 16:28 UTC (permalink / raw)
To: kjlx; +Cc: eric.dumazet, netdev
In-Reply-To: <20170608201238.GC2553@templeofstupid.com>
From: Krister Johansen <kjlx@templeofstupid.com>
Date: Thu, 8 Jun 2017 13:12:38 -0700
> It looks like this:
>
> Message from syslogd@flamingo at Apr 26 00:45:00 ...
> kernel:unregister_netdevice: waiting for lo to become free. Usage count = 4
>
> They seem to coincide with net namespace teardown.
>
> The message is emitted by netdev_wait_allrefs().
>
> Forced a kdump in netdev_run_todo, but found that the refcount on the lo
> device was already 0 at the time we got to the panic.
>
> Used bcc to check the blocking in netdev_run_todo. The only places
> where we're off cpu there are in the rcu_barrier() and msleep() calls.
> That behavior is expected. The msleep time coincides with the amount of
> time we spend waiting for the refcount to reach zero; the rcu_barrier()
> wait times are not excessive.
>
> After looking through the list of callbacks that the netdevice notifiers
> invoke in this path, it appears that the dst_dev_event is the most
> interesting. The dst_ifdown path places a hold on the loopback_dev as
> part of releasing the dev associated with the original dst cache entry.
> Most of our notifier callbacks are straight-forward, but this one a)
> looks complex, and b) places a hold on the network interface in
> question.
>
> I constructed a new bcc script that watches various events in the
> liftime of a dst cache entry. Note that dst_ifdown will take a hold on
> the loopback device until the invalidated dst entry gets freed.
...
> The way this works is that if there's still a reference on the dst entry
> at the time we try to free it, it gets placed in the gc list by
> __dst_free and the dst_destroy() call is invoked by the gc task once the
> refcount is 0. If the gc task processes a 10th or less of its entries
> on a single pass, it inreases the amount of time it waits between gc
> intervals.
>
> Looking at the gc_task intervals, they started at 663ms when we invoked
> __dst_free(). After that, they increased to 1663, 3136, 5567, 8191,
> 10751, and 14848. The release that set the refcnt to 0 on our dst entry
> occurred after the gc_task was enqueued for 14 second interval so we had
> to wait longer than the warning time in wait_allrefs in order for the
> dst entry to get free'd and the hold on 'lo' to be released.
>
> A simple solution to this problem is to have dst_dev_event() reset the
> gc timer, which causes us to process this list shortly after the
> gc_mutex is relased when dst_dev_event() completes.
>
> Signed-off-by: Krister Johansen <kjlx@templeofstupid.com>
Yeah this is one of the more unsatisfying areas of dst and device handling
in the tree, thanks for working on this.
Applied and queued up for -stable, thanks again.
^ permalink raw reply
* Re: [PATCH] sh_eth: add support to change MTU
From: Sergei Shtylyov @ 2017-06-09 16:31 UTC (permalink / raw)
To: Niklas Söderlund; +Cc: netdev, linux-renesas-soc
In-Reply-To: <20170609153034.8777-1-niklas.soderlund+renesas@ragnatech.se>
Hello!
On 06/09/2017 06:30 PM, Niklas Söderlund wrote:
> The hardware supports the MTU to be changed and the driver it self is
> somewhat prepared to support this. This patch hooks up the callbacks to
> be able to change the MTU from user-space.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
[...]
One more thing off my back, thanks! :-)
I'm OK with this patch in principle (but have several nits):
Acked-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
> index f68c4db656eda846..da41eda7bfada6b9 100644
> --- a/drivers/net/ethernet/renesas/sh_eth.c
> +++ b/drivers/net/ethernet/renesas/sh_eth.c
[...]
> @@ -3171,6 +3184,13 @@ static int sh_eth_drv_probe(struct platform_device *pdev)
> }
> sh_eth_set_default_cpu_data(mdp->cd);
>
> + /* Datasheet states max MTU should be 2048 but due to the
User's manual. :-)
Somehow I thought it supports jumbo frames but the manual doesn't confirm
that... ah, that's EtherAVB! :-)
> + * aliment calculations in sh_eth_ring_init() the practical
Alignment.
> + * MTU is a bit less. Maybe this can be optimized some more.
Undoubtedly... :-)
[...]
MBR, Sergei
^ permalink raw reply
* Re: [PATCH net-next v2 3/3] udp: try to avoid 2 cache miss on dequeue
From: David Miller @ 2017-06-09 16:33 UTC (permalink / raw)
To: pabeni; +Cc: netdev, edumazet
In-Reply-To: <1497023069.3416.18.camel@redhat.com>
From: Paolo Abeni <pabeni@redhat.com>
Date: Fri, 09 Jun 2017 17:44:29 +0200
> I'll re-submit v3 unchanged, if there are no objections.
No objections from me.
^ permalink raw reply
* Re: [PATCH 1/2(net.git)] stmmac: fix ptp header for GMAC3 hw timestamp
From: David Miller @ 2017-06-09 16:39 UTC (permalink / raw)
To: Mario_Molitor; +Cc: peppe.cavallaro, alexandre.torgue, netdev, linux-kernel
In-Reply-To: <trinity-0deb7b33-2ac6-4e6d-9e43-c5feb0c09e14-1496957473112@3capp-webde-bs49>
From: "Mario Molitor" <Mario_Molitor@web.de>
Date: Thu, 8 Jun 2017 23:31:13 +0200
> From ce9c334037fce37ccd715124cda57d1fd6d8cfe8 Mon Sep 17 00:00:00 2001
> From: Mario Molitor <mario_molitor@web.de>
> Date: Thu, 8 Jun 2017 22:41:02 +0200
> Subject: [PATCH 1/2] stmmac: fix ptp header for GMAC3 hw timestamp
>
> According the CYCLON V documention only the bit 16 of snaptypesel should
> set.
> (more information see Table 17-20 (cv_5v4.pdf) :
> Timestamp Snapshot Dependency on Register Bits)
>
> Fixes: d2042052a0aa ("stmmac: update the PTP header file")
> Signed-off-by: Mario Molitor <mario_molitor@web.de>
Applied.
^ permalink raw reply
* Re: [PATCH 2/2(net.git)] stmmac: fix for hw timestamp of GMAC3 unit
From: David Miller @ 2017-06-09 16:39 UTC (permalink / raw)
To: Mario_Molitor; +Cc: peppe.cavallaro, alexandre.torgue, netdev, linux-kernel
In-Reply-To: <trinity-8bed4ae0-8914-46c8-b515-dda572c13b88-1496957702157@3capp-webde-bs49>
From: "Mario Molitor" <Mario_Molitor@web.de>
Date: Thu, 8 Jun 2017 23:35:02 +0200
> From d5c520880a5f6b470cb150b9aae67341089b9395 Mon Sep 17 00:00:00 2001
> From: Mario Molitor <mario_molitor@web.de>
> Date: Thu, 8 Jun 2017 23:03:09 +0200
> Subject: [PATCH 2/2] stmmac: fix for hw timestamp of GMAC3 unit
>
> 1.) Bugfix of function stmmac_get_tx_hwtstamp.
> Corrected the tx timestamp available check (same as 4.8 and older)
> Change printout from info syslevel to debug.
>
> 2.) Bugfix of function stmmac_get_rx_hwtstamp.
> Corrected the rx timestamp available check (same as 4.8 and older)
> Change printout from info syslevel to debug.
>
> Fixes: ba1ffd74df74 ("stmmac: fix PTP support for GMAC4")
> Signed-off-by: Mario Molitor <mario_molitor@web.de>
Applied.
^ permalink raw reply
* Re: [net-bluetooth] question about potential null pointer dereference
From: Marcel Holtmann @ 2017-06-09 16:48 UTC (permalink / raw)
To: Gustavo A. R. Silva
Cc: Gustavo F. Padovan, Johan Hedberg, David S. Miller,
open list:BLUETOOTH DRIVERS, netdev, linux-kernel
In-Reply-To: <20170530223629.Horde.l1icLUD-56lD-xqEDjo6CDT@gator4166.hostgator.com>
Hi Gustavo,
>>>> While looking into Coverity ID 1357456 I ran into the following piece of code at net/bluetooth/smp.c:166
>>>>
>>>> 166/* The following functions map to the LE SC SMP crypto functions
>>>> 167 * AES-CMAC, f4, f5, f6, g2 and h6.
>>>> 168 */
>>>> 169
>>>> 170static int aes_cmac(struct crypto_shash *tfm, const u8 k[16], const u8 *m,
>>>> 171 size_t len, u8 mac[16])
>>>> 172{
>>>> 173 uint8_t tmp[16], mac_msb[16], msg_msb[CMAC_MSG_MAX];
>>>> 174 SHASH_DESC_ON_STACK(desc, tfm);
>>>> 175 int err;
>>>> 176
>>>> 177 if (len > CMAC_MSG_MAX)
>>>> 178 return -EFBIG;
>>>> 179
>>>> 180 if (!tfm) {
>>>> 181 BT_ERR("tfm %p", tfm);
>>>> 182 return -EINVAL;
>>>> 183 }
>>>> 184
>
> BTW, what do you think about removing the IF block above?
what do you mean by this?
>>>> 185 desc->tfm = tfm;
>>>> 186 desc->flags = 0;
>>>> 187
>>>> 188 /* Swap key and message from LSB to MSB */
>>>> 189 swap_buf(k, tmp, 16);
>>>> 190 swap_buf(m, msg_msb, len);
>>>> 191
>>>> 192 SMP_DBG("msg (len %zu) %*phN", len, (int) len, m);
>>>> 193 SMP_DBG("key %16phN", k);
>>>> 194
>>>> 195 err = crypto_shash_setkey(tfm, tmp, 16);
>>>> 196 if (err) {
>>>> 197 BT_ERR("cipher setkey failed: %d", err);
>>>> 198 return err;
>>>> 199 }
>>>> 200
>>>> 201 err = crypto_shash_digest(desc, msg_msb, len, mac_msb);
>>>> 202 shash_desc_zero(desc);
>>>> 203 if (err) {
>>>> 204 BT_ERR("Hash computation error %d", err);
>>>> 205 return err;
>>>> 206 }
>>>> 207
>>>> 208 swap_buf(mac_msb, mac, 16);
>>>> 209
>>>> 210 SMP_DBG("mac %16phN", mac);
>>>> 211
>>>> 212 return 0;
>>>> 213}
>>>>
>>>> The issue here is that line 180 implies that pointer tfm might be NULL. If this is the case, there is a potential NULL pointer dereference at line 174 once pointer tfm is indirectly dereferenced inside macro SHASH_DESC_ON_STACK().
>>>>
>>>> My question is if there is any chance that pointer tfm maybe be NULL when calling macro SHASH_DESC_ON_STACK()?
>>>
>>> I think the part you are after is this:
>>>
>>> smp->tfm_cmac = crypto_alloc_shash("cmac(aes)", 0, 0);
>>> if (IS_ERR(smp->tfm_cmac)) {
>>> BT_ERR("Unable to create CMAC crypto context");
>>> crypto_free_cipher(smp->tfm_aes);
>>> kzfree(smp);
>>> return NULL;
>>> }
>>>
>>
>> Yeah, this makes it all clear.
>>
>>> So the tfm_cmac is part of the smp structure. However if there is no cipher, we destroy the smp structure and essentially run without SMP support. So it can not really be called anyway.
>>>
>>
>> What I take from this is that as a general rule, I should first try to identify whether the code I'm debugging is reachable or not, depending on the specific structures and variables I'm interested in.
>>
>>> Maybe commenting this might be a good idea.
>>>
>>
>> Yep, it wouldn't hurt.
Patches are welcome :)
Regards
Marcel
^ permalink raw reply
* Re: [PATCH net-next v2] geneve: add missing rx stats accounting
From: David Miller @ 2017-06-09 16:50 UTC (permalink / raw)
To: girish.moodalbail; +Cc: netdev, pshelar
In-Reply-To: <1496966868-6032-1-git-send-email-girish.moodalbail@oracle.com>
From: Girish Moodalbail <girish.moodalbail@oracle.com>
Date: Thu, 8 Jun 2017 17:07:48 -0700
> There are few places on the receive path where packet drops and packet
> errors were not accounted for. This patch fixes that issue.
>
> Signed-off-by: Girish Moodalbail <girish.moodalbail@oracle.com>
Applied, thank you.
^ permalink raw reply
* Re: [PATCH net-next] liquidio: disallow enabling firmware debug from a VF
From: David Miller @ 2017-06-09 16:51 UTC (permalink / raw)
To: felix.manlunas; +Cc: netdev, raghu.vatsavayi, derek.chickles, satananda.burla
In-Reply-To: <20170609022036.GA1327@felix-thinkpad.cavium.com>
From: Felix Manlunas <felix.manlunas@cavium.com>
Date: Thu, 8 Jun 2017 19:20:36 -0700
> From: Derek Chickles <derek.chickles@cavium.com>
>
> Disallow enabling firmware debug from a VF. Only PF is allowed to do that.
>
> Signed-off-by: Derek Chickles <derek.chickles@cavium.com>
> Signed-off-by: Felix Manlunas <felix.manlunas@cavium.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next 0/5] nfp: FW app build name reporting
From: David Miller @ 2017-06-09 16:53 UTC (permalink / raw)
To: jakub.kicinski; +Cc: netdev, oss-drivers
In-Reply-To: <20170609035614.12691-1-jakub.kicinski@netronome.com>
From: Jakub Kicinski <jakub.kicinski@netronome.com>
Date: Thu, 8 Jun 2017 20:56:09 -0700
> This series adds reporting FW build name in ethtool -i. Most
> of the patches are restructuring where information caching is
> done. There is also a minor error path fix.
>
> These are last few patches finishing the basic nfp_app support.
Series applied, thank you.
^ permalink raw reply
* Re: [PATCH net] ipv4: igmp: fix a use after free
From: Cong Wang @ 2017-06-09 17:01 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Xin Long, Andrey Konovalov, David S. Miller, netdev
In-Reply-To: <1497023774.736.92.camel@edumazet-glaptop3.roam.corp.google.com>
On Fri, Jun 9, 2017 at 8:56 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Fri, 2017-06-09 at 14:24 +0800, Xin Long wrote:
>> On Fri, Jun 9, 2017 at 8:59 AM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
>>
>> > On Thu, Jun 8, 2017 at 1:33 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>> >> I mentioned (in https://lkml.org/lkml/2017/5/31/619 ) that we might need
>> >> to defer freeing after rcu grace period but for some reason decided it
>> >> was not needed.
>> Yes, this one could fix it.
>>
>> >
>> > This one makes sense, it is the second time I saw the use-after-free
>> > in igmp code, both are because we don't respect the RCU rule to free
>> > an element in the list.
>> >
>> >>
>> >> What about :
>> >
>> > But not sure if all ip_ma_put() callers want ip_mc_clear_src().
>> If that's problem, there may be another way:
>>
>> leave ip_mc_clear_src as it is, just add pmc->lock to protect this call.
>>
>> this use-after-free was actually caused by using pmc->sources/tomb
>> in add_grec while ip_mc_clear_src is freeing them. add_grec is already
>> under pmc->lock, so to add pmc->lock for ip_mc_clear_src should be
>> enough to protect the list pmc->sources/tomb.
>>
>> wdyt ?
>
> This would we weird.
>
> When we free skb components, we do not grab a spinlock.
>
> When we free something, just make sure we must be the last user of it.
>
> RCU rules -> Must respect RCU grace period before delete.
>
> No need for extra spinlock.
This is what I thought in my first response, until I realized
it is not pure RCU, otherwise pmc->lock should not be taken
in igmpv3_send_cr(). It seems the code is mixing the use
of spinlock and RCU.
We need RCU anyway, ip_check_mc_rcu() is the real fast
path where we don't take spinlock. I think we will need more
work.
^ permalink raw reply
* Re: [PATCH net-next v2] cxgb4: handle interrupt raised when FW crashes
From: David Miller @ 2017-06-09 17:12 UTC (permalink / raw)
To: ganeshgr; +Cc: netdev, nirranjan, indranil, venkatesh, rahul.lakkireddy
In-Reply-To: <1496986955-11024-1-git-send-email-ganeshgr@chelsio.com>
From: Ganesh Goudar <ganeshgr@chelsio.com>
Date: Fri, 9 Jun 2017 11:12:35 +0530
> From: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>
>
> Handle TIMER0INT when FW crashes. Check for PCIE_FW[FW_EVAL]
> and if it says "Device FW Crashed", then treat it as fatal.
> Else, non-fatal.
>
> Signed-off-by: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>
> Signed-off-by: Ganesh Goudar <ganeshgr@chelsio.com>
> ---
> v2: Following the reverse chirstmas tree variable ordering
Applied, thank you.
^ permalink raw reply
* Re: [PATCH v2 net-next] Ipvlan should return an error when an address is already in use.
From: Krister Johansen @ 2017-06-09 17:13 UTC (permalink / raw)
To: David Miller; +Cc: kjlx, maheshb, netdev
In-Reply-To: <20170609.122646.100677990071641882.davem@davemloft.net>
On Fri, Jun 09, 2017 at 12:26:46PM -0400, David Miller wrote:
> From: Krister Johansen <kjlx@templeofstupid.com>
> Date: Thu, 8 Jun 2017 13:12:14 -0700
>
> > The ipvlan code already knows how to detect when a duplicate address is
> > about to be assigned to an ipvlan device. However, that failure is not
> > propogated outward and leads to a silent failure.
> >
> > Introduce a validation step at ip address creation time and allow device
> > drivers to register to validate the incoming ip addresses. The ipvlan
> > code is the first consumer. If it detects an address in use, we can
> > return an error to the user before beginning to commit the new ifa in
> > the networking code.
> >
> > This can be especially useful if it is necessary to provision many
> > ipvlans in containers. The provisioning software (or operator) can use
> > this to detect situations where an ip address is unexpectedly in use.
> >
> > Signed-off-by: Krister Johansen <kjlx@templeofstupid.com>
>
> Ok, applied, thank you.
Thanks, did this look otherwise alright? I was a little nervous about
dropping and re-acquiring the rcu_read_lock_bh() in net/ipv6/addrconf.c
around line 975, but in the current design holding rcu_read_lock_bh()
causes the in_softirq() check in the validator (and the add/remove
ipvlan code itself) to return NOTIFY_DONE immediately.
AFAICT, the rcu_read_lock was to protect the idev. I changed that to
get subsequently released in the outbound path. I was also unsure if
it's safe to call in6_dev_put from a bh context.
Thanks,
-K
^ permalink raw reply
* Re: [PATCH] net: Fix inconsistent teardown and release of private netdev state.
From: David Miller @ 2017-06-09 17:14 UTC (permalink / raw)
To: johannes; +Cc: netdev
In-Reply-To: <1497018827.12088.1.camel@sipsolutions.net>
From: Johannes Berg <johannes@sipsolutions.net>
Date: Fri, 09 Jun 2017 16:33:47 +0200
> Right. Do you want me to put that into my tree? I could do it tonight,
> or perhaps only Monday though.
Please submit it formally to netdev for me to apply.
I want to queue it up with the original patch for -stable to make
sure all the fallout is addressed.
Thank you.
^ permalink raw reply
* Re: [PATCH v2 net-next] Ipvlan should return an error when an address is already in use.
From: David Miller @ 2017-06-09 17:15 UTC (permalink / raw)
To: kjlx; +Cc: maheshb, netdev
In-Reply-To: <20170609171310.GA2766@templeofstupid.com>
From: Krister Johansen <kjlx@templeofstupid.com>
Date: Fri, 9 Jun 2017 10:13:10 -0700
> On Fri, Jun 09, 2017 at 12:26:46PM -0400, David Miller wrote:
>> From: Krister Johansen <kjlx@templeofstupid.com>
>> Date: Thu, 8 Jun 2017 13:12:14 -0700
>>
>> > The ipvlan code already knows how to detect when a duplicate address is
>> > about to be assigned to an ipvlan device. However, that failure is not
>> > propogated outward and leads to a silent failure.
>> >
>> > Introduce a validation step at ip address creation time and allow device
>> > drivers to register to validate the incoming ip addresses. The ipvlan
>> > code is the first consumer. If it detects an address in use, we can
>> > return an error to the user before beginning to commit the new ifa in
>> > the networking code.
>> >
>> > This can be especially useful if it is necessary to provision many
>> > ipvlans in containers. The provisioning software (or operator) can use
>> > this to detect situations where an ip address is unexpectedly in use.
>> >
>> > Signed-off-by: Krister Johansen <kjlx@templeofstupid.com>
>>
>> Ok, applied, thank you.
>
> Thanks, did this look otherwise alright?
Yes, I was mildly unsatisfied with the ipv6 addrconf situation but I know
very well about that, and those kinds of addresses aren't of interest
for what you are trying to achieve right?
^ permalink raw reply
* Reply for more details
From: Mr.David Kekeli @ 2017-06-09 17:17 UTC (permalink / raw)
My Greetings,
I am a banker by profession and currently holds the position of Chief
Auditor in our bank, I have the ability to transfer unclaimed funds of
U.S.$ 6.2 m (6.2 million dollars) that belong to one of our late
customer, who bear the same last name with you who died in a car crash
along with his Family and no one came to put claim the funds, if left
unclaimed the fund will be Transferred to the state treasury in the
bank so I invite you to a Deal where we can facilitate the transfer of
this fund, and I promise You 50% as a percentage of the fund will be
transferred into your account, meanwhile here are the needed
information from you for the facilitation of the funds.
1.Your First Name
2.Your Last Name
3.Your Telephone number
4.Your Age
5.Your Country
6.Your occupation
7.Your Email Address
Am waiting for your urgent reply so that we will starts immediately,
Sorry if you received this letter in your spam, Due to recent
connection error here in my country. May Almighty God Bless You!
I wait to hear from you for more details,
Best Regard,
David Kekeli.
^ permalink raw reply
* Re: [PATCH] net: Fix inconsistent teardown and release of private netdev state.
From: Stephen Hemminger @ 2017-06-09 17:21 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20170607.155411.201795978436906426.davem@davemloft.net>
On Wed, 07 Jun 2017 15:54:11 -0400 (EDT)
David Miller <davem@davemloft.net> wrote:
> Network devices can allocate reasources and private memory using
> netdev_ops->ndo_init(). However, the release of these resources
> can occur in one of two different places.
>
> Either netdev_ops->ndo_uninit() or netdev->destructor().
>
> The decision of which operation frees the resources depends upon
> whether it is necessary for all netdev refs to be released before it
> is safe to perform the freeing.
>
> netdev_ops->ndo_uninit() presumably can occur right after the
> NETDEV_UNREGISTER notifier completes and the unicast and multicast
> address lists are flushed.
>
> netdev->destructor(), on the other hand, does not run until the
> netdev references all go away.
>
> Further complicating the situation is that netdev->destructor()
> almost universally does also a free_netdev().
>
> This creates a problem for the logic in register_netdevice().
> Because all callers of register_netdevice() manage the freeing
> of the netdev, and invoke free_netdev(dev) if register_netdevice()
> fails.
>
> If netdev_ops->ndo_init() succeeds, but something else fails inside
> of register_netdevice(), it does call ndo_ops->ndo_uninit(). But
> it is not able to invoke netdev->destructor().
>
> This is because netdev->destructor() will do a free_netdev() and
> then the caller of register_netdevice() will do the same.
>
> However, this means that the resources that would normally be released
> by netdev->destructor() will not be.
>
> Over the years drivers have added local hacks to deal with this, by
> invoking their destructor parts by hand when register_netdevice()
> fails.
>
> Many drivers do not try to deal with this, and instead we have leaks.
>
> Let's close this hole by formalizing the distinction between what
> private things need to be freed up by netdev->destructor() and whether
> the driver needs unregister_netdevice() to perform the free_netdev().
>
> netdev->priv_destructor() performs all actions to free up the private
> resources that used to be freed by netdev->destructor(), except for
> free_netdev().
>
> netdev->needs_free_netdev is a boolean that indicates whether
> free_netdev() should be done at the end of unregister_netdevice().
>
> Now, register_netdevice() can sanely release all resources after
> ndo_ops->ndo_init() succeeds, by invoking both ndo_ops->ndo_uninit()
> and netdev->priv_destructor().
>
> And at the end of unregister_netdevice(), we invoke
> netdev->priv_destructor() and optionally call free_netdev().
>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> ---
>
> This is from a few weeks ago, pushed to 'net' and queued up for
> -stable.
>
> drivers/net/bonding/bond_main.c | 6 +++---
> drivers/net/caif/caif_hsi.c | 2 +-
> drivers/net/caif/caif_serial.c | 2 +-
> drivers/net/caif/caif_spi.c | 2 +-
> drivers/net/caif/caif_virtio.c | 2 +-
> drivers/net/can/slcan.c | 7 +++----
> drivers/net/can/vcan.c | 2 +-
> drivers/net/can/vxcan.c | 2 +-
> drivers/net/dummy.c | 4 ++--
> drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c | 2 +-
> drivers/net/geneve.c | 2 +-
> drivers/net/gtp.c | 2 +-
> drivers/net/hamradio/6pack.c | 2 +-
> drivers/net/hamradio/bpqether.c | 2 +-
> drivers/net/ifb.c | 4 ++--
> drivers/net/ipvlan/ipvlan_main.c | 2 +-
> drivers/net/loopback.c | 4 ++--
> drivers/net/macsec.c | 4 ++--
> drivers/net/macvlan.c | 2 +-
> drivers/net/nlmon.c | 2 +-
> drivers/net/slip/slip.c | 7 +++----
> drivers/net/team/team.c | 4 ++--
> drivers/net/tun.c | 4 ++--
> drivers/net/usb/cdc-phonet.c | 2 +-
> drivers/net/usb/qmi_wwan.c | 2 +-
> drivers/net/veth.c | 4 ++--
> drivers/net/vrf.c | 2 +-
> drivers/net/vsockmon.c | 2 +-
> drivers/net/vxlan.c | 2 +-
> drivers/net/wan/dlci.c | 2 +-
> drivers/net/wan/hdlc_fr.c | 2 +-
> drivers/net/wan/lapbether.c | 2 +-
> drivers/net/wireless/ath/ath6kl/main.c | 2 +-
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 1 -
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c | 3 ++-
> drivers/net/wireless/intersil/hostap/hostap_main.c | 2 +-
> drivers/net/wireless/mac80211_hwsim.c | 2 +-
> drivers/net/wireless/marvell/mwifiex/main.c | 2 +-
> drivers/staging/rtl8188eu/os_dep/mon.c | 2 +-
> drivers/usb/gadget/function/f_phonet.c | 2 +-
> include/linux/netdevice.h | 7 ++++---
> net/8021q/vlan_dev.c | 4 ++--
> net/batman-adv/soft-interface.c | 5 ++---
> net/bluetooth/6lowpan.c | 2 +-
> net/bridge/br_device.c | 2 +-
> net/caif/chnl_net.c | 4 ++--
> net/core/dev.c | 8 ++++++--
> net/hsr/hsr_device.c | 4 ++--
> net/ieee802154/6lowpan/core.c | 2 +-
> net/ipv4/ip_tunnel.c | 4 ++--
> net/ipv4/ipmr.c | 2 +-
> net/ipv6/ip6_gre.c | 9 +++++----
> net/ipv6/ip6_tunnel.c | 8 ++++----
> net/ipv6/ip6_vti.c | 8 ++++----
> net/ipv6/ip6mr.c | 2 +-
> net/ipv6/sit.c | 6 +++---
> net/irda/irlan/irlan_eth.c | 2 +-
> net/l2tp/l2tp_eth.c | 2 +-
> net/mac80211/iface.c | 6 +++---
> net/mac802154/iface.c | 7 +++----
> net/openvswitch/vport-internal_dev.c | 4 ++--
> net/phonet/pep-gprs.c | 2 +-
> 62 files changed, 105 insertions(+), 103 deletions(-)
Is there anything in Documentation/networking/netdevices.txt about this to
avoid any future issues?
^ permalink raw reply
* Re: [PATCH v2 net-next] Ipvlan should return an error when an address is already in use.
From: Krister Johansen @ 2017-06-09 17:25 UTC (permalink / raw)
To: David Miller; +Cc: kjlx, maheshb, netdev
In-Reply-To: <20170609.131510.1262676940920520377.davem@davemloft.net>
On Fri, Jun 09, 2017 at 01:15:10PM -0400, David Miller wrote:
> From: Krister Johansen <kjlx@templeofstupid.com>
> Date: Fri, 9 Jun 2017 10:13:10 -0700
>
> > On Fri, Jun 09, 2017 at 12:26:46PM -0400, David Miller wrote:
> >> From: Krister Johansen <kjlx@templeofstupid.com>
> >> Date: Thu, 8 Jun 2017 13:12:14 -0700
> >>
> >> > The ipvlan code already knows how to detect when a duplicate address is
> >> > about to be assigned to an ipvlan device. However, that failure is not
> >> > propogated outward and leads to a silent failure.
> >> >
> >> > Introduce a validation step at ip address creation time and allow device
> >> > drivers to register to validate the incoming ip addresses. The ipvlan
> >> > code is the first consumer. If it detects an address in use, we can
> >> > return an error to the user before beginning to commit the new ifa in
> >> > the networking code.
> >> >
> >> > This can be especially useful if it is necessary to provision many
> >> > ipvlans in containers. The provisioning software (or operator) can use
> >> > this to detect situations where an ip address is unexpectedly in use.
> >> >
> >> > Signed-off-by: Krister Johansen <kjlx@templeofstupid.com>
> >>
> >> Ok, applied, thank you.
> >
> > Thanks, did this look otherwise alright?
>
> Yes, I was mildly unsatisfied with the ipv6 addrconf situation but I know
> very well about that, and those kinds of addresses aren't of interest
> for what you are trying to achieve right?
That's correct. I'm basically trying to catch the case where 'ip addr'
or its equivalent rtnetlink invocation manually configure an ip address
on a ipvlan.
-K
^ permalink raw reply
* Re: [PATCH net] netlink: don't send unknown nsid
From: Flavio Leitner @ 2017-06-09 17:33 UTC (permalink / raw)
To: Nicolas Dichtel; +Cc: davem, netdev
In-Reply-To: <057303a1-5d27-1b24-c8b0-d3cf46b14825@6wind.com>
On Thu, Jun 08, 2017 at 10:31:53AM +0200, Nicolas Dichtel wrote:
> Le 07/06/2017 à 21:14, Flavio Leitner a écrit :
> > Let's say the app is restarted, or another monitoring app is executed
> > with enough perms. How will it identify the error condition?
> Your app wants to monitor a subset of netns. It means that you already have a
> way to identify those netns, something like a file stored somewhere
> (/var/run/netns/, /proc/<pid>/ns/net, ...). Thus, it's easy to check if those
> netns have a nsid assigned in the netns where your app will open the socket.
>
> This option was called NETLINK_F_LISTEN_ALL_NSID, because it only enables to
> listen netns *with* a nsid assigned, nothing more. It's up to the user to ensure
> that nsid are correctly assigned.
Makes sense, thanks.
--
Flavio
^ permalink raw reply
* Re: [PATCH net] ipv4: igmp: fix a use after free
From: Xin Long @ 2017-06-09 18:05 UTC (permalink / raw)
To: Cong Wang; +Cc: Eric Dumazet, Andrey Konovalov, David S. Miller, netdev
In-Reply-To: <CAM_iQpVcLiE3ru1ukq9CCTvTN_+-77XapOWT=1EgF0DgB224-w@mail.gmail.com>
On Sat, Jun 10, 2017 at 1:01 AM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> On Fri, Jun 9, 2017 at 8:56 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>> On Fri, 2017-06-09 at 14:24 +0800, Xin Long wrote:
>>> On Fri, Jun 9, 2017 at 8:59 AM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
>>>
>>> > On Thu, Jun 8, 2017 at 1:33 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>>> >> I mentioned (in https://lkml.org/lkml/2017/5/31/619 ) that we might need
>>> >> to defer freeing after rcu grace period but for some reason decided it
>>> >> was not needed.
>>> Yes, this one could fix it.
>>>
>>> >
>>> > This one makes sense, it is the second time I saw the use-after-free
>>> > in igmp code, both are because we don't respect the RCU rule to free
>>> > an element in the list.
>>> >
>>> >>
>>> >> What about :
>>> >
>>> > But not sure if all ip_ma_put() callers want ip_mc_clear_src().
>>> If that's problem, there may be another way:
>>>
>>> leave ip_mc_clear_src as it is, just add pmc->lock to protect this call.
>>>
>>> this use-after-free was actually caused by using pmc->sources/tomb
>>> in add_grec while ip_mc_clear_src is freeing them. add_grec is already
>>> under pmc->lock, so to add pmc->lock for ip_mc_clear_src should be
>>> enough to protect the list pmc->sources/tomb.
>>>
>>> wdyt ?
>>
>> This would we weird.
>>
>> When we free skb components, we do not grab a spinlock.
>>
>> When we free something, just make sure we must be the last user of it.
>>
>> RCU rules -> Must respect RCU grace period before delete.
>>
>> No need for extra spinlock.
>
> This is what I thought in my first response, until I realized
> it is not pure RCU, otherwise pmc->lock should not be taken
> in igmpv3_send_cr(). It seems the code is mixing the use
> of spinlock and RCU.
rcu lock is for pmc not being freed, and spinlock is for pmc's
members' modification. is there some rule these two locks
should be mixed?
>
> We need RCU anyway, ip_check_mc_rcu() is the real fast
> path where we don't take spinlock. I think we will need more
> work.
It seems all add_grec() callings needs spinlock, maybe add_grec
modifies pmc's member. it's hard to drop spinlock.
from ip_check_mc_rcu you mentioned about, it should be right
to call ip_mc_clear_src after rcu grace period, like Eric's patch.
^ 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