* Re: [PATCH net-next] net: dsa: mv88e6xxx: missing unlock on error path
From: David Miller @ 2018-08-14 17:08 UTC (permalink / raw)
To: dan.carpenter; +Cc: andrew, vivien.didelot, f.fainelli, netdev, kernel-janitors
In-Reply-To: <20180814090905.rm6i665c5utl7d32@kili.mountain>
From: Dan Carpenter <dan.carpenter@oracle.com>
Date: Tue, 14 Aug 2018 12:09:05 +0300
> We added a new error path, but we need to drop the lock before we return.
>
> Fixes: 2d2e1dd29962 ("net: dsa: mv88e6xxx: Cache the port cmode")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Applied.
^ permalink raw reply
* Re: [PATCH net] cxgb4: Add new T5 PCI device ids 0x50af and 0x50b0
From: David Miller @ 2018-08-14 17:09 UTC (permalink / raw)
To: ganeshgr; +Cc: netdev, dt
In-Reply-To: <1534243897-22877-1-git-send-email-ganeshgr@chelsio.com>
From: Ganesh Goudar <ganeshgr@chelsio.com>
Date: Tue, 14 Aug 2018 16:21:37 +0530
> Signed-off-by: Ganesh Goudar <ganeshgr@chelsio.com>
Applied.
^ permalink raw reply
* [PATCH] hv/netvsc: Fix NULL dereference at single queue mode fallback
From: Takashi Iwai @ 2018-08-14 17:10 UTC (permalink / raw)
To: Stephen Hemminger
Cc: K . Y . Srinivasan, Haiyang Zhang, David S . Miller, devel,
netdev
The recent commit 916c5e1413be ("hv/netvsc: fix handling of fallback
to single queue mode") tried to fix the fallback behavior to a single
queue mode, but it changed the function to return zero incorrectly,
while the function should return an object pointer. Eventually this
leads to a NULL dereference at the callers that expect non-NULL
value.
Fix it by returning the proper net_device object.
Fixes: 916c5e1413be ("hv/netvsc: fix handling of fallback to single queue mode")
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
drivers/net/hyperv/rndis_filter.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/hyperv/rndis_filter.c b/drivers/net/hyperv/rndis_filter.c
index 408ece27131c..2a5209f23f29 100644
--- a/drivers/net/hyperv/rndis_filter.c
+++ b/drivers/net/hyperv/rndis_filter.c
@@ -1338,7 +1338,7 @@ struct netvsc_device *rndis_filter_device_add(struct hv_device *dev,
/* setting up multiple channels failed */
net_device->max_chn = 1;
net_device->num_chn = 1;
- return 0;
+ return net_device;
err_dev_remv:
rndis_filter_device_remove(dev, net_device);
--
2.18.0
^ permalink raw reply related
* Re: [Intel-wired-lan] [PATCH next-queue 0/8] ixgbe/ixgbevf: IPsec offload support for VFs
From: Shannon Nelson @ 2018-08-14 17:10 UTC (permalink / raw)
To: Alexander Duyck; +Cc: intel-wired-lan, Jeff Kirsher, Steffen Klassert, Netdev
In-Reply-To: <CAKgT0UdcijJ4f4gKeHj6t_EGuKjE7P5oZ0UH0_jfuHQ05RJALA@mail.gmail.com>
On 8/14/2018 8:30 AM, Alexander Duyck wrote:
> On Mon, Aug 13, 2018 at 11:43 AM Shannon Nelson
> <shannon.nelson@oracle.com> wrote:
>>
>> This set of patches implements IPsec hardware offload for VF devices in
>> Intel's 10Gbe x540 family of Ethernet devices.
[...]
>
> So the one question I would have about this patch set is what happens
> if you are setting up a ipsec connection between the PF and one of the
> VFs on the same port/function? Do the ipsec offloads get translated
> across the Tx loopback or do they end up causing issues? Specifically
> I would be interested in seeing the results of a test either between
> two VFs, or the PF and one of the VFs on the same port.
>
> - Alex
>
There is definitely something funky in the internal switch connection,
as messages going from PF to VF with an offloaded encryption don't seem
to get received by the VF, at least when in a VEB setup. If I only set
up offloads on the Rx on both PF and VF, and don't offload the Tx, then
things work.
I don't have a setup to test this, but I suspect that in a VEPA
configuration, with packets going out to the switch and turned around
back in, the Tx encryption offload would happen as expected.
sln
^ permalink raw reply
* RE: [PATCH] dt-bindings: net: ravb: Add support for r8a774a1 SoC
From: Fabrizio Castro @ 2018-08-14 17:13 UTC (permalink / raw)
To: David Miller
Cc: robh+dt@kernel.org, mark.rutland@arm.com,
sergei.shtylyov@cogentembedded.com, geert+renesas@glider.be,
horms+renesas@verge.net.au, Biju Das, Yoshihiro Shimoda,
netdev@vger.kernel.org, linux-renesas-soc@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
horms@verge.net.au, Chris Paterson
In-Reply-To: <20180814.100957.1728722828479885181.davem@davemloft.net>
Hello David,
> Subject: Re: [PATCH] dt-bindings: net: ravb: Add support for r8a774a1 SoC
>
> From: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> Date: Tue, 14 Aug 2018 13:33:37 +0100
>
> > Document RZ/G2M (R8A774A1) SoC bindings.
> >
> > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> > Reviewed-by: Biju Das <biju.das@bp.renesas.com>
>
> Which tree is this targetting?
This patch applies on next-20180814
Thanks,
Fab
>
> Thanks.
Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered No. 04586709.
^ permalink raw reply
* Re: [PATCH] hv/netvsc: Fix NULL dereference at single queue mode fallback
From: Stephen Hemminger @ 2018-08-14 17:18 UTC (permalink / raw)
To: Takashi Iwai
Cc: Stephen Hemminger, K . Y . Srinivasan, Haiyang Zhang,
David S . Miller, devel, netdev
In-Reply-To: <20180814171050.26356-1-tiwai@suse.de>
On Tue, 14 Aug 2018 19:10:50 +0200
Takashi Iwai <tiwai@suse.de> wrote:
> The recent commit 916c5e1413be ("hv/netvsc: fix handling of fallback
> to single queue mode") tried to fix the fallback behavior to a single
> queue mode, but it changed the function to return zero incorrectly,
> while the function should return an object pointer. Eventually this
> leads to a NULL dereference at the callers that expect non-NULL
> value.
>
> Fix it by returning the proper net_device object.
>
> Fixes: 916c5e1413be ("hv/netvsc: fix handling of fallback to single queue mode")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
Reviewed-by: Stephen Hemminger <stephen@networkplumber.org>
^ permalink raw reply
* Re: [PATCH net-next v6 08/11] net: sched: don't release reference on action overwrite
From: Vlad Buslov @ 2018-08-14 17:23 UTC (permalink / raw)
To: Cong Wang
Cc: Linux Kernel Network Developers, David Miller, Jamal Hadi Salim,
Jiri Pirko, Alexei Starovoitov, Daniel Borkmann,
Yevgeny Kliteynik, Jiri Pirko
In-Reply-To: <CAM_iQpV3ftNR6H0CEXQ01T08WTWNig3xcLxSi7joV+W6atAamA@mail.gmail.com>
On Mon 13 Aug 2018 at 23:00, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> On Thu, Jul 5, 2018 at 7:24 AM Vlad Buslov <vladbu@mellanox.com> wrote:
>> diff --git a/net/sched/act_ife.c b/net/sched/act_ife.c
>> index 89a761395c94..acea3feae762 100644
>> --- a/net/sched/act_ife.c
>> +++ b/net/sched/act_ife.c
> ...
>> @@ -548,6 +546,8 @@ static int tcf_ife_init(struct net *net, struct nlattr *nla,
>>
>> if (exists)
>> spin_unlock_bh(&ife->tcf_lock);
>> + tcf_idr_release(*a, bind);
>> +
>> kfree(p);
>> return err;
>> }
>
> With this change, you seem release it twice when nla_parse_nested() fails
> for ACT_P_CREATED case...?
Thank you, great catch!
>
> Looks like what you want is the following?
>
> if (err) {
> tcf_idr_release(*a, bind);
> kfree(p);
> return err;
> }
Yes. Sending the fix.
^ permalink raw reply
* Re: [PATCH] bluetooth/{bnep,cmtp,hidp}: Remove unnecessary smp_mb__{before,after}_atomic
From: Brian Norris @ 2018-08-14 20:16 UTC (permalink / raw)
To: Andrea Parri
Cc: Marcel Holtmann, Johan Hedberg, David S . Miller, linux-bluetooth,
netdev, linux-kernel, Jeffy Chen, Brian Norris, AL Yu-Chen Cho
In-Reply-To: <1534272066-9366-1-git-send-email-andrea.parri@amarulasolutions.com>
On Tue, Aug 14, 2018 at 08:41:06PM +0200, Andrea Parri wrote:
> The barriers are unneeded; wait_woken() and woken_wake_function()
> already provide us with the required synchronization: remove them
> and document that we're relying on the (implicit) synchronization
> provided by wait_woken() and woken_wake_function().
>
> Signed-off-by: Andrea Parri <andrea.parri@amarulasolutions.com>
> ---
> net/bluetooth/bnep/core.c | 7 ++++---
> net/bluetooth/cmtp/core.c | 14 ++++++++------
> net/bluetooth/hidp/core.c | 13 ++++++++-----
> 3 files changed, 20 insertions(+), 14 deletions(-)
Seems good to me:
Reviewed-by: Brian Norris <computersforpeace@gmail.com>
^ permalink raw reply
* Re: [PATCH] hv/netvsc: Fix NULL dereference at single queue mode fallback
From: David Miller @ 2018-08-14 17:29 UTC (permalink / raw)
To: tiwai; +Cc: sthemmin, kys, haiyangz, devel, netdev
In-Reply-To: <20180814171050.26356-1-tiwai@suse.de>
From: Takashi Iwai <tiwai@suse.de>
Date: Tue, 14 Aug 2018 19:10:50 +0200
> The recent commit 916c5e1413be ("hv/netvsc: fix handling of fallback
> to single queue mode") tried to fix the fallback behavior to a single
> queue mode, but it changed the function to return zero incorrectly,
> while the function should return an object pointer. Eventually this
> leads to a NULL dereference at the callers that expect non-NULL
> value.
>
> Fix it by returning the proper net_device object.
>
> Fixes: 916c5e1413be ("hv/netvsc: fix handling of fallback to single queue mode")
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
Applied and queued up for -stable.
Please do not put explicit "CC: stable" notations in networking patches, I queue
up and submit networking patches to -stable explicitly.
Thank you.
^ permalink raw reply
* [PATCH net-next] net: sched: act_ife: always release ife action on init error
From: Vlad Buslov @ 2018-08-14 17:29 UTC (permalink / raw)
To: netdev; +Cc: davem, jhs, xiyou.wangcong, jiri, Vlad Buslov
Action init API was changed to always take reference to action, even when
overwriting existing action. Substitute conditional action release, which
was executed only if action is newly created, with unconditional release in
tcf_ife_init() error handling code to prevent double free or memory leak in
case of overwrite.
Fixes: 4e8ddd7f1758 ("net: sched: don't release reference on action overwrite")
Reported-by: Cong Wang <xiyou.wangcong@gmail.com>
Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
---
net/sched/act_ife.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/net/sched/act_ife.c b/net/sched/act_ife.c
index 5d200495e467..c524edcad900 100644
--- a/net/sched/act_ife.c
+++ b/net/sched/act_ife.c
@@ -551,9 +551,6 @@ static int tcf_ife_init(struct net *net, struct nlattr *nla,
NULL, NULL);
if (err) {
metadata_parse_err:
- if (ret == ACT_P_CREATED)
- tcf_idr_release(*a, bind);
-
if (exists)
spin_unlock_bh(&ife->tcf_lock);
tcf_idr_release(*a, bind);
@@ -574,11 +571,10 @@ static int tcf_ife_init(struct net *net, struct nlattr *nla,
*/
err = use_all_metadata(ife);
if (err) {
- if (ret == ACT_P_CREATED)
- tcf_idr_release(*a, bind);
-
if (exists)
spin_unlock_bh(&ife->tcf_lock);
+ tcf_idr_release(*a, bind);
+
kfree(p);
return err;
}
--
2.7.5
^ permalink raw reply related
* Re: [PATCH net-next] net: sched: act_ife: disable bh when taking ife_mod_lock
From: Vlad Buslov @ 2018-08-14 17:34 UTC (permalink / raw)
To: Cong Wang
Cc: Linux Kernel Network Developers, David Miller, Jamal Hadi Salim,
Jiri Pirko
In-Reply-To: <CAM_iQpUk6vo_9StceKQ+RLgB6FN_+yxPGC6BSeKsKwpWjGdw7Q@mail.gmail.com>
On Mon 13 Aug 2018 at 23:18, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> Hi, Vlad,
>
> Could you help to test my fixes?
>
> I just pushed them into my own git repo:
> https://github.com/congwang/linux/commits/net-sched-fixes
>
> Particularly, this is the revert:
> https://github.com/congwang/linux/commit/b3f51c4ab8272cc8d3244848e528fce1426c4659
> and this is my fix for the lockdep warning you reported:
> https://github.com/congwang/linux/commit/ecadcde94919183e9f0d5bc376f05e731baf2661
>
> I don't have environment to test ife modules.
Hi Cong,
I've run the test with your patch applied and couldn't reproduce the
lockdep warning.
>
> BTW, this is the fix for the deadlock I spotted:
> https://github.com/congwang/linux/commit/44f3d7f5b6ed2d4a46177e6c658fa23b76141afa
>
> Thanks!
^ permalink raw reply
* Re: [PATCH 1/1] NFC: Fix possible memory corruption when handling SHDLC I-Frame commands
From: Suren Baghdasaryan @ 2018-08-14 20:26 UTC (permalink / raw)
To: Dan Carpenter
Cc: security, Kevin Deus, Samuel Ortiz, David S. Miller, Allen Pais,
Kees Cook, linux-wireless, netdev, linux-kernel
In-Reply-To: <CAJuCfpF0Y6wc1z-rbn2wdWROgc_FkD2EX5HzDMcZ1oB5Z=dXSw@mail.gmail.com>
On Tue, Aug 14, 2018 at 9:57 AM, Suren Baghdasaryan <surenb@google.com> wrote:
> On Tue, Aug 14, 2018 at 2:54 AM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
>> Thanks. This is great. I'm so glad these are finally getting fixed.
>>
>> Do we need to fix nfc_hci_msg_rx_work() and nfc_hci_recv_from_llc() as
>> well? In nfc_hci_recv_from_llc() we allow pipe to be NFC_HCI_FRAGMENT
>> (0x7f) so that's one element beyond the end of the array and the
>> NFC_HCI_HCP_RESPONSE isn't checked.
>>
>> Also nci_hci_msg_rx_work() and nci_hci_data_received_cb() use
>> NCI_HCP_MSG_GET_PIPE() so those could be off by one.
>
> Good point. From hci.h:
>
> /*
> * According to specification 102 622 chapter 4.4 Pipes,
> * the pipe identifier is 7 bits long.
> */
> #define NFC_HCI_MAX_PIPES 127
>
> And then:
>
> struct nfc_hci_dev {
> ...
> struct nfc_hci_pipe pipes[NFC_HCI_MAX_PIPES];
> ...
> }
>
> I think the correct fix would be to change it to:
>
> struct nfc_hci_pipe pipes[NFC_HCI_MAX_PIPES + 1];
>
> What do you think?
>
Just to be clear, this would fix the problem Dan described in his
reply and it should be implemented in a separate patch. The original
fix is still valid.
>>
>> regards,
>> dan carpenter
>>
^ permalink raw reply
* Re: [PATCH][bpf-next] bpf: test: fix spelling mistake "REUSEEPORT" -> "REUSEPORT"
From: Alexei Starovoitov @ 2018-08-14 17:39 UTC (permalink / raw)
To: Colin King; +Cc: Alexei Starovoitov, Daniel Borkmann, Shuah Khan, netdev, davem
In-Reply-To: <20180813140032.2208-1-colin.king@canonical.com>
On Mon, Aug 13, 2018 at 03:00:32PM +0100, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> Trivial fix to spelling mistake in error message
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Acked-by: Alexei Starovoitov <ast@kernel.org>
Dave, may be you can take this one directly, since PR for net-next is not yet sent to Linus?
^ permalink raw reply
* Re: [PATCH][bpf-next] bpf: test: fix spelling mistake "REUSEEPORT" -> "REUSEPORT"
From: David Miller @ 2018-08-14 17:43 UTC (permalink / raw)
To: alexei.starovoitov; +Cc: colin.king, ast, daniel, shuah, netdev
In-Reply-To: <20180814173911.ymxmgb3vehfu4wfd@ast-mbp>
From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Date: Tue, 14 Aug 2018 10:39:12 -0700
> On Mon, Aug 13, 2018 at 03:00:32PM +0100, Colin King wrote:
>> From: Colin Ian King <colin.king@canonical.com>
>>
>> Trivial fix to spelling mistake in error message
>>
>> Signed-off-by: Colin Ian King <colin.king@canonical.com>
>
> Acked-by: Alexei Starovoitov <ast@kernel.org>
>
> Dave, may be you can take this one directly, since PR for net-next is not yet sent to Linus?
Sure, done.
^ permalink raw reply
* Re: [PATCH 1/1] NFC: Fix possible memory corruption when handling SHDLC I-Frame commands
From: Kees Cook @ 2018-08-14 20:33 UTC (permalink / raw)
To: Suren Baghdasaryan
Cc: Dan Carpenter, Security Officers, Kevin Deus, Samuel Ortiz,
David S. Miller, Allen Pais, linux-wireless, Network Development,
LKML
In-Reply-To: <CAJuCfpHSZuabLr6+t_n7UDamJCAP6EPTNEZ6kNqWOhk2mpYADg@mail.gmail.com>
On Tue, Aug 14, 2018 at 1:26 PM, Suren Baghdasaryan <surenb@google.com> wrote:
> On Tue, Aug 14, 2018 at 9:57 AM, Suren Baghdasaryan <surenb@google.com> wrote:
>> On Tue, Aug 14, 2018 at 2:54 AM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
>>> Thanks. This is great. I'm so glad these are finally getting fixed.
>>>
>>> Do we need to fix nfc_hci_msg_rx_work() and nfc_hci_recv_from_llc() as
>>> well? In nfc_hci_recv_from_llc() we allow pipe to be NFC_HCI_FRAGMENT
>>> (0x7f) so that's one element beyond the end of the array and the
>>> NFC_HCI_HCP_RESPONSE isn't checked.
>>>
>>> Also nci_hci_msg_rx_work() and nci_hci_data_received_cb() use
>>> NCI_HCP_MSG_GET_PIPE() so those could be off by one.
>>
>> Good point. From hci.h:
>>
>> /*
>> * According to specification 102 622 chapter 4.4 Pipes,
>> * the pipe identifier is 7 bits long.
>> */
>> #define NFC_HCI_MAX_PIPES 127
>>
>> And then:
>>
>> struct nfc_hci_dev {
>> ...
>> struct nfc_hci_pipe pipes[NFC_HCI_MAX_PIPES];
>> ...
>> }
>>
>> I think the correct fix would be to change it to:
>>
>> struct nfc_hci_pipe pipes[NFC_HCI_MAX_PIPES + 1];
>>
>> What do you think?
>>
>
> Just to be clear, this would fix the problem Dan described in his
> reply and it should be implemented in a separate patch. The original
> fix is still valid.
I think you could merge the fixes into a single patch.
-Kees
--
Kees Cook
Pixel Security
^ permalink raw reply
* Re: [PATCH] hv/netvsc: Fix NULL dereference at single queue mode fallback
From: Takashi Iwai @ 2018-08-14 17:47 UTC (permalink / raw)
To: David Miller; +Cc: sthemmin, kys, haiyangz, devel, netdev
In-Reply-To: <20180814.102932.1425242139845133705.davem@davemloft.net>
On Tue, 14 Aug 2018 19:29:32 +0200,
David Miller wrote:
>
> From: Takashi Iwai <tiwai@suse.de>
> Date: Tue, 14 Aug 2018 19:10:50 +0200
>
> > The recent commit 916c5e1413be ("hv/netvsc: fix handling of fallback
> > to single queue mode") tried to fix the fallback behavior to a single
> > queue mode, but it changed the function to return zero incorrectly,
> > while the function should return an object pointer. Eventually this
> > leads to a NULL dereference at the callers that expect non-NULL
> > value.
> >
> > Fix it by returning the proper net_device object.
> >
> > Fixes: 916c5e1413be ("hv/netvsc: fix handling of fallback to single queue mode")
> > Signed-off-by: Takashi Iwai <tiwai@suse.de>
>
> Applied and queued up for -stable.
>
> Please do not put explicit "CC: stable" notations in networking patches, I queue
> up and submit networking patches to -stable explicitly.
OK, noted for the next time. Thanks!
Takashi
^ permalink raw reply
* Re: [PATCH 1/3] dt-bindings: net: dsa: Add compatibility strings for Broadcom Omega
From: Rob Herring @ 2018-08-14 20:41 UTC (permalink / raw)
To: Arun Parameswaran
Cc: David S. Miller, Florian Fainelli, Andrew Lunn, Vivien Didelot,
Mark Rutland, netdev, devicetree, linux-kernel,
bcm-kernel-feedback-list, Arun Parameswaran
In-Reply-To: <1533661364-15381-2-git-send-email-arun.parameswaran@broadcom.com>
On Tue, 7 Aug 2018 10:02:42 -0700, Arun Parameswaran wrote:
> Add compatibility strings for the internal switch in the Broadcom
> Omega SoC family (BCM5831X/BCM1140X) to B53.
>
> Signed-off-by: Arun Parameswaran <arun.parameswaran@broadcom.com>
> ---
> Documentation/devicetree/bindings/net/dsa/b53.txt | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* RE: [PATCH] dt-bindings: net: ravb: Add support for r8a774a1 SoC
From: Fabrizio Castro @ 2018-08-14 17:58 UTC (permalink / raw)
To: David Miller
Cc: robh+dt@kernel.org, mark.rutland@arm.com,
sergei.shtylyov@cogentembedded.com, geert+renesas@glider.be,
horms+renesas@verge.net.au, Biju Das, Yoshihiro Shimoda,
netdev@vger.kernel.org, linux-renesas-soc@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
horms@verge.net.au, Chris Paterson
In-Reply-To: <20180814.101934.1621967443721994871.davem@davemloft.net>
Hello David,
> From: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> Date: Tue, 14 Aug 2018 17:13:30 +0000
>
> > Hello David,
> >
> >> Subject: Re: [PATCH] dt-bindings: net: ravb: Add support for r8a774a1 SoC
> >>
> >> From: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> >> Date: Tue, 14 Aug 2018 13:33:37 +0100
> >>
> >> > Document RZ/G2M (R8A774A1) SoC bindings.
> >> >
> >> > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> >> > Reviewed-by: Biju Das <biju.das@bp.renesas.com>
> >>
> >> Which tree is this targetting?
> >
> > This patch applies on next-20180814
>
> I'm asking something different. I'm asking which subsystem tree, and
> which maintainer, should apply this patch.
I think this is appropriate for net-next, and as far as I can tell previous patches have either been applied by you (mostly) or Rob Herring.
Maybe I should have put "[PATCH net-next]" in the Subject like the following?
https://patchwork.kernel.org/patch/10314835/
Thanks,
Fab
Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered No. 04586709.
^ permalink raw reply
* [PATCH bpf] bpf: fix a rcu usage warning in bpf_prog_array_copy_core()
From: Yonghong Song @ 2018-08-14 18:01 UTC (permalink / raw)
To: ast, daniel, netdev; +Cc: kernel-team, Roman Gushchin
Commit 394e40a29788 ("bpf: extend bpf_prog_array to store pointers
to the cgroup storage") refactored the bpf_prog_array_copy_core()
to accommodate new structure bpf_prog_array_item which contains
bpf_prog array itself.
In the old code, we had
perf_event_query_prog_array():
mutex_lock(...)
bpf_prog_array_copy_call():
prog = rcu_dereference_check(array, 1)->progs
bpf_prog_array_copy_core(prog, ...)
mutex_unlock(...)
With the above commit, we had
perf_event_query_prog_array():
mutex_lock(...)
bpf_prog_array_copy_call():
bpf_prog_array_copy_core(array, ...):
item = rcu_dereference(array)->items;
...
mutex_unlock(...)
The new code will trigger a lockdep rcu checking warning.
The fix is to change rcu_dereference() to rcu_dereference_check()
to prevent such a warning.
Reported-by: syzbot+6e72317008eef84a216b@syzkaller.appspotmail.com
Fixes: 394e40a29788 ("bpf: extend bpf_prog_array to store pointers to the cgroup storage")
Cc: Roman Gushchin <guro@fb.com>
Signed-off-by: Yonghong Song <yhs@fb.com>
---
kernel/bpf/core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 4d09e610777f..3f5bf1af0826 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -1579,7 +1579,7 @@ static bool bpf_prog_array_copy_core(struct bpf_prog_array __rcu *array,
struct bpf_prog_array_item *item;
int i = 0;
- item = rcu_dereference(array)->items;
+ item = rcu_dereference_check(array, 1)->items;
for (; item->prog; item++) {
if (item->prog == &dummy_bpf_prog.prog)
continue;
--
2.17.1
^ permalink raw reply related
* Re: [PATCH 1/1] NFC: Fix possible memory corruption when handling SHDLC I-Frame commands
From: Suren Baghdasaryan @ 2018-08-14 20:55 UTC (permalink / raw)
To: Kees Cook
Cc: Dan Carpenter, Security Officers, Kevin Deus, Samuel Ortiz,
David S. Miller, Allen Pais, linux-wireless, Network Development,
LKML
In-Reply-To: <CAGXu5jKqcpbpG0Ky4-yXV4d5q10HtGwJtdyPKcrwkJUPhA_Mpg@mail.gmail.com>
On Tue, Aug 14, 2018 at 1:33 PM, Kees Cook <keescook@chromium.org> wrote:
> On Tue, Aug 14, 2018 at 1:26 PM, Suren Baghdasaryan <surenb@google.com> wrote:
>> On Tue, Aug 14, 2018 at 9:57 AM, Suren Baghdasaryan <surenb@google.com> wrote:
>>> On Tue, Aug 14, 2018 at 2:54 AM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
>>>> Thanks. This is great. I'm so glad these are finally getting fixed.
>>>>
>>>> Do we need to fix nfc_hci_msg_rx_work() and nfc_hci_recv_from_llc() as
>>>> well? In nfc_hci_recv_from_llc() we allow pipe to be NFC_HCI_FRAGMENT
>>>> (0x7f) so that's one element beyond the end of the array and the
>>>> NFC_HCI_HCP_RESPONSE isn't checked.
>>>>
>>>> Also nci_hci_msg_rx_work() and nci_hci_data_received_cb() use
>>>> NCI_HCP_MSG_GET_PIPE() so those could be off by one.
>>>
>>> Good point. From hci.h:
>>>
>>> /*
>>> * According to specification 102 622 chapter 4.4 Pipes,
>>> * the pipe identifier is 7 bits long.
>>> */
>>> #define NFC_HCI_MAX_PIPES 127
>>>
>>> And then:
>>>
>>> struct nfc_hci_dev {
>>> ...
>>> struct nfc_hci_pipe pipes[NFC_HCI_MAX_PIPES];
>>> ...
>>> }
>>>
>>> I think the correct fix would be to change it to:
>>>
>>> struct nfc_hci_pipe pipes[NFC_HCI_MAX_PIPES + 1];
>>>
>>> What do you think?
>>>
>>
>> Just to be clear, this would fix the problem Dan described in his
>> reply and it should be implemented in a separate patch. The original
>> fix is still valid.
>
> I think you could merge the fixes into a single patch.
Couple reasons I would prefer to keep them separate:
- I feel that descriptions for these two issues should be different
and it's easier if we don't mix them up
- This one is already merged into Android kernels, so would be easier
to introduce the second fix separately
- I would like to give credit to people who noticed the problem (in
this case those are different people)
However if more people think we should fix both issues in the same
patch I'll happily do that.
Thanks!
>
> -Kees
>
> --
> Kees Cook
> Pixel Security
^ permalink raw reply
* Re: [PATCH v1 2/3] zinc: Introduce minimal cryptography library
From: Eric Biggers @ 2018-08-14 21:12 UTC (permalink / raw)
To: Jason A. Donenfeld
Cc: Eric Biggers, Linux Crypto Mailing List, LKML, Netdev,
David Miller, Andrew Lutomirski, Greg Kroah-Hartman, Samuel Neves,
Daniel J . Bernstein, Tanja Lange, Jean-Philippe Aumasson,
Karthikeyan Bhargavan
In-Reply-To: <CAHmME9pbwj0RaX4YM9epGOSgDRNUt=GA4UzO_iKcW=Md7kG6JQ@mail.gmail.com>
On Fri, Aug 03, 2018 at 04:33:50AM +0200, Jason A. Donenfeld wrote:
>
> > Also, earlier when I tested OpenSSL's ChaCha NEON implementation on ARM
> > Cortex-A7 it was actually quite a bit slower than the one in the Linux
> > kernel written by Ard Biesheuvel... I trust that when claiming the
> > performance of all implementations you're adding is "state-of-the-art
> > and unrivaled", you actually compared them to the ones already in the
> > Linux kernel which you're advocating replacing, right? :-)
>
> Yes, I have, and my results don't corroborate your findings. It will
> be interesting to get out a wider variety of hardware for comparisons.
> I suspect, also, that if the snarky emoticons subside, AndyP would be
> very interested in whatever we find and could have interest in
> improving implementations, should we actually find performance
> differences.
>
On ARM Cortex-A7, OpenSSL's ChaCha20 implementation is 13.9 cpb (cycles per
byte), whereas Linux's is faster: 11.9 cpb. I've also recently improved the
Linux implementation to 11.3 cpb and would like to send out a patch soon...
I've also written a scalar ChaCha20 implementation (no NEON instructions!) that
is 12.2 cpb on one block at a time on Cortex-A7, taking advantage of the free
rotates; that would be useful for the single permutation used to compute
XChaCha's subkey, and also for the ends of messages.
The reason Linux's ChaCha20 NEON implementation is faster than OpenSSL's is that
Linux's does 4 blocks at once using NEON instructions, and the words are
de-interleaved so the rows don't need to be shifted between each round.
OpenSSL's implementation, on the other hand, only does 3 blocks at once with
NEON instructions and has to shift the rows between each round. OpenSSL's
implementation also does a 4th block at the same time using regular ARM
instructions, but that doesn't help on Cortex-A7; it just makes it slower.
I understand there are tradeoffs, and different implementations can be faster on
different CPUs. Just know that from my point of view, switching to the OpenSSL
implementation actually introduces a performance regression, and we care a *lot*
about this since we need ChaCha to be absolutely as fast as possible for HPolyC
disk encryption. So if your proposal goes in, I'd likely need to write a patch
to get the old performance back, at least on Cortex-A7...
Also, I don't know whether Andy P. considered the 4xNEON implementation
technique. It could even be fastest on other ARM CPUs too, I don't know.
- Eric
^ permalink raw reply
* [PATCH] bluetooth/{bnep,cmtp,hidp}: Remove unnecessary smp_mb__{before,after}_atomic
From: Andrea Parri @ 2018-08-14 18:41 UTC (permalink / raw)
To: Marcel Holtmann, Johan Hedberg, David S . Miller
Cc: linux-bluetooth, netdev, linux-kernel, Jeffy Chen, Brian Norris,
AL Yu-Chen Cho, Andrea Parri
The barriers are unneeded; wait_woken() and woken_wake_function()
already provide us with the required synchronization: remove them
and document that we're relying on the (implicit) synchronization
provided by wait_woken() and woken_wake_function().
Signed-off-by: Andrea Parri <andrea.parri@amarulasolutions.com>
---
net/bluetooth/bnep/core.c | 7 ++++---
net/bluetooth/cmtp/core.c | 14 ++++++++------
net/bluetooth/hidp/core.c | 13 ++++++++-----
3 files changed, 20 insertions(+), 14 deletions(-)
diff --git a/net/bluetooth/bnep/core.c b/net/bluetooth/bnep/core.c
index 7b3965861013c..43c284158f63e 100644
--- a/net/bluetooth/bnep/core.c
+++ b/net/bluetooth/bnep/core.c
@@ -489,9 +489,6 @@ static int bnep_session(void *arg)
add_wait_queue(sk_sleep(sk), &wait);
while (1) {
- /* Ensure session->terminate is updated */
- smp_mb__before_atomic();
-
if (atomic_read(&s->terminate))
break;
/* RX */
@@ -512,6 +509,10 @@ static int bnep_session(void *arg)
break;
netif_wake_queue(dev);
+ /*
+ * wait_woken() performs the necessary memory barriers
+ * for us; see the header comment for this primitive.
+ */
wait_woken(&wait, TASK_INTERRUPTIBLE, MAX_SCHEDULE_TIMEOUT);
}
remove_wait_queue(sk_sleep(sk), &wait);
diff --git a/net/bluetooth/cmtp/core.c b/net/bluetooth/cmtp/core.c
index 7f26a5a19ff6d..07cfa3249f83a 100644
--- a/net/bluetooth/cmtp/core.c
+++ b/net/bluetooth/cmtp/core.c
@@ -288,9 +288,6 @@ static int cmtp_session(void *arg)
add_wait_queue(sk_sleep(sk), &wait);
while (1) {
- /* Ensure session->terminate is updated */
- smp_mb__before_atomic();
-
if (atomic_read(&session->terminate))
break;
if (sk->sk_state != BT_CONNECTED)
@@ -306,6 +303,10 @@ static int cmtp_session(void *arg)
cmtp_process_transmit(session);
+ /*
+ * wait_woken() performs the necessary memory barriers
+ * for us; see the header comment for this primitive.
+ */
wait_woken(&wait, TASK_INTERRUPTIBLE, MAX_SCHEDULE_TIMEOUT);
}
remove_wait_queue(sk_sleep(sk), &wait);
@@ -431,9 +432,10 @@ int cmtp_del_connection(struct cmtp_conndel_req *req)
/* Stop session thread */
atomic_inc(&session->terminate);
- /* Ensure session->terminate is updated */
- smp_mb__after_atomic();
-
+ /*
+ * See the comment preceding the call to wait_woken()
+ * in cmtp_session().
+ */
wake_up_interruptible(sk_sleep(session->sock->sk));
} else
err = -ENOENT;
diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
index 1036e4fa1ea28..e621551712f59 100644
--- a/net/bluetooth/hidp/core.c
+++ b/net/bluetooth/hidp/core.c
@@ -1074,6 +1074,10 @@ static int hidp_session_start_sync(struct hidp_session *session)
static void hidp_session_terminate(struct hidp_session *session)
{
atomic_inc(&session->terminate);
+ /*
+ * See the comment preceding the call to wait_woken()
+ * in hidp_session_run().
+ */
wake_up_interruptible(&hidp_session_wq);
}
@@ -1193,8 +1197,6 @@ static void hidp_session_run(struct hidp_session *session)
* thread is woken up by ->sk_state_changed().
*/
- /* Ensure session->terminate is updated */
- smp_mb__before_atomic();
if (atomic_read(&session->terminate))
break;
@@ -1228,14 +1230,15 @@ static void hidp_session_run(struct hidp_session *session)
hidp_process_transmit(session, &session->ctrl_transmit,
session->ctrl_sock);
+ /*
+ * wait_woken() performs the necessary memory barriers
+ * for us; see the header comment for this primitive.
+ */
wait_woken(&wait, TASK_INTERRUPTIBLE, MAX_SCHEDULE_TIMEOUT);
}
remove_wait_queue(&hidp_session_wq, &wait);
atomic_inc(&session->terminate);
-
- /* Ensure session->terminate is updated */
- smp_mb__after_atomic();
}
static int hidp_session_wake_function(wait_queue_entry_t *wait,
--
2.7.4
^ permalink raw reply related
* [PATCH net-next] net: sched: always disable bh when taking tcf_lock
From: Vlad Buslov @ 2018-08-14 18:46 UTC (permalink / raw)
To: netdev; +Cc: davem, jhs, xiyou.wangcong, jiri, ast, daniel, Vlad Buslov
Recently, ops->init() and ops->dump() of all actions were modified to
always obtain tcf_lock when accessing private action state. Actions that
don't depend on tcf_lock for synchronization with their data path use
non-bh locking API. However, tcf_lock is also used to protect rate
estimator stats in softirq context by timer callback.
Change ops->init() and ops->dump() of all actions to disable bh when using
tcf_lock to prevent deadlock reported by following lockdep warning:
[ 105.470398] ================================
[ 105.475014] WARNING: inconsistent lock state
[ 105.479628] 4.18.0-rc8+ #664 Not tainted
[ 105.483897] --------------------------------
[ 105.488511] inconsistent {SOFTIRQ-ON-W} -> {IN-SOFTIRQ-W} usage.
[ 105.494871] swapper/16/0 [HC0[0]:SC1[1]:HE1:SE0] takes:
[ 105.500449] 00000000f86c012e (&(&p->tcfa_lock)->rlock){+.?.}, at: est_fetch_counters+0x3c/0xa0
[ 105.509696] {SOFTIRQ-ON-W} state was registered at:
[ 105.514925] _raw_spin_lock+0x2c/0x40
[ 105.519022] tcf_bpf_init+0x579/0x820 [act_bpf]
[ 105.523990] tcf_action_init_1+0x4e4/0x660
[ 105.528518] tcf_action_init+0x1ce/0x2d0
[ 105.532880] tcf_exts_validate+0x1d8/0x200
[ 105.537416] fl_change+0x55a/0x268b [cls_flower]
[ 105.542469] tc_new_tfilter+0x748/0xa20
[ 105.546738] rtnetlink_rcv_msg+0x56a/0x6d0
[ 105.551268] netlink_rcv_skb+0x18d/0x200
[ 105.555628] netlink_unicast+0x2d0/0x370
[ 105.559990] netlink_sendmsg+0x3b9/0x6a0
[ 105.564349] sock_sendmsg+0x6b/0x80
[ 105.568271] ___sys_sendmsg+0x4a1/0x520
[ 105.572547] __sys_sendmsg+0xd7/0x150
[ 105.576655] do_syscall_64+0x72/0x2c0
[ 105.580757] entry_SYSCALL_64_after_hwframe+0x49/0xbe
[ 105.586243] irq event stamp: 489296
[ 105.590084] hardirqs last enabled at (489296): [<ffffffffb507e639>] _raw_spin_unlock_irq+0x29/0x40
[ 105.599765] hardirqs last disabled at (489295): [<ffffffffb507e745>] _raw_spin_lock_irq+0x15/0x50
[ 105.609277] softirqs last enabled at (489292): [<ffffffffb413a6a3>] irq_enter+0x83/0xa0
[ 105.618001] softirqs last disabled at (489293): [<ffffffffb413a800>] irq_exit+0x140/0x190
[ 105.626813]
other info that might help us debug this:
[ 105.633976] Possible unsafe locking scenario:
[ 105.640526] CPU0
[ 105.643325] ----
[ 105.646125] lock(&(&p->tcfa_lock)->rlock);
[ 105.650747] <Interrupt>
[ 105.653717] lock(&(&p->tcfa_lock)->rlock);
[ 105.658514]
*** DEADLOCK ***
[ 105.665349] 1 lock held by swapper/16/0:
[ 105.669629] #0: 00000000a640ad99 ((&est->timer)){+.-.}, at: call_timer_fn+0x10b/0x550
[ 105.678200]
stack backtrace:
[ 105.683194] CPU: 16 PID: 0 Comm: swapper/16 Not tainted 4.18.0-rc8+ #664
[ 105.690249] Hardware name: Supermicro SYS-2028TP-DECR/X10DRT-P, BIOS 2.0b 03/30/2017
[ 105.698626] Call Trace:
[ 105.701421] <IRQ>
[ 105.703791] dump_stack+0x92/0xeb
[ 105.707461] print_usage_bug+0x336/0x34c
[ 105.711744] mark_lock+0x7c9/0x980
[ 105.715500] ? print_shortest_lock_dependencies+0x2e0/0x2e0
[ 105.721424] ? check_usage_forwards+0x230/0x230
[ 105.726315] __lock_acquire+0x923/0x26f0
[ 105.730597] ? debug_show_all_locks+0x240/0x240
[ 105.735478] ? mark_lock+0x493/0x980
[ 105.739412] ? check_chain_key+0x140/0x1f0
[ 105.743861] ? __lock_acquire+0x836/0x26f0
[ 105.748323] ? lock_acquire+0x12e/0x290
[ 105.752516] lock_acquire+0x12e/0x290
[ 105.756539] ? est_fetch_counters+0x3c/0xa0
[ 105.761084] _raw_spin_lock+0x2c/0x40
[ 105.765099] ? est_fetch_counters+0x3c/0xa0
[ 105.769633] est_fetch_counters+0x3c/0xa0
[ 105.773995] est_timer+0x87/0x390
[ 105.777670] ? est_fetch_counters+0xa0/0xa0
[ 105.782210] ? lock_acquire+0x12e/0x290
[ 105.786410] call_timer_fn+0x161/0x550
[ 105.790512] ? est_fetch_counters+0xa0/0xa0
[ 105.795055] ? del_timer_sync+0xd0/0xd0
[ 105.799249] ? __lock_is_held+0x93/0x110
[ 105.803531] ? mark_held_locks+0x20/0xe0
[ 105.807813] ? _raw_spin_unlock_irq+0x29/0x40
[ 105.812525] ? est_fetch_counters+0xa0/0xa0
[ 105.817069] ? est_fetch_counters+0xa0/0xa0
[ 105.821610] run_timer_softirq+0x3c4/0x9f0
[ 105.826064] ? lock_acquire+0x12e/0x290
[ 105.830257] ? __bpf_trace_timer_class+0x10/0x10
[ 105.835237] ? __lock_is_held+0x25/0x110
[ 105.839517] __do_softirq+0x11d/0x7bf
[ 105.843542] irq_exit+0x140/0x190
[ 105.847208] smp_apic_timer_interrupt+0xac/0x3b0
[ 105.852182] apic_timer_interrupt+0xf/0x20
[ 105.856628] </IRQ>
[ 105.859081] RIP: 0010:cpuidle_enter_state+0xd8/0x4d0
[ 105.864395] Code: 46 ff 48 89 44 24 08 0f 1f 44 00 00 31 ff e8 cf ec 46 ff 80 7c 24 07 00 0f 85 1d 02 00 00 e8 9f 90 4b ff fb 66 0f 1f 44 00 00 <4c> 8b 6c 24 08 4d 29 fd 0f 80 36 03 00 00 4c 89 e8 48 ba cf f7 53
[ 105.884288] RSP: 0018:ffff8803ad94fd20 EFLAGS: 00000246 ORIG_RAX: ffffffffffffff13
[ 105.892494] RAX: 0000000000000000 RBX: ffffe8fb300829c0 RCX: ffffffffb41e19e1
[ 105.899988] RDX: 0000000000000007 RSI: dffffc0000000000 RDI: ffff8803ad9358ac
[ 105.907503] RBP: ffffffffb6636300 R08: 0000000000000004 R09: 0000000000000000
[ 105.914997] R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000004
[ 105.922487] R13: ffffffffb6636140 R14: ffffffffb66362d8 R15: 000000188d36091b
[ 105.929988] ? trace_hardirqs_on_caller+0x141/0x2d0
[ 105.935232] do_idle+0x28e/0x320
[ 105.938817] ? arch_cpu_idle_exit+0x40/0x40
[ 105.943361] ? mark_lock+0x8c1/0x980
[ 105.947295] ? _raw_spin_unlock_irqrestore+0x32/0x60
[ 105.952619] cpu_startup_entry+0xc2/0xd0
[ 105.956900] ? cpu_in_idle+0x20/0x20
[ 105.960830] ? _raw_spin_unlock_irqrestore+0x32/0x60
[ 105.966146] ? trace_hardirqs_on_caller+0x141/0x2d0
[ 105.971391] start_secondary+0x2b5/0x360
[ 105.975669] ? set_cpu_sibling_map+0x1330/0x1330
[ 105.980654] secondary_startup_64+0xa5/0xb0
Taking tcf_lock in sample action with bh disabled causes lockdep to issue a
warning regarding possible irq lock inversion dependency between tcf_lock,
and psample_groups_lock that is taken when holding tcf_lock in sample init:
[ 162.108959] Possible interrupt unsafe locking scenario:
[ 162.116386] CPU0 CPU1
[ 162.121277] ---- ----
[ 162.126162] lock(psample_groups_lock);
[ 162.130447] local_irq_disable();
[ 162.136772] lock(&(&p->tcfa_lock)->rlock);
[ 162.143957] lock(psample_groups_lock);
[ 162.150813] <Interrupt>
[ 162.153808] lock(&(&p->tcfa_lock)->rlock);
[ 162.158608]
*** DEADLOCK ***
In order to prevent potential lock inversion dependency between tcf_lock
and psample_groups_lock, extract call to psample_group_get() from tcf_lock
protected section in sample action init function.
Fixes: 4e232818bd32 ("net: sched: act_mirred: remove dependency on rtnl lock")
Fixes: 764e9a24480f ("net: sched: act_vlan: remove dependency on rtnl lock")
Fixes: 729e01260989 ("net: sched: act_tunnel_key: remove dependency on rtnl lock")
Fixes: d77284956656 ("net: sched: act_sample: remove dependency on rtnl lock")
Fixes: e8917f437006 ("net: sched: act_gact: remove dependency on rtnl lock")
Fixes: b6a2b971c0b0 ("net: sched: act_csum: remove dependency on rtnl lock")
Fixes: 2142236b4584 ("net: sched: act_bpf: remove dependency on rtnl lock")
Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
---
net/sched/act_bpf.c | 10 +++++-----
net/sched/act_csum.c | 10 +++++-----
net/sched/act_gact.c | 10 +++++-----
net/sched/act_mirred.c | 16 ++++++++--------
net/sched/act_sample.c | 25 ++++++++++++++-----------
net/sched/act_tunnel_key.c | 10 +++++-----
net/sched/act_vlan.c | 10 +++++-----
7 files changed, 47 insertions(+), 44 deletions(-)
diff --git a/net/sched/act_bpf.c b/net/sched/act_bpf.c
index 9e8a33f9fee3..3d56ab69614c 100644
--- a/net/sched/act_bpf.c
+++ b/net/sched/act_bpf.c
@@ -147,7 +147,7 @@ static int tcf_bpf_dump(struct sk_buff *skb, struct tc_action *act,
struct tcf_t tm;
int ret;
- spin_lock(&prog->tcf_lock);
+ spin_lock_bh(&prog->tcf_lock);
opt.action = prog->tcf_action;
if (nla_put(skb, TCA_ACT_BPF_PARMS, sizeof(opt), &opt))
goto nla_put_failure;
@@ -164,11 +164,11 @@ static int tcf_bpf_dump(struct sk_buff *skb, struct tc_action *act,
TCA_ACT_BPF_PAD))
goto nla_put_failure;
- spin_unlock(&prog->tcf_lock);
+ spin_unlock_bh(&prog->tcf_lock);
return skb->len;
nla_put_failure:
- spin_unlock(&prog->tcf_lock);
+ spin_unlock_bh(&prog->tcf_lock);
nlmsg_trim(skb, tp);
return -1;
}
@@ -340,7 +340,7 @@ static int tcf_bpf_init(struct net *net, struct nlattr *nla,
prog = to_bpf(*act);
- spin_lock(&prog->tcf_lock);
+ spin_lock_bh(&prog->tcf_lock);
if (res != ACT_P_CREATED)
tcf_bpf_prog_fill_cfg(prog, &old);
@@ -352,7 +352,7 @@ static int tcf_bpf_init(struct net *net, struct nlattr *nla,
prog->tcf_action = parm->action;
rcu_assign_pointer(prog->filter, cfg.filter);
- spin_unlock(&prog->tcf_lock);
+ spin_unlock_bh(&prog->tcf_lock);
if (res == ACT_P_CREATED) {
tcf_idr_insert(tn, *act);
diff --git a/net/sched/act_csum.c b/net/sched/act_csum.c
index f01c59ba6d12..422fdcd9b196 100644
--- a/net/sched/act_csum.c
+++ b/net/sched/act_csum.c
@@ -96,11 +96,11 @@ static int tcf_csum_init(struct net *net, struct nlattr *nla,
}
params_new->update_flags = parm->update_flags;
- spin_lock(&p->tcf_lock);
+ spin_lock_bh(&p->tcf_lock);
p->tcf_action = parm->action;
rcu_swap_protected(p->params, params_new,
lockdep_is_held(&p->tcf_lock));
- spin_unlock(&p->tcf_lock);
+ spin_unlock_bh(&p->tcf_lock);
if (params_new)
kfree_rcu(params_new, rcu);
@@ -604,7 +604,7 @@ static int tcf_csum_dump(struct sk_buff *skb, struct tc_action *a, int bind,
};
struct tcf_t t;
- spin_lock(&p->tcf_lock);
+ spin_lock_bh(&p->tcf_lock);
params = rcu_dereference_protected(p->params,
lockdep_is_held(&p->tcf_lock));
opt.action = p->tcf_action;
@@ -616,12 +616,12 @@ static int tcf_csum_dump(struct sk_buff *skb, struct tc_action *a, int bind,
tcf_tm_dump(&t, &p->tcf_tm);
if (nla_put_64bit(skb, TCA_CSUM_TM, sizeof(t), &t, TCA_CSUM_PAD))
goto nla_put_failure;
- spin_unlock(&p->tcf_lock);
+ spin_unlock_bh(&p->tcf_lock);
return skb->len;
nla_put_failure:
- spin_unlock(&p->tcf_lock);
+ spin_unlock_bh(&p->tcf_lock);
nlmsg_trim(skb, b);
return -1;
}
diff --git a/net/sched/act_gact.c b/net/sched/act_gact.c
index bfccd34a3968..e5fd620c034a 100644
--- a/net/sched/act_gact.c
+++ b/net/sched/act_gact.c
@@ -113,7 +113,7 @@ static int tcf_gact_init(struct net *net, struct nlattr *nla,
gact = to_gact(*a);
- spin_lock(&gact->tcf_lock);
+ spin_lock_bh(&gact->tcf_lock);
gact->tcf_action = parm->action;
#ifdef CONFIG_GACT_PROB
if (p_parm) {
@@ -126,7 +126,7 @@ static int tcf_gact_init(struct net *net, struct nlattr *nla,
gact->tcfg_ptype = p_parm->ptype;
}
#endif
- spin_unlock(&gact->tcf_lock);
+ spin_unlock_bh(&gact->tcf_lock);
if (ret == ACT_P_CREATED)
tcf_idr_insert(tn, *a);
@@ -183,7 +183,7 @@ static int tcf_gact_dump(struct sk_buff *skb, struct tc_action *a,
};
struct tcf_t t;
- spin_lock(&gact->tcf_lock);
+ spin_lock_bh(&gact->tcf_lock);
opt.action = gact->tcf_action;
if (nla_put(skb, TCA_GACT_PARMS, sizeof(opt), &opt))
goto nla_put_failure;
@@ -202,12 +202,12 @@ static int tcf_gact_dump(struct sk_buff *skb, struct tc_action *a,
tcf_tm_dump(&t, &gact->tcf_tm);
if (nla_put_64bit(skb, TCA_GACT_TM, sizeof(t), &t, TCA_GACT_PAD))
goto nla_put_failure;
- spin_unlock(&gact->tcf_lock);
+ spin_unlock_bh(&gact->tcf_lock);
return skb->len;
nla_put_failure:
- spin_unlock(&gact->tcf_lock);
+ spin_unlock_bh(&gact->tcf_lock);
nlmsg_trim(skb, b);
return -1;
}
diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
index 327be257033d..93138c61f8d1 100644
--- a/net/sched/act_mirred.c
+++ b/net/sched/act_mirred.c
@@ -159,14 +159,14 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
}
m = to_mirred(*a);
- spin_lock(&m->tcf_lock);
+ spin_lock_bh(&m->tcf_lock);
m->tcf_action = parm->action;
m->tcfm_eaction = parm->eaction;
if (parm->ifindex) {
dev = dev_get_by_index(net, parm->ifindex);
if (!dev) {
- spin_unlock(&m->tcf_lock);
+ spin_unlock_bh(&m->tcf_lock);
tcf_idr_release(*a, bind);
return -ENODEV;
}
@@ -177,7 +177,7 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
dev_put(dev);
m->tcfm_mac_header_xmit = mac_header_xmit;
}
- spin_unlock(&m->tcf_lock);
+ spin_unlock_bh(&m->tcf_lock);
if (ret == ACT_P_CREATED) {
spin_lock(&mirred_list_lock);
@@ -305,7 +305,7 @@ static int tcf_mirred_dump(struct sk_buff *skb, struct tc_action *a, int bind,
struct net_device *dev;
struct tcf_t t;
- spin_lock(&m->tcf_lock);
+ spin_lock_bh(&m->tcf_lock);
opt.action = m->tcf_action;
opt.eaction = m->tcfm_eaction;
dev = tcf_mirred_dev_dereference(m);
@@ -318,12 +318,12 @@ static int tcf_mirred_dump(struct sk_buff *skb, struct tc_action *a, int bind,
tcf_tm_dump(&t, &m->tcf_tm);
if (nla_put_64bit(skb, TCA_MIRRED_TM, sizeof(t), &t, TCA_MIRRED_PAD))
goto nla_put_failure;
- spin_unlock(&m->tcf_lock);
+ spin_unlock_bh(&m->tcf_lock);
return skb->len;
nla_put_failure:
- spin_unlock(&m->tcf_lock);
+ spin_unlock_bh(&m->tcf_lock);
nlmsg_trim(skb, b);
return -1;
}
@@ -356,7 +356,7 @@ static int mirred_device_event(struct notifier_block *unused,
if (event == NETDEV_UNREGISTER) {
spin_lock(&mirred_list_lock);
list_for_each_entry(m, &mirred_list, tcfm_list) {
- spin_lock(&m->tcf_lock);
+ spin_lock_bh(&m->tcf_lock);
if (tcf_mirred_dev_dereference(m) == dev) {
dev_put(dev);
/* Note : no rcu grace period necessary, as
@@ -364,7 +364,7 @@ static int mirred_device_event(struct notifier_block *unused,
*/
RCU_INIT_POINTER(m->tcfm_dev, NULL);
}
- spin_unlock(&m->tcf_lock);
+ spin_unlock_bh(&m->tcf_lock);
}
spin_unlock(&mirred_list_lock);
}
diff --git a/net/sched/act_sample.c b/net/sched/act_sample.c
index 81071afe1b43..207b4132d1b0 100644
--- a/net/sched/act_sample.c
+++ b/net/sched/act_sample.c
@@ -44,6 +44,7 @@ static int tcf_sample_init(struct net *net, struct nlattr *nla,
struct nlattr *tb[TCA_SAMPLE_MAX + 1];
struct psample_group *psample_group;
struct tc_sample *parm;
+ u32 psample_group_num;
struct tcf_sample *s;
bool exists = false;
int ret, err;
@@ -78,25 +79,27 @@ static int tcf_sample_init(struct net *net, struct nlattr *nla,
tcf_idr_release(*a, bind);
return -EEXIST;
}
- s = to_sample(*a);
- spin_lock(&s->tcf_lock);
- s->tcf_action = parm->action;
- s->rate = nla_get_u32(tb[TCA_SAMPLE_RATE]);
- s->psample_group_num = nla_get_u32(tb[TCA_SAMPLE_PSAMPLE_GROUP]);
- psample_group = psample_group_get(net, s->psample_group_num);
+ psample_group_num = nla_get_u32(tb[TCA_SAMPLE_PSAMPLE_GROUP]);
+ psample_group = psample_group_get(net, psample_group_num);
if (!psample_group) {
- spin_unlock(&s->tcf_lock);
tcf_idr_release(*a, bind);
return -ENOMEM;
}
+
+ s = to_sample(*a);
+
+ spin_lock_bh(&s->tcf_lock);
+ s->tcf_action = parm->action;
+ s->rate = nla_get_u32(tb[TCA_SAMPLE_RATE]);
+ s->psample_group_num = psample_group_num;
RCU_INIT_POINTER(s->psample_group, psample_group);
if (tb[TCA_SAMPLE_TRUNC_SIZE]) {
s->truncate = true;
s->trunc_size = nla_get_u32(tb[TCA_SAMPLE_TRUNC_SIZE]);
}
- spin_unlock(&s->tcf_lock);
+ spin_unlock_bh(&s->tcf_lock);
if (ret == ACT_P_CREATED)
tcf_idr_insert(tn, *a);
@@ -183,7 +186,7 @@ static int tcf_sample_dump(struct sk_buff *skb, struct tc_action *a,
};
struct tcf_t t;
- spin_lock(&s->tcf_lock);
+ spin_lock_bh(&s->tcf_lock);
opt.action = s->tcf_action;
if (nla_put(skb, TCA_SAMPLE_PARMS, sizeof(opt), &opt))
goto nla_put_failure;
@@ -201,12 +204,12 @@ static int tcf_sample_dump(struct sk_buff *skb, struct tc_action *a,
if (nla_put_u32(skb, TCA_SAMPLE_PSAMPLE_GROUP, s->psample_group_num))
goto nla_put_failure;
- spin_unlock(&s->tcf_lock);
+ spin_unlock_bh(&s->tcf_lock);
return skb->len;
nla_put_failure:
- spin_unlock(&s->tcf_lock);
+ spin_unlock_bh(&s->tcf_lock);
nlmsg_trim(skb, b);
return -1;
}
diff --git a/net/sched/act_tunnel_key.c b/net/sched/act_tunnel_key.c
index ba2ae9f75ef5..8f09cf08d8fe 100644
--- a/net/sched/act_tunnel_key.c
+++ b/net/sched/act_tunnel_key.c
@@ -354,11 +354,11 @@ static int tunnel_key_init(struct net *net, struct nlattr *nla,
params_new->tcft_action = parm->t_action;
params_new->tcft_enc_metadata = metadata;
- spin_lock(&t->tcf_lock);
+ spin_lock_bh(&t->tcf_lock);
t->tcf_action = parm->action;
rcu_swap_protected(t->params, params_new,
lockdep_is_held(&t->tcf_lock));
- spin_unlock(&t->tcf_lock);
+ spin_unlock_bh(&t->tcf_lock);
if (params_new)
kfree_rcu(params_new, rcu);
@@ -485,7 +485,7 @@ static int tunnel_key_dump(struct sk_buff *skb, struct tc_action *a,
};
struct tcf_t tm;
- spin_lock(&t->tcf_lock);
+ spin_lock_bh(&t->tcf_lock);
params = rcu_dereference_protected(t->params,
lockdep_is_held(&t->tcf_lock));
opt.action = t->tcf_action;
@@ -520,12 +520,12 @@ static int tunnel_key_dump(struct sk_buff *skb, struct tc_action *a,
if (nla_put_64bit(skb, TCA_TUNNEL_KEY_TM, sizeof(tm),
&tm, TCA_TUNNEL_KEY_PAD))
goto nla_put_failure;
- spin_unlock(&t->tcf_lock);
+ spin_unlock_bh(&t->tcf_lock);
return skb->len;
nla_put_failure:
- spin_unlock(&t->tcf_lock);
+ spin_unlock_bh(&t->tcf_lock);
nlmsg_trim(skb, b);
return -1;
}
diff --git a/net/sched/act_vlan.c b/net/sched/act_vlan.c
index 5bde17fe3608..1470488a3fd0 100644
--- a/net/sched/act_vlan.c
+++ b/net/sched/act_vlan.c
@@ -213,10 +213,10 @@ static int tcf_vlan_init(struct net *net, struct nlattr *nla,
p->tcfv_push_prio = push_prio;
p->tcfv_push_proto = push_proto;
- spin_lock(&v->tcf_lock);
+ spin_lock_bh(&v->tcf_lock);
v->tcf_action = parm->action;
rcu_swap_protected(v->vlan_p, p, lockdep_is_held(&v->tcf_lock));
- spin_unlock(&v->tcf_lock);
+ spin_unlock_bh(&v->tcf_lock);
if (p)
kfree_rcu(p, rcu);
@@ -249,7 +249,7 @@ static int tcf_vlan_dump(struct sk_buff *skb, struct tc_action *a,
};
struct tcf_t t;
- spin_lock(&v->tcf_lock);
+ spin_lock_bh(&v->tcf_lock);
opt.action = v->tcf_action;
p = rcu_dereference_protected(v->vlan_p, lockdep_is_held(&v->tcf_lock));
opt.v_action = p->tcfv_action;
@@ -268,12 +268,12 @@ static int tcf_vlan_dump(struct sk_buff *skb, struct tc_action *a,
tcf_tm_dump(&t, &v->tcf_tm);
if (nla_put_64bit(skb, TCA_VLAN_TM, sizeof(t), &t, TCA_VLAN_PAD))
goto nla_put_failure;
- spin_unlock(&v->tcf_lock);
+ spin_unlock_bh(&v->tcf_lock);
return skb->len;
nla_put_failure:
- spin_unlock(&v->tcf_lock);
+ spin_unlock_bh(&v->tcf_lock);
nlmsg_trim(skb, b);
return -1;
}
--
2.7.5
^ permalink raw reply related
* Re: [PATCH next-queue 3/8] ixgbe: add VF ipsec management
From: Shannon Nelson @ 2018-08-14 18:55 UTC (permalink / raw)
To: kbuild test robot
Cc: kbuild-all, intel-wired-lan, jeffrey.t.kirsher, steffen.klassert,
netdev
In-Reply-To: <201808141311.4srvjTmi%fengguang.wu@intel.com>
On 8/13/2018 10:31 PM, kbuild test robot wrote:
> Hi Shannon,
>
> Thank you for the patch! Yet something to improve:
>
> [auto build test ERROR on jkirsher-next-queue/dev-queue]
> [also build test ERROR on v4.18 next-20180813]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
>
> url: https://github.com/0day-ci/linux/commits/Shannon-Nelson/ixgbe-ixgbevf-IPsec-offload-support-for-VFs/20180814-074800
> base: https://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/next-queue.git dev-queue
> config: x86_64-randconfig-v0-08131550 (attached as .config)
> compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
> reproduce:
> # save the attached .config to linux build tree
> make ARCH=x86_64
>
> All errors (new ones prefixed by >>):
>
> drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.o: In function `ixgbe_ipsec_vf_add_sa':
>>> drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c:917: undefined reference to `xfrm_aead_get_byname'
> make[1]: *** [vmlinux] Error 1
> make[1]: Target '_all' not remade because of errors.
>
Huh, odd. I'm not able to reproduce this error using your config file
in a net-next tree of vintage v4.18-rc8 or in Jeff's dev-queue branch.
It looks like I'm using an older compiler (4.8.5) but that shouldn't
make a difference here.
sln
^ permalink raw reply
* Re: [PATCH 1/1] NFC: Fix possible memory corruption when handling SHDLC I-Frame commands
From: Kees Cook @ 2018-08-14 21:49 UTC (permalink / raw)
To: Suren Baghdasaryan
Cc: Dan Carpenter, Security Officers, Kevin Deus, Samuel Ortiz,
David S. Miller, Allen Pais, linux-wireless, Network Development,
LKML
In-Reply-To: <CAJuCfpEX69Sbd=0i0ismP6q2NsZGUF4gMSCr1G5NdXd3n1NyZA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Tue, Aug 14, 2018 at 1:55 PM, Suren Baghdasaryan <surenb-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> wrote:
> On Tue, Aug 14, 2018 at 1:33 PM, Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
>> On Tue, Aug 14, 2018 at 1:26 PM, Suren Baghdasaryan <surenb-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> wrote:
>>> On Tue, Aug 14, 2018 at 9:57 AM, Suren Baghdasaryan <surenb-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> wrote:
>>>> On Tue, Aug 14, 2018 at 2:54 AM, Dan Carpenter <dan.carpenter-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> wrote:
>>>>> Thanks. This is great. I'm so glad these are finally getting fixed.
>>>>>
>>>>> Do we need to fix nfc_hci_msg_rx_work() and nfc_hci_recv_from_llc() as
>>>>> well? In nfc_hci_recv_from_llc() we allow pipe to be NFC_HCI_FRAGMENT
>>>>> (0x7f) so that's one element beyond the end of the array and the
>>>>> NFC_HCI_HCP_RESPONSE isn't checked.
>>>>>
>>>>> Also nci_hci_msg_rx_work() and nci_hci_data_received_cb() use
>>>>> NCI_HCP_MSG_GET_PIPE() so those could be off by one.
>>>>
>>>> Good point. From hci.h:
>>>>
>>>> /*
>>>> * According to specification 102 622 chapter 4.4 Pipes,
>>>> * the pipe identifier is 7 bits long.
>>>> */
>>>> #define NFC_HCI_MAX_PIPES 127
>>>>
>>>> And then:
>>>>
>>>> struct nfc_hci_dev {
>>>> ...
>>>> struct nfc_hci_pipe pipes[NFC_HCI_MAX_PIPES];
>>>> ...
>>>> }
>>>>
>>>> I think the correct fix would be to change it to:
>>>>
>>>> struct nfc_hci_pipe pipes[NFC_HCI_MAX_PIPES + 1];
>>>>
>>>> What do you think?
>>>>
>>>
>>> Just to be clear, this would fix the problem Dan described in his
>>> reply and it should be implemented in a separate patch. The original
>>> fix is still valid.
>>
>> I think you could merge the fixes into a single patch.
>
> Couple reasons I would prefer to keep them separate:
> - I feel that descriptions for these two issues should be different
> and it's easier if we don't mix them up
> - This one is already merged into Android kernels, so would be easier
> to introduce the second fix separately
> - I would like to give credit to people who noticed the problem (in
> this case those are different people)
>
> However if more people think we should fix both issues in the same
> patch I'll happily do that.
> Thanks!
If it's already landed separately somewhere else, then yeah, 2 patches
sounds good. No objection either way from me!
-Kees
--
Kees Cook
Pixel Security
^ 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