* [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] 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
* 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 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] 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] 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: [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
* [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: [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
* 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-next] net: dsa: mv88e6xxx: bitwise vs logical bug
From: David Miller @ 2018-08-14 17:05 UTC (permalink / raw)
To: dan.carpenter; +Cc: andrew, vivien.didelot, f.fainelli, netdev, kernel-janitors
In-Reply-To: <20180814090643.ilis47jhsaa2sbhi@kili.mountain>
From: Dan Carpenter <dan.carpenter@oracle.com>
Date: Tue, 14 Aug 2018 12:06:43 +0300
> We are trying to test if these flags are set but there are some && vs &
> typos.
>
> Fixes: efd1ba6af93f ("net: dsa: mv88e6xxx: Add SERDES phydev_mac_change up for 6390")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Applied.
^ permalink raw reply
* Re: [PATCH] 9p/xen: fix check for xenbus_read error in front_probe
From: Stefano Stabellini @ 2018-08-14 17:03 UTC (permalink / raw)
To: Dominique Martinet
Cc: v9fs-developer, Dominique Martinet, netdev, Stefano Stabellini,
Eric Van Hensbergen, Latchesar Ionkov
In-Reply-To: <1534236007-10170-1-git-send-email-asmadeus@codewreck.org>
On Tue, 14 Aug 2018, Dominique Martinet wrote:
> From: Dominique Martinet <dominique.martinet@cea.fr>
>
> If the xen bus exists but does not expose the proper interface, it is
> possible to get a non-zero length but still some error, leading to
> strcmp failing trying to load invalid memory addresses e.g.
> fffffffffffffffe.
>
> There is then no need to check length when there is no error, as the
> xenbus driver guarantees that the string is nul-terminated.
>
> Signed-off-by: Dominique Martinet <dominique.martinet@cea.fr>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Eric Van Hensbergen <ericvh@gmail.com>
> Cc: Latchesar Ionkov <lucho@ionkov.net>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
> ---
>
> This is a trivial bug I stumbled on when setting up xen with p9fs and
> running the VM in pvm: it had enough in the bus to trigger the probe
> but then there was no version and it tried to return ENOENT but len
> was set to the lower-level message size.
>
> net/9p/trans_xen.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/net/9p/trans_xen.c b/net/9p/trans_xen.c
> index 1a5b38892eb4..f76beadddfc3 100644
> --- a/net/9p/trans_xen.c
> +++ b/net/9p/trans_xen.c
> @@ -391,9 +391,9 @@ static int xen_9pfs_front_probe(struct xenbus_device *dev,
> unsigned int max_rings, max_ring_order, len = 0;
>
> versions = xenbus_read(XBT_NIL, dev->otherend, "versions", &len);
> - if (!len)
> - return -EINVAL;
> + if (IS_ERR(versions))
> + return PTR_ERR(versions);
> if (strcmp(versions, "1")) {
> kfree(versions);
> return -EINVAL;
> }
> --
> 2.17.1
>
^ permalink raw reply
* Re: [PATCH net-next] ieee802154: hwsim: using right kind of iteration
From: David Miller @ 2018-08-14 16:59 UTC (permalink / raw)
To: aring; +Cc: netdev, linux-wpan, kernel, stefan
In-Reply-To: <20180812202456.22317-1-aring@mojatatu.com>
From: Alexander Aring <aring@mojatatu.com>
Date: Sun, 12 Aug 2018 16:24:56 -0400
> This patch fixes the error path to unsubscribe all other phy's from
> current phy. The actually code using a wrong kind of list iteration may
> copied from the case to unsubscribe the current phy from all other
> phy's.
>
> Cc: Stefan Schmidt <stefan@datenfreihafen.org>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Fixes: f25da51fdc38 ("ieee802154: hwsim: add replacement for fakelb")
> Signed-off-by: Alexander Aring <aring@mojatatu.com>
I'll apply this directly, thanks.
^ permalink raw reply
* Re: [PATCH] rds: fix building with IPV6=m
From: Greg Thelen @ 2018-08-14 19:15 UTC (permalink / raw)
To: Arnd Bergmann, Santosh Shilimkar, David S. Miller
Cc: Arnd Bergmann, Anders Roxell, Ka-Cheong Poon, Stephen Hemminger,
netdev, linux-rdma, rds-devel, linux-kernel
In-Reply-To: <20180814090752.1681486-1-arnd@arndb.de>
Arnd Bergmann <arnd@arndb.de> wrote:
> When CONFIG_RDS_TCP is built-in and CONFIG_IPV6 is a loadable
> module, we get a link error agains the modular ipv6_chk_addr()
> function:
>
> net/rds/tcp.o: In function `rds_tcp_laddr_check':
> tcp.c:(.text+0x3b2): undefined reference to `ipv6_chk_addr'
>
> This adds back a dependency that forces RDS_TCP to also be
> a loadable module when IPV6 is one.
>
> Fixes: e65d4d96334e ("rds: Remove IPv6 dependency")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Acked-by: Greg Thelen <gthelenq@google.com>
^ permalink raw reply
* [PATCH V2 net-next 5/8] net: hns3: Fix desc num set to default when setting channel
From: Salil Mehta @ 2018-08-14 16:13 UTC (permalink / raw)
To: davem
Cc: salil.mehta, yisen.zhuang, lipeng321, mehta.salil.lnk, netdev,
linux-kernel, linuxarm, Yunsheng Lin
In-Reply-To: <20180814161319.16392-1-salil.mehta@huawei.com>
From: Yunsheng Lin <linyunsheng@huawei.com>
When user set the channel num using "ethtool -L ethX", the desc num
of BD will set to default value, which will cause desc num set by
user lost problem.
This patch fixes it by restoring the desc num set by user when setting
channel num.
Fixes: 09f2af6405b8 ("net: hns3: add support to modify tqps number")
Signed-off-by: Yunsheng Lin <linyunsheng@huawei.com>
Signed-off-by: Peng Li <lipeng321@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
.../ethernet/hisilicon/hns3/hns3pf/hclge_main.c | 22 ++++++++++++----------
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
index 2e9c8b9..8577dfc 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
@@ -1267,35 +1267,37 @@ static int hclge_map_tqps_to_func(struct hclge_dev *hdev, u16 func_id,
return ret;
}
-static int hclge_assign_tqp(struct hclge_vport *vport,
- struct hnae3_queue **tqp, u16 num_tqps)
+static int hclge_assign_tqp(struct hclge_vport *vport)
{
+ struct hnae3_knic_private_info *kinfo = &vport->nic.kinfo;
struct hclge_dev *hdev = vport->back;
int i, alloced;
for (i = 0, alloced = 0; i < hdev->num_tqps &&
- alloced < num_tqps; i++) {
+ alloced < kinfo->num_tqps; i++) {
if (!hdev->htqp[i].alloced) {
hdev->htqp[i].q.handle = &vport->nic;
hdev->htqp[i].q.tqp_index = alloced;
- tqp[alloced] = &hdev->htqp[i].q;
+ hdev->htqp[i].q.desc_num = kinfo->num_desc;
+ kinfo->tqp[alloced] = &hdev->htqp[i].q;
hdev->htqp[i].alloced = true;
alloced++;
}
}
- vport->alloc_tqps = num_tqps;
+ vport->alloc_tqps = kinfo->num_tqps;
return 0;
}
-static int hclge_knic_setup(struct hclge_vport *vport, u16 num_tqps)
+static int hclge_knic_setup(struct hclge_vport *vport,
+ u16 num_tqps, u16 num_desc)
{
struct hnae3_handle *nic = &vport->nic;
struct hnae3_knic_private_info *kinfo = &nic->kinfo;
struct hclge_dev *hdev = vport->back;
int i, ret;
- kinfo->num_desc = hdev->num_desc;
+ kinfo->num_desc = num_desc;
kinfo->rx_buf_len = hdev->rx_buf_len;
kinfo->num_tc = min_t(u16, num_tqps, hdev->tm_info.num_tc);
kinfo->rss_size
@@ -1322,7 +1324,7 @@ static int hclge_knic_setup(struct hclge_vport *vport, u16 num_tqps)
if (!kinfo->tqp)
return -ENOMEM;
- ret = hclge_assign_tqp(vport, kinfo->tqp, kinfo->num_tqps);
+ ret = hclge_assign_tqp(vport);
if (ret)
dev_err(&hdev->pdev->dev, "fail to assign TQPs %d.\n", ret);
@@ -1388,7 +1390,7 @@ static int hclge_vport_setup(struct hclge_vport *vport, u16 num_tqps)
nic->numa_node_mask = hdev->numa_node_mask;
if (hdev->ae_dev->dev_type == HNAE3_DEV_KNIC) {
- ret = hclge_knic_setup(vport, num_tqps);
+ ret = hclge_knic_setup(vport, num_tqps, hdev->num_desc);
if (ret) {
dev_err(&hdev->pdev->dev, "knic setup failed %d\n",
ret);
@@ -5936,7 +5938,7 @@ static int hclge_set_channels(struct hnae3_handle *handle, u32 new_tqps_num)
/* Free old tqps, and reallocate with new tqp number when nic setup */
hclge_release_tqp(vport);
- ret = hclge_knic_setup(vport, new_tqps_num);
+ ret = hclge_knic_setup(vport, new_tqps_num, kinfo->num_desc);
if (ret) {
dev_err(&hdev->pdev->dev, "setup nic fail, ret =%d\n", ret);
return ret;
--
2.7.4
^ permalink raw reply related
* Re: [Question] bluetooth/{bnep,cmtp,hidp}: memory barriers
From: Andrea Parri @ 2018-08-14 18:33 UTC (permalink / raw)
To: JeffyChen
Cc: Brian Norris, Marcel Holtmann, Johan Hedberg, David S. Miller,
linux-bluetooth, netdev, linux-kernel, Brian Norris,
AL Yu-Chen Cho
In-Reply-To: <5B725A12.8050409@rock-chips.com>
Hi Jeffy, Brian,
On Tue, Aug 14, 2018 at 12:26:58PM +0800, JeffyChen wrote:
> Hi guys,
>
> Thanks for your mails, and sorry for the late response..
>
> On 08/14/2018 07:18 AM, Brian Norris wrote:
> >
> >commit 5da8e47d849d3d37b14129f038782a095b9ad049
> >Author: Jeffy Chen<jeffy.chen@rock-chips.com>
> >Date: Tue Jun 27 17:34:44 2017 +0800
> >
> > Bluetooth: hidp: fix possible might sleep error in hidp_session_thread
> >
> >that*some* kind of barrier was stuck in there simply as a response to
> >comments like this, that were going away:
> >
> >- *
> >- * Note: set_current_state() performs any necessary
> >- * memory-barriers for us.
> > */
> >- set_current_state(TASK_INTERRUPTIBLE);
> >
> >+ /* Ensure session->terminate is updated */
> >+ smp_mb__before_atomic();
> >
> >
> >It was probably an attempt to fill in the gap for the
> >set_current_state() (and comment) which was being removed. I believe
> >Jeffy originally added more barriers in other places, but I convinced
> >him not to.
>
> right, i was trying to avoid losing memory-barriers when removing
> set_current_state and changing wake_up_process to wake_up_interruptible.
>
> and checking these code again, it's true the smp_mb__before_atomic before
> atomic_read is not needed, the smp_mb after atomic_inc(&session->terminate)
> should be enough.
>
> and as Brian point out, there's already an smp_store_mb at the end of
> wait_woken, i agree we can remove all the smp_mb__{before,after}_atomic() i
> wrongly added :)
Thank you for checking this once again. I'll send out a patch removing
these barriers shortly.
Andrea
P.S. I'm out of office for the next two weeks, so my replies could come
with some delay until ~ -rc1...
>
> >
> >I have to say, I'm not really up-to-speed on the use of manual barriers
> >in Linux (it's much preferable when they're wrapped into higher-level
> >data structures already), but I believe the main intention here is to
> >ensure that any change to 'terminate' that happened during the previous
> >"wait_woken()" would be visible to our atomic_read().
> >
> >Looking into wait_woken(), I'm feeling like none of these additional
> >barriers are necessary at all. I believe wait_woken() handles the
> >visibility issues we care about (that if we were woken for termination,
> >we'll see the terminating condition).
>
>
^ permalink raw reply
* Re: [PATCH RFT] net: dsa: Allow configuring CPU port VLANs
From: Florian Fainelli @ 2018-08-14 18:30 UTC (permalink / raw)
To: Petr Machata
Cc: Ilias Apalodimas, netdev, jiri, Andrew Lunn, Vivien Didelot,
David S. Miller, open list
In-Reply-To: <wihva8csth9.fsf@dev-r-vrt-156.mtr.labs.mlnx>
On 08/14/2018 11:17 AM, Petr Machata wrote:
> Florian Fainelli <f.fainelli@gmail.com> writes:
>
>> On 06/25/2018 02:17 AM, Ilias Apalodimas wrote:
>>> On Mon, Jun 25, 2018 at 12:13:10PM +0300, Petr Machata wrote:
>>>> Florian Fainelli <f.fainelli@gmail.com> writes:
>>>>
>>>>> if (netif_is_bridge_master(vlan->obj.orig_dev))
>>>>> - return -EOPNOTSUPP;
>>>>> + info.port = dp->cpu_dp->index;
>>>>
>>>> The condition above will trigger also when a VLAN is added on a member
>>>> port, and there's no other port with that VLAN. In that case the VLAN
>>>> comes without the BRIDGE_VLAN_INFO_BRENTRY flag. In mlxsw we have this
>>>> to get the bridge VLANs:
>>>>
>>>> if (netif_is_bridge_master(orig_dev)) {
>>>> [...]
>>>> if ((vlan->flags & BRIDGE_VLAN_INFO_BRENTRY) &&
>>>> [...]
>>>>
>>>> This doesn't appear to be done in DSA unless I'm missing something.
>>> Petr's right. This will trigger for VLANs added on 'not cpu ports' if the VLAN
>>> is not already a member.
>>>
>>> This command has BRIDGE_VLAN_INFO_BRENTRY set:
>>> bridge vlan add dev br0 vid 100 pvid untagged self
>>> I had the same issue on my CPSW RFC and solved it
>>> exactly the same was as Petr suggested.
>>
>> Humm, there must be something obvious I am missing, but the following
>> don't exactly result in what I would expect after adding a check for
>> vlan->flags & BRIDGE_VLAN_INFO_BRENTRY:
>>
>> brctl addbr br0
>> echo 1 > /sys/class/net/br0/bridge/vlan_filtering
>> brctl addif br0 lan1
>
> From what I see, the CPU port is configured with VLAN 1 as soon as the
> bridge is created:
>
> # brctl addbr br
> # bridge v sh dev br
> port vlan ids
> br 1 PVID Egress Untagged
>
> I expect there are events for this (but I didn't check), but the driver
> won't see them, because it doesn't have any ports attached to the bridge
> yet.
Correct, there are no ports attached yet.
>
>> #1 results in lan1 being programmed with VID 1, PVID, untagged, but not
>> the CPU port. I would have sort of expected that the bridge layer would
>> also push the configuration to br0/CPU port since this is the default VLAN:
>>
>> bridge vlan show dev br0
>> port vlan ids
>> br0 1 PVID Egress Untagged
>
> OK, apparently we are talking past each other. When you say "CPU port",
> is "br0" not what you have in mind? Because I see 1 configured at br0 in
> your listings.
Yes, when I write "CPU/management" port, I mean br0 here, or at least,
when we see an event targeting br0, we re-generate it to target the
CPU/management port of the switch here.
VLAN 1 is definitively added to the br0 interface as a BR_ENTRY but we
also need to program the CPU port of the switch to be in this VLAN entry.
>
>> bridge vlan add vid 2 dev lan1
>>
>> #2 same thing, results in only lan1 being programmed with VID 2, tagged
>> but that is expected because we are creating the VLAN only for the
>> user-facing port.
>>
>> bridge vlan add vid 3 dev br0 self
>>
>> #3 results in the CPU port being programmed with VID 3, tagged, again,
>> this is expected because we are only programming the bridge master/CPU
>> port here.
>>
>> Does #1 also happen for cpsw and mlxsw or do you actually get events
>> about the bridge's default VLAN configuration? Or does the switch driver
>> actually need to obtain that at the time the port is enslaved somehow?
>
> I don't think we care about the CPU port in mlxsw. If the packet matches
> one of the local MACs, it gets trapped anyway, so all this stuff is then
> handled in slow path.
OK, that makes sense, you really don't have a notion of a CPU/management
port like we do in DSA switches.
On those switches, the Ethernet MAC is connected to the management port
of the switch, and so, at the time you enslave the first user-facing
port, we must configure both the CPU/management port of the switch as
well as the user-facing port to be within the desired VLAN IDs (and
attributes) in order for packets to ingress (when vlan_filtering is
enabled).
I suppose the way forward is to either query the bridge layer for the
default_pvid at the time of enslavement of the ports, or, "replay" the
event that led to the creation of the default br0 VLAN entry? First
option means we can make this specific to DSA (and similar
configurations) whereas the other means we might have to update all
switchdev drivers and possibly ignore that "replay" event.
Does that make sense?
--
Florian
^ permalink raw reply
* Re: [Intel-wired-lan] [PATCH next-queue 0/8] ixgbe/ixgbevf: IPsec offload support for VFs
From: Alexander Duyck @ 2018-08-14 15:30 UTC (permalink / raw)
To: Shannon Nelson; +Cc: intel-wired-lan, Jeff Kirsher, Steffen Klassert, Netdev
In-Reply-To: <1534185825-12451-1-git-send-email-shannon.nelson@oracle.com>
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.
>
> The IPsec HW offload feature has been in the x540/Niantic family of
> network devices since their release in 2009, but there was no Linux
> kernel support for the offload until 2017. After the XFRM code added
> support for the offload last year, the hw offload was added to the ixgbe
> PF driver.
>
> Since the related x540 VF device uses same setup as the PF for implementing
> the offload, adding the feature to the ixgbevf seemed like a good idea.
> In this case, the PF owns the device registers, so the VF simply packages
> up the request information into a VF<->PF message and the PF does the
> device configuration. The resulting IPsec throughput is roughly equivalent
> to what we see in the PF - nearly line-rate, with the expected drop in CPU
> cycles burned. (I'm not great at performance statistics, I'll let better
> folks do the actual measurements as they pertain to their own usage)
>
> To make use of the capability, first two things are needed: the PF must
> be told to enable the offload for VFs (it is off by default) and the VF
> must be trusted. A new ethtool priv-flag for ixgbe is added to control
> VF offload support. For example:
>
> ethtool --set-priv-flags eth0 vf-ipsec on
> ip link set eth0 vf 1 trust on
>
> Once those are set up and the VF device is UP, the user can add SAs the
> same as for PFs, whether the VF is in the host or has been assigned to
> a VM.
>
> Note that the x540 chip supports a total of 1024 Rx plus 1024 Tx Security
> Associations (SAs), shared among the PF and VFs that might request them.
> It is entirely possible for a single VF to soak up all the offload
> capability, which would likely annoy some people. It seems rather
> arbitrary to try to set a limit for how many a VF could be allowed,
> but this is mitigated somewhat by the need for "trust" and "vf-ipsec"
> to be enabled. I suppose we could come up with a way to make a limit
> configurable, but there is no existing method for adding that kind
> configuration so I'll leave that to a future discussion.
>
> Currently this doesn't support Tx offload as the hardware encryption
> engine doesn't seem to engage on the Tx packets. This may be a lingering
> driver bug, more investigation is needed. Until then, requests for a Tx
> offload are failed and the userland requester will need to add Tx SAs
> without the offload attribute.
>
> Given that we don't have Tx offload support, the benefit here is less
> than it could be, but is definitely still noticeable. For example, with
> informal iperf testing over a 10Gbps link, with full offload in a PF on
> one side and a VF in a VM on the other side on a CPU with AES instructions:
>
> Reference:
> No IPsec: 9.4 Gbps
> IPsec offload btwn two PFs: 9.2 Gbps
> VF as the iperf receiver:
> IPsec offload on PF, none on VF: 6.8 Gbps
> IPsec offload on PF and VF: 9.2 Gbps << biggest benefit
> VF as the iperf sender:
> IPsec offload on PF, none on VF: 4.8 Gbps
> IPsec offload on PF and VF: 4.8 Gbps
>
> The iperf traffic is primarily uni-directional, and we can see the most
> benefit when VF is the iperf server and is receiving the test traffic.
> Watching output from sar also shows a nice decrease in CPU utilization.
>
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
^ permalink raw reply
* Re: [PATCH RFT] net: dsa: Allow configuring CPU port VLANs
From: Petr Machata @ 2018-08-14 18:17 UTC (permalink / raw)
To: Florian Fainelli
Cc: Ilias Apalodimas, netdev, jiri, Andrew Lunn, Vivien Didelot,
David S. Miller, open list
In-Reply-To: <9ce291a4-b40d-81d8-1c1a-c4311e5cc113@gmail.com>
Florian Fainelli <f.fainelli@gmail.com> writes:
> On 06/25/2018 02:17 AM, Ilias Apalodimas wrote:
>> On Mon, Jun 25, 2018 at 12:13:10PM +0300, Petr Machata wrote:
>>> Florian Fainelli <f.fainelli@gmail.com> writes:
>>>
>>>> if (netif_is_bridge_master(vlan->obj.orig_dev))
>>>> - return -EOPNOTSUPP;
>>>> + info.port = dp->cpu_dp->index;
>>>
>>> The condition above will trigger also when a VLAN is added on a member
>>> port, and there's no other port with that VLAN. In that case the VLAN
>>> comes without the BRIDGE_VLAN_INFO_BRENTRY flag. In mlxsw we have this
>>> to get the bridge VLANs:
>>>
>>> if (netif_is_bridge_master(orig_dev)) {
>>> [...]
>>> if ((vlan->flags & BRIDGE_VLAN_INFO_BRENTRY) &&
>>> [...]
>>>
>>> This doesn't appear to be done in DSA unless I'm missing something.
>> Petr's right. This will trigger for VLANs added on 'not cpu ports' if the VLAN
>> is not already a member.
>>
>> This command has BRIDGE_VLAN_INFO_BRENTRY set:
>> bridge vlan add dev br0 vid 100 pvid untagged self
>> I had the same issue on my CPSW RFC and solved it
>> exactly the same was as Petr suggested.
>
> Humm, there must be something obvious I am missing, but the following
> don't exactly result in what I would expect after adding a check for
> vlan->flags & BRIDGE_VLAN_INFO_BRENTRY:
>
> brctl addbr br0
> echo 1 > /sys/class/net/br0/bridge/vlan_filtering
> brctl addif br0 lan1
>From what I see, the CPU port is configured with VLAN 1 as soon as the
bridge is created:
# brctl addbr br
# bridge v sh dev br
port vlan ids
br 1 PVID Egress Untagged
I expect there are events for this (but I didn't check), but the driver
won't see them, because it doesn't have any ports attached to the bridge
yet.
> #1 results in lan1 being programmed with VID 1, PVID, untagged, but not
> the CPU port. I would have sort of expected that the bridge layer would
> also push the configuration to br0/CPU port since this is the default VLAN:
>
> bridge vlan show dev br0
> port vlan ids
> br0 1 PVID Egress Untagged
OK, apparently we are talking past each other. When you say "CPU port",
is "br0" not what you have in mind? Because I see 1 configured at br0 in
your listings.
> bridge vlan add vid 2 dev lan1
>
> #2 same thing, results in only lan1 being programmed with VID 2, tagged
> but that is expected because we are creating the VLAN only for the
> user-facing port.
>
> bridge vlan add vid 3 dev br0 self
>
> #3 results in the CPU port being programmed with VID 3, tagged, again,
> this is expected because we are only programming the bridge master/CPU
> port here.
>
> Does #1 also happen for cpsw and mlxsw or do you actually get events
> about the bridge's default VLAN configuration? Or does the switch driver
> actually need to obtain that at the time the port is enslaved somehow?
I don't think we care about the CPU port in mlxsw. If the packet matches
one of the local MACs, it gets trapped anyway, so all this stuff is then
handled in slow path.
Petr
^ permalink raw reply
* Re: [PATCH RFC net-next] openvswitch: Queue upcalls to userspace in per-port round-robin order
From: Stefano Brivio @ 2018-08-14 15:25 UTC (permalink / raw)
To: William Tu
Cc: Ben Pfaff, Matteo Croce, Pravin B Shelar, Justin Pettit,
Greg Rose, netdev, <dev@openvswitch.org>, Jiri Benc,
Aaron Conole, Joe Stringer
In-Reply-To: <CALDO+SZ=y=LHmwic0Fh260FUxV6z-KUOVOf7WJPkNDeTE6BxNQ@mail.gmail.com>
Hi William,
On Fri, 10 Aug 2018 07:11:01 -0700
William Tu <u9012063@gmail.com> wrote:
> > int rr_select_srcport(struct dp_upcall_info *upcall)
> > {
> > /* look up source port from upcall->skb... */
> > }
> >
> > And we could then easily extend this to use BPF with maps one day.
> >
> >
> Hi Stefano,
>
> If you want to experiment with BPF, Joe and I have some prototype.
> We implemented the upcall mechanism using BPF perf event helper function
> https://github.com/williamtu/ovs-ebpf/blob/master/bpf/datapath.c#L62
>
> And there are threads polling the perf ring buffer to receive packets from
> BPF.
> https://github.com/williamtu/ovs-ebpf/blob/master/lib/perf-event.c#L232
Interesting, thanks for the pointers!
> If I follow the discussion correctly, before upcall, you need to queue
> packets based on different configurations (vport/hash/vni/5-tuple/...)
> and queue to different buckets when congestion happens.
Yes, correct.
> In this case, you
> probably needs a BPF map to enqueue/dequeue the packet.BPF queue map is
> not supported yet, but there is patch available:
> [iovisor-dev] [RFC PATCH 1/3] bpf: add bpf queue map
>
> So how to enqueue and dequeue packets depends on user's BPF implementation.
> This allows fairness scheme to be extensible.
For the moment being we'll try to ensure that BPF can be plugged there
rather easily. I see the advantage, but I'd rather do this as a second
step.
--
Stefano
^ permalink raw reply
* Re: [V9fs-developer] [PATCH 2/2] 9p: Add refcount to p9_req_t
From: Tomas Bortoli @ 2018-08-14 17:44 UTC (permalink / raw)
To: piaojun, asmadeus, ericvh, rminnich, lucho
Cc: Dominique Martinet, netdev, linux-kernel, syzkaller,
v9fs-developer, davem
In-Reply-To: <5B72329F.80902@huawei.com>
On 08/14/2018 03:38 AM, piaojun wrote:
> Hi Tomas & Dominique,
>
> On 2018/8/11 22:42, Tomas Bortoli wrote:
>> To avoid use-after-free(s), use a refcount to keep track of the
>> usable references to any instantiated struct p9_req_t.
>>
>> This commit adds p9_req_put(), p9_req_get() and p9_req_try_get() as
>> wrappers to kref_put(), kref_get() and kref_get_unless_zero().
>> These are used by the client and the transports to keep track of
>> valid requests' references.
>>
>> p9_free_req() is added back and used as callback by kref_put().
>>
>> Add SLAB_TYPESAFE_BY_RCU as it ensures that the memory freed by
>> kmem_cache_free() will not be reused for another type until the rcu
>> synchronisation period is over, so an address gotten under rcu read
>> lock is safe to inc_ref() without corrupting random memory while
>> the lock is held.
>>
>> Co-developed-by: Dominique Martinet <dominique.martinet@cea.fr>
>> Signed-off-by: Tomas Bortoli <tomasbortoli@gmail.com>
>> Reported-by: syzbot+467050c1ce275af2a5b8@syzkaller.appspotmail.com
>> Signed-off-by: Dominique Martinet <dominique.martinet@cea.fr>
>> ---
>> include/net/9p/client.h | 14 +++++++++++++
>> net/9p/client.c | 54 +++++++++++++++++++++++++++++++++++++++++++------
>> net/9p/trans_fd.c | 11 +++++++++-
>> net/9p/trans_rdma.c | 1 +
>> 4 files changed, 73 insertions(+), 7 deletions(-)
>>
>> diff --git a/include/net/9p/client.h b/include/net/9p/client.h
>> index 735f3979d559..947a570307a6 100644
>> --- a/include/net/9p/client.h
>> +++ b/include/net/9p/client.h
>> @@ -94,6 +94,7 @@ enum p9_req_status_t {
>> struct p9_req_t {
>> int status;
>> int t_err;
>> + struct kref refcount;
>> wait_queue_head_t wq;
>> struct p9_fcall tc;
>> struct p9_fcall rc;
>> @@ -233,6 +234,19 @@ int p9_client_lock_dotl(struct p9_fid *fid, struct p9_flock *flock, u8 *status);
>> int p9_client_getlock_dotl(struct p9_fid *fid, struct p9_getlock *fl);
>> void p9_fcall_fini(struct p9_fcall *fc);
>> struct p9_req_t *p9_tag_lookup(struct p9_client *, u16);
>> +
>> +static inline void p9_req_get(struct p9_req_t *r)
>> +{
>> + kref_get(&r->refcount);
>> +}
>> +
>> +static inline int p9_req_try_get(struct p9_req_t *r)
>> +{
>> + return kref_get_unless_zero(&r->refcount);
>> +}
>> +
>> +int p9_req_put(struct p9_req_t *r);
>> +
>> void p9_client_cb(struct p9_client *c, struct p9_req_t *req, int status);
>>
>> int p9_parse_header(struct p9_fcall *, int32_t *, int8_t *, int16_t *, int);
>> diff --git a/net/9p/client.c b/net/9p/client.c
>> index 7942c0bfcc5b..83f2f0aadc14 100644
>> --- a/net/9p/client.c
>> +++ b/net/9p/client.c
>> @@ -310,6 +310,18 @@ p9_tag_alloc(struct p9_client *c, int8_t type, unsigned int max_size)
>> if (tag < 0)
>> goto free;
>>
>> + /* Init ref to two because in the general case there is one ref
>> + * that is put asynchronously by a writer thread, one ref
>> + * temporarily given by p9_tag_lookup and put by p9_client_cb
>> + * in the recv thread, and one ref put by p9_remove_tag in the
>
> There is a spell mistake, p9_remove_tag->p9_tag_remove, and sorry for not
> pointing this in last comment.
>
> Thanks,
> Jun
>
>> + * main thread. The only exception is virtio that does not use
>> + * p9_tag_lookup but does not have a writer thread either
>> + * (the write happens synchronously in the request/zc_request
>> + * callback), so p9_client_cb eats the second ref there
>> + * as the pointer is duplicated directly by virtqueue_add_sgs()
>> + */
>> + refcount_set(&req->refcount.refcount, 2);
>> +
>> return req;
>>
>> free:
>> @@ -333,10 +345,21 @@ struct p9_req_t *p9_tag_lookup(struct p9_client *c, u16 tag)
>> struct p9_req_t *req;
>>
>> rcu_read_lock();
>> +again:
>> req = idr_find(&c->reqs, tag);
>> - /* There's no refcount on the req; a malicious server could cause
>> - * us to dereference a NULL pointer
>> - */
>> + if (req) {
>> + /* We have to be careful with the req found under rcu_read_lock
>> + * Thanks to SLAB_TYPESAFE_BY_RCU we can safely try to get the
>> + * ref again without corrupting other data, then check again
>> + * that the tag matches once we have the ref
>> + */
>> + if (!p9_req_try_get(req))
>> + goto again;
>> + if (req->tc.tag != tag) {
>> + p9_req_put(req);
>> + goto again;
>> + }
>> + }
>> rcu_read_unlock();
>>
>> return req;
>> @@ -350,7 +373,7 @@ EXPORT_SYMBOL(p9_tag_lookup);
>> *
>> * Context: Any context.
>> */
>> -static void p9_tag_remove(struct p9_client *c, struct p9_req_t *r)
>> +static int p9_tag_remove(struct p9_client *c, struct p9_req_t *r)
>> {
>> unsigned long flags;
>> u16 tag = r->tc.tag;
>> @@ -359,11 +382,23 @@ static void p9_tag_remove(struct p9_client *c, struct p9_req_t *r)
>> spin_lock_irqsave(&c->lock, flags);
>> idr_remove(&c->reqs, tag);
>> spin_unlock_irqrestore(&c->lock, flags);
>> + return p9_req_put(r);
>> +}
>> +
>> +static void p9_req_free(struct kref *ref)
>> +{
>> + struct p9_req_t *r = container_of(ref, struct p9_req_t, refcount);
>> p9_fcall_fini(&r->tc);
>> p9_fcall_fini(&r->rc);
>> kmem_cache_free(p9_req_cache, r);
>> }
>>
>> +int p9_req_put(struct p9_req_t *r)
>> +{
>> + return kref_put(&r->refcount, p9_req_free);
>> +}
>> +EXPORT_SYMBOL(p9_req_put);
>> +
>> /**
>> * p9_tag_cleanup - cleans up tags structure and reclaims resources
>> * @c: v9fs client struct
>> @@ -379,7 +414,9 @@ static void p9_tag_cleanup(struct p9_client *c)
>> rcu_read_lock();
>> idr_for_each_entry(&c->reqs, req, id) {
>> pr_info("Tag %d still in use\n", id);
>> - p9_tag_remove(c, req);
>> + if (p9_tag_remove(c, req) == 0)
>> + pr_warn("Packet with tag %d has still references",
>> + req->tc.tag);
>> }
>> rcu_read_unlock();
>> }
>> @@ -403,6 +440,7 @@ void p9_client_cb(struct p9_client *c, struct p9_req_t *req, int status)
>>
>> wake_up(&req->wq);
>> p9_debug(P9_DEBUG_MUX, "wakeup: %d\n", req->tc.tag);
>> + p9_req_put(req);
>> }
>> EXPORT_SYMBOL(p9_client_cb);
>>
>> @@ -682,6 +720,8 @@ static struct p9_req_t *p9_client_prepare_req(struct p9_client *c,
>> return req;
>> reterr:
>> p9_tag_remove(c, req);
>> + /* We have to put also the 2nd reference as it won't be used */
>> + p9_req_put(req);
>> return ERR_PTR(err);
>> }
>>
>> @@ -716,6 +756,8 @@ p9_client_rpc(struct p9_client *c, int8_t type, const char *fmt, ...)
>>
>> err = c->trans_mod->request(c, req);
>> if (err < 0) {
>> + /* write won't happen */
>> + p9_req_put(req);
>> if (err != -ERESTARTSYS && err != -EFAULT)
>> c->status = Disconnected;
>> goto recalc_sigpending;
>> @@ -2241,7 +2283,7 @@ EXPORT_SYMBOL(p9_client_readlink);
>>
>> int __init p9_client_init(void)
>> {
>> - p9_req_cache = KMEM_CACHE(p9_req_t, 0);
>> + p9_req_cache = KMEM_CACHE(p9_req_t, SLAB_TYPESAFE_BY_RCU);
>> return p9_req_cache ? 0 : -ENOMEM;
>> }
>>
>> diff --git a/net/9p/trans_fd.c b/net/9p/trans_fd.c
>> index 20f46f13fe83..686e24e355d0 100644
>> --- a/net/9p/trans_fd.c
>> +++ b/net/9p/trans_fd.c
>> @@ -132,6 +132,7 @@ struct p9_conn {
>> struct list_head req_list;
>> struct list_head unsent_req_list;
>> struct p9_req_t *req;
>> + struct p9_req_t *wreq;
>> char tmp_buf[7];
>> struct p9_fcall rc;
>> int wpos;
>> @@ -383,6 +384,7 @@ static void p9_read_work(struct work_struct *work)
>> m->rc.sdata = NULL;
>> m->rc.offset = 0;
>> m->rc.capacity = 0;
>> + p9_req_put(m->req);
>> m->req = NULL;
>> }
>>
>> @@ -472,6 +474,8 @@ static void p9_write_work(struct work_struct *work)
>> m->wbuf = req->tc.sdata;
>> m->wsize = req->tc.size;
>> m->wpos = 0;
>> + p9_req_get(req);
>> + m->wreq = req;
>> spin_unlock(&m->client->lock);
>> }
>>
>> @@ -492,8 +496,11 @@ static void p9_write_work(struct work_struct *work)
>> }
>>
>> m->wpos += err;
>> - if (m->wpos == m->wsize)
>> + if (m->wpos == m->wsize) {
>> m->wpos = m->wsize = 0;
>> + p9_req_put(m->wreq);
>> + m->wreq = NULL;
>> + }
>>
>> end_clear:
>> clear_bit(Wworksched, &m->wsched);
>> @@ -694,6 +701,7 @@ static int p9_fd_cancel(struct p9_client *client, struct p9_req_t *req)
>> if (req->status == REQ_STATUS_UNSENT) {
>> list_del(&req->req_list);
>> req->status = REQ_STATUS_FLSHD;
>> + p9_req_put(req);
>> ret = 0;
>> }
>> spin_unlock(&client->lock);
>> @@ -711,6 +719,7 @@ static int p9_fd_cancelled(struct p9_client *client, struct p9_req_t *req)
>> spin_lock(&client->lock);
>> list_del(&req->req_list);
>> spin_unlock(&client->lock);
>> + p9_req_put(req);
>>
>> return 0;
>> }
>> diff --git a/net/9p/trans_rdma.c b/net/9p/trans_rdma.c
>> index c60655c90c9e..8cff368a11e3 100644
>> --- a/net/9p/trans_rdma.c
>> +++ b/net/9p/trans_rdma.c
>> @@ -365,6 +365,7 @@ send_done(struct ib_cq *cq, struct ib_wc *wc)
>> c->busa, c->req->tc.size,
>> DMA_TO_DEVICE);
>> up(&rdma->sq_sem);
>> + p9_req_put(c->req);
>> kfree(c);
>> }
>>
>>
>
Spell fixed
Tomas
^ permalink raw reply
* [PATCH v2 2/2] 9p: Add refcount to p9_req_t
From: Tomas Bortoli @ 2018-08-14 17:43 UTC (permalink / raw)
To: asmadeus, ericvh, rminnich, lucho
Cc: davem, v9fs-developer, netdev, linux-kernel, syzkaller,
Tomas Bortoli, Dominique Martinet
In-Reply-To: <20180814174342.11068-1-tomasbortoli@gmail.com>
To avoid use-after-free(s), use a refcount to keep track of the
usable references to any instantiated struct p9_req_t.
This commit adds p9_req_put(), p9_req_get() and p9_req_try_get() as
wrappers to kref_put(), kref_get() and kref_get_unless_zero().
These are used by the client and the transports to keep track of
valid requests' references.
p9_free_req() is added back and used as callback by kref_put().
Add SLAB_TYPESAFE_BY_RCU as it ensures that the memory freed by
kmem_cache_free() will not be reused for another type until the rcu
synchronisation period is over, so an address gotten under rcu read
lock is safe to inc_ref() without corrupting random memory while
the lock is held.
Co-developed-by: Dominique Martinet <dominique.martinet@cea.fr>
Signed-off-by: Tomas Bortoli <tomasbortoli@gmail.com>
Reported-by: syzbot+467050c1ce275af2a5b8@syzkaller.appspotmail.com
Signed-off-by: Dominique Martinet <dominique.martinet@cea.fr>
---
include/net/9p/client.h | 14 +++++++++++++
net/9p/client.c | 54 +++++++++++++++++++++++++++++++++++++++++++------
net/9p/trans_fd.c | 11 +++++++++-
net/9p/trans_rdma.c | 1 +
4 files changed, 73 insertions(+), 7 deletions(-)
diff --git a/include/net/9p/client.h b/include/net/9p/client.h
index 735f3979d559..947a570307a6 100644
--- a/include/net/9p/client.h
+++ b/include/net/9p/client.h
@@ -94,6 +94,7 @@ enum p9_req_status_t {
struct p9_req_t {
int status;
int t_err;
+ struct kref refcount;
wait_queue_head_t wq;
struct p9_fcall tc;
struct p9_fcall rc;
@@ -233,6 +234,19 @@ int p9_client_lock_dotl(struct p9_fid *fid, struct p9_flock *flock, u8 *status);
int p9_client_getlock_dotl(struct p9_fid *fid, struct p9_getlock *fl);
void p9_fcall_fini(struct p9_fcall *fc);
struct p9_req_t *p9_tag_lookup(struct p9_client *, u16);
+
+static inline void p9_req_get(struct p9_req_t *r)
+{
+ kref_get(&r->refcount);
+}
+
+static inline int p9_req_try_get(struct p9_req_t *r)
+{
+ return kref_get_unless_zero(&r->refcount);
+}
+
+int p9_req_put(struct p9_req_t *r);
+
void p9_client_cb(struct p9_client *c, struct p9_req_t *req, int status);
int p9_parse_header(struct p9_fcall *, int32_t *, int8_t *, int16_t *, int);
diff --git a/net/9p/client.c b/net/9p/client.c
index 7942c0bfcc5b..c9bb5d41afa4 100644
--- a/net/9p/client.c
+++ b/net/9p/client.c
@@ -310,6 +310,18 @@ p9_tag_alloc(struct p9_client *c, int8_t type, unsigned int max_size)
if (tag < 0)
goto free;
+ /* Init ref to two because in the general case there is one ref
+ * that is put asynchronously by a writer thread, one ref
+ * temporarily given by p9_tag_lookup and put by p9_client_cb
+ * in the recv thread, and one ref put by p9_tag_remove in the
+ * main thread. The only exception is virtio that does not use
+ * p9_tag_lookup but does not have a writer thread either
+ * (the write happens synchronously in the request/zc_request
+ * callback), so p9_client_cb eats the second ref there
+ * as the pointer is duplicated directly by virtqueue_add_sgs()
+ */
+ refcount_set(&req->refcount.refcount, 2);
+
return req;
free:
@@ -333,10 +345,21 @@ struct p9_req_t *p9_tag_lookup(struct p9_client *c, u16 tag)
struct p9_req_t *req;
rcu_read_lock();
+again:
req = idr_find(&c->reqs, tag);
- /* There's no refcount on the req; a malicious server could cause
- * us to dereference a NULL pointer
- */
+ if (req) {
+ /* We have to be careful with the req found under rcu_read_lock
+ * Thanks to SLAB_TYPESAFE_BY_RCU we can safely try to get the
+ * ref again without corrupting other data, then check again
+ * that the tag matches once we have the ref
+ */
+ if (!p9_req_try_get(req))
+ goto again;
+ if (req->tc.tag != tag) {
+ p9_req_put(req);
+ goto again;
+ }
+ }
rcu_read_unlock();
return req;
@@ -350,7 +373,7 @@ EXPORT_SYMBOL(p9_tag_lookup);
*
* Context: Any context.
*/
-static void p9_tag_remove(struct p9_client *c, struct p9_req_t *r)
+static int p9_tag_remove(struct p9_client *c, struct p9_req_t *r)
{
unsigned long flags;
u16 tag = r->tc.tag;
@@ -359,11 +382,23 @@ static void p9_tag_remove(struct p9_client *c, struct p9_req_t *r)
spin_lock_irqsave(&c->lock, flags);
idr_remove(&c->reqs, tag);
spin_unlock_irqrestore(&c->lock, flags);
+ return p9_req_put(r);
+}
+
+static void p9_req_free(struct kref *ref)
+{
+ struct p9_req_t *r = container_of(ref, struct p9_req_t, refcount);
p9_fcall_fini(&r->tc);
p9_fcall_fini(&r->rc);
kmem_cache_free(p9_req_cache, r);
}
+int p9_req_put(struct p9_req_t *r)
+{
+ return kref_put(&r->refcount, p9_req_free);
+}
+EXPORT_SYMBOL(p9_req_put);
+
/**
* p9_tag_cleanup - cleans up tags structure and reclaims resources
* @c: v9fs client struct
@@ -379,7 +414,9 @@ static void p9_tag_cleanup(struct p9_client *c)
rcu_read_lock();
idr_for_each_entry(&c->reqs, req, id) {
pr_info("Tag %d still in use\n", id);
- p9_tag_remove(c, req);
+ if (p9_tag_remove(c, req) == 0)
+ pr_warn("Packet with tag %d has still references",
+ req->tc.tag);
}
rcu_read_unlock();
}
@@ -403,6 +440,7 @@ void p9_client_cb(struct p9_client *c, struct p9_req_t *req, int status)
wake_up(&req->wq);
p9_debug(P9_DEBUG_MUX, "wakeup: %d\n", req->tc.tag);
+ p9_req_put(req);
}
EXPORT_SYMBOL(p9_client_cb);
@@ -682,6 +720,8 @@ static struct p9_req_t *p9_client_prepare_req(struct p9_client *c,
return req;
reterr:
p9_tag_remove(c, req);
+ /* We have to put also the 2nd reference as it won't be used */
+ p9_req_put(req);
return ERR_PTR(err);
}
@@ -716,6 +756,8 @@ p9_client_rpc(struct p9_client *c, int8_t type, const char *fmt, ...)
err = c->trans_mod->request(c, req);
if (err < 0) {
+ /* write won't happen */
+ p9_req_put(req);
if (err != -ERESTARTSYS && err != -EFAULT)
c->status = Disconnected;
goto recalc_sigpending;
@@ -2241,7 +2283,7 @@ EXPORT_SYMBOL(p9_client_readlink);
int __init p9_client_init(void)
{
- p9_req_cache = KMEM_CACHE(p9_req_t, 0);
+ p9_req_cache = KMEM_CACHE(p9_req_t, SLAB_TYPESAFE_BY_RCU);
return p9_req_cache ? 0 : -ENOMEM;
}
diff --git a/net/9p/trans_fd.c b/net/9p/trans_fd.c
index 20f46f13fe83..686e24e355d0 100644
--- a/net/9p/trans_fd.c
+++ b/net/9p/trans_fd.c
@@ -132,6 +132,7 @@ struct p9_conn {
struct list_head req_list;
struct list_head unsent_req_list;
struct p9_req_t *req;
+ struct p9_req_t *wreq;
char tmp_buf[7];
struct p9_fcall rc;
int wpos;
@@ -383,6 +384,7 @@ static void p9_read_work(struct work_struct *work)
m->rc.sdata = NULL;
m->rc.offset = 0;
m->rc.capacity = 0;
+ p9_req_put(m->req);
m->req = NULL;
}
@@ -472,6 +474,8 @@ static void p9_write_work(struct work_struct *work)
m->wbuf = req->tc.sdata;
m->wsize = req->tc.size;
m->wpos = 0;
+ p9_req_get(req);
+ m->wreq = req;
spin_unlock(&m->client->lock);
}
@@ -492,8 +496,11 @@ static void p9_write_work(struct work_struct *work)
}
m->wpos += err;
- if (m->wpos == m->wsize)
+ if (m->wpos == m->wsize) {
m->wpos = m->wsize = 0;
+ p9_req_put(m->wreq);
+ m->wreq = NULL;
+ }
end_clear:
clear_bit(Wworksched, &m->wsched);
@@ -694,6 +701,7 @@ static int p9_fd_cancel(struct p9_client *client, struct p9_req_t *req)
if (req->status == REQ_STATUS_UNSENT) {
list_del(&req->req_list);
req->status = REQ_STATUS_FLSHD;
+ p9_req_put(req);
ret = 0;
}
spin_unlock(&client->lock);
@@ -711,6 +719,7 @@ static int p9_fd_cancelled(struct p9_client *client, struct p9_req_t *req)
spin_lock(&client->lock);
list_del(&req->req_list);
spin_unlock(&client->lock);
+ p9_req_put(req);
return 0;
}
diff --git a/net/9p/trans_rdma.c b/net/9p/trans_rdma.c
index c60655c90c9e..8cff368a11e3 100644
--- a/net/9p/trans_rdma.c
+++ b/net/9p/trans_rdma.c
@@ -365,6 +365,7 @@ send_done(struct ib_cq *cq, struct ib_wc *wc)
c->busa, c->req->tc.size,
DMA_TO_DEVICE);
up(&rdma->sq_sem);
+ p9_req_put(c->req);
kfree(c);
}
--
2.11.0
^ permalink raw reply related
* [PATCH v2 1/2] WIP: 9p: rename p9_free_req() function
From: Tomas Bortoli @ 2018-08-14 17:43 UTC (permalink / raw)
To: asmadeus, ericvh, rminnich, lucho
Cc: davem, v9fs-developer, netdev, linux-kernel, syzkaller,
Tomas Bortoli, Dominique Martinet
In sight of the next patch to add a refcount in p9_req_t, rename
the p9_free_req() function in p9_release_req().
In the next patch the actual kfree will be moved to another function.
Signed-off-by: Tomas Bortoli <tomasbortoli@gmail.com>
Signed-off-by: Dominique Martinet <dominique.martinet@cea.fr>
---
net/9p/client.c | 100 ++++++++++++++++++++++++++++----------------------------
1 file changed, 50 insertions(+), 50 deletions(-)
diff --git a/net/9p/client.c b/net/9p/client.c
index 6c57ab1294d7..7942c0bfcc5b 100644
--- a/net/9p/client.c
+++ b/net/9p/client.c
@@ -344,13 +344,13 @@ struct p9_req_t *p9_tag_lookup(struct p9_client *c, u16 tag)
EXPORT_SYMBOL(p9_tag_lookup);
/**
- * p9_free_req - Free a request.
+ * p9_tag_remove - Remove a tag.
* @c: Client session.
- * @r: Request to free.
+ * @r: Request of reference.
*
* Context: Any context.
*/
-static void p9_free_req(struct p9_client *c, struct p9_req_t *r)
+static void p9_tag_remove(struct p9_client *c, struct p9_req_t *r)
{
unsigned long flags;
u16 tag = r->tc.tag;
@@ -379,7 +379,7 @@ static void p9_tag_cleanup(struct p9_client *c)
rcu_read_lock();
idr_for_each_entry(&c->reqs, req, id) {
pr_info("Tag %d still in use\n", id);
- p9_free_req(c, req);
+ p9_tag_remove(c, req);
}
rcu_read_unlock();
}
@@ -647,7 +647,7 @@ static int p9_client_flush(struct p9_client *c, struct p9_req_t *oldreq)
if (c->trans_mod->cancelled)
c->trans_mod->cancelled(c, oldreq);
- p9_free_req(c, req);
+ p9_tag_remove(c, req);
return 0;
}
@@ -681,7 +681,7 @@ static struct p9_req_t *p9_client_prepare_req(struct p9_client *c,
trace_9p_client_req(c, type, req->tc.tag);
return req;
reterr:
- p9_free_req(c, req);
+ p9_tag_remove(c, req);
return ERR_PTR(err);
}
@@ -691,7 +691,7 @@ static struct p9_req_t *p9_client_prepare_req(struct p9_client *c,
* @type: type of request
* @fmt: protocol format string (see protocol.c)
*
- * Returns request structure (which client must free using p9_free_req)
+ * Returns request structure (which client must free using p9_tag_remove)
*/
static struct p9_req_t *
@@ -767,7 +767,7 @@ p9_client_rpc(struct p9_client *c, int8_t type, const char *fmt, ...)
if (!err)
return req;
reterr:
- p9_free_req(c, req);
+ p9_tag_remove(c, req);
return ERR_PTR(safe_errno(err));
}
@@ -782,7 +782,7 @@ p9_client_rpc(struct p9_client *c, int8_t type, const char *fmt, ...)
* @hdrlen: reader header size, This is the size of response protocol data
* @fmt: protocol format string (see protocol.c)
*
- * Returns request structure (which client must free using p9_free_req)
+ * Returns request structure (which client must free using p9_tag_remove)
*/
static struct p9_req_t *p9_client_zc_rpc(struct p9_client *c, int8_t type,
struct iov_iter *uidata,
@@ -849,7 +849,7 @@ static struct p9_req_t *p9_client_zc_rpc(struct p9_client *c, int8_t type,
if (!err)
return req;
reterr:
- p9_free_req(c, req);
+ p9_tag_remove(c, req);
return ERR_PTR(safe_errno(err));
}
@@ -952,7 +952,7 @@ static int p9_client_version(struct p9_client *c)
error:
kfree(version);
- p9_free_req(c, req);
+ p9_tag_remove(c, req);
return err;
}
@@ -1094,7 +1094,7 @@ struct p9_fid *p9_client_attach(struct p9_client *clnt, struct p9_fid *afid,
err = p9pdu_readf(&req->rc, clnt->proto_version, "Q", &qid);
if (err) {
trace_9p_protocol_dump(clnt, &req->rc);
- p9_free_req(clnt, req);
+ p9_tag_remove(clnt, req);
goto error;
}
@@ -1103,7 +1103,7 @@ struct p9_fid *p9_client_attach(struct p9_client *clnt, struct p9_fid *afid,
memmove(&fid->qid, &qid, sizeof(struct p9_qid));
- p9_free_req(clnt, req);
+ p9_tag_remove(clnt, req);
return fid;
error:
@@ -1151,10 +1151,10 @@ struct p9_fid *p9_client_walk(struct p9_fid *oldfid, uint16_t nwname,
err = p9pdu_readf(&req->rc, clnt->proto_version, "R", &nwqids, &wqids);
if (err) {
trace_9p_protocol_dump(clnt, &req->rc);
- p9_free_req(clnt, req);
+ p9_tag_remove(clnt, req);
goto clunk_fid;
}
- p9_free_req(clnt, req);
+ p9_tag_remove(clnt, req);
p9_debug(P9_DEBUG_9P, "<<< RWALK nwqid %d:\n", nwqids);
@@ -1229,7 +1229,7 @@ int p9_client_open(struct p9_fid *fid, int mode)
fid->iounit = iounit;
free_and_error:
- p9_free_req(clnt, req);
+ p9_tag_remove(clnt, req);
error:
return err;
}
@@ -1274,7 +1274,7 @@ int p9_client_create_dotl(struct p9_fid *ofid, const char *name, u32 flags, u32
ofid->iounit = iounit;
free_and_error:
- p9_free_req(clnt, req);
+ p9_tag_remove(clnt, req);
error:
return err;
}
@@ -1319,7 +1319,7 @@ int p9_client_fcreate(struct p9_fid *fid, const char *name, u32 perm, int mode,
fid->iounit = iounit;
free_and_error:
- p9_free_req(clnt, req);
+ p9_tag_remove(clnt, req);
error:
return err;
}
@@ -1353,7 +1353,7 @@ int p9_client_symlink(struct p9_fid *dfid, const char *name,
qid->type, (unsigned long long)qid->path, qid->version);
free_and_error:
- p9_free_req(clnt, req);
+ p9_tag_remove(clnt, req);
error:
return err;
}
@@ -1373,7 +1373,7 @@ int p9_client_link(struct p9_fid *dfid, struct p9_fid *oldfid, const char *newna
return PTR_ERR(req);
p9_debug(P9_DEBUG_9P, "<<< RLINK\n");
- p9_free_req(clnt, req);
+ p9_tag_remove(clnt, req);
return 0;
}
EXPORT_SYMBOL(p9_client_link);
@@ -1397,7 +1397,7 @@ int p9_client_fsync(struct p9_fid *fid, int datasync)
p9_debug(P9_DEBUG_9P, "<<< RFSYNC fid %d\n", fid->fid);
- p9_free_req(clnt, req);
+ p9_tag_remove(clnt, req);
error:
return err;
@@ -1432,7 +1432,7 @@ int p9_client_clunk(struct p9_fid *fid)
p9_debug(P9_DEBUG_9P, "<<< RCLUNK fid %d\n", fid->fid);
- p9_free_req(clnt, req);
+ p9_tag_remove(clnt, req);
error:
/*
* Fid is not valid even after a failed clunk
@@ -1466,7 +1466,7 @@ int p9_client_remove(struct p9_fid *fid)
p9_debug(P9_DEBUG_9P, "<<< RREMOVE fid %d\n", fid->fid);
- p9_free_req(clnt, req);
+ p9_tag_remove(clnt, req);
error:
if (err == -ERESTARTSYS)
p9_client_clunk(fid);
@@ -1493,7 +1493,7 @@ int p9_client_unlinkat(struct p9_fid *dfid, const char *name, int flags)
}
p9_debug(P9_DEBUG_9P, "<<< RUNLINKAT fid %d %s\n", dfid->fid, name);
- p9_free_req(clnt, req);
+ p9_tag_remove(clnt, req);
error:
return err;
}
@@ -1545,7 +1545,7 @@ p9_client_read(struct p9_fid *fid, u64 offset, struct iov_iter *to, int *err)
"D", &count, &dataptr);
if (*err) {
trace_9p_protocol_dump(clnt, &req->rc);
- p9_free_req(clnt, req);
+ p9_tag_remove(clnt, req);
break;
}
if (rsize < count) {
@@ -1555,7 +1555,7 @@ p9_client_read(struct p9_fid *fid, u64 offset, struct iov_iter *to, int *err)
p9_debug(P9_DEBUG_9P, "<<< RREAD count %d\n", count);
if (!count) {
- p9_free_req(clnt, req);
+ p9_tag_remove(clnt, req);
break;
}
@@ -1565,7 +1565,7 @@ p9_client_read(struct p9_fid *fid, u64 offset, struct iov_iter *to, int *err)
offset += n;
if (n != count) {
*err = -EFAULT;
- p9_free_req(clnt, req);
+ p9_tag_remove(clnt, req);
break;
}
} else {
@@ -1573,7 +1573,7 @@ p9_client_read(struct p9_fid *fid, u64 offset, struct iov_iter *to, int *err)
total += count;
offset += count;
}
- p9_free_req(clnt, req);
+ p9_tag_remove(clnt, req);
}
return total;
}
@@ -1617,7 +1617,7 @@ p9_client_write(struct p9_fid *fid, u64 offset, struct iov_iter *from, int *err)
*err = p9pdu_readf(&req->rc, clnt->proto_version, "d", &count);
if (*err) {
trace_9p_protocol_dump(clnt, &req->rc);
- p9_free_req(clnt, req);
+ p9_tag_remove(clnt, req);
break;
}
if (rsize < count) {
@@ -1627,7 +1627,7 @@ p9_client_write(struct p9_fid *fid, u64 offset, struct iov_iter *from, int *err)
p9_debug(P9_DEBUG_9P, "<<< RWRITE count %d\n", count);
- p9_free_req(clnt, req);
+ p9_tag_remove(clnt, req);
iov_iter_advance(from, count);
total += count;
offset += count;
@@ -1661,7 +1661,7 @@ struct p9_wstat *p9_client_stat(struct p9_fid *fid)
err = p9pdu_readf(&req->rc, clnt->proto_version, "wS", &ignored, ret);
if (err) {
trace_9p_protocol_dump(clnt, &req->rc);
- p9_free_req(clnt, req);
+ p9_tag_remove(clnt, req);
goto error;
}
@@ -1678,7 +1678,7 @@ struct p9_wstat *p9_client_stat(struct p9_fid *fid)
from_kgid(&init_user_ns, ret->n_gid),
from_kuid(&init_user_ns, ret->n_muid));
- p9_free_req(clnt, req);
+ p9_tag_remove(clnt, req);
return ret;
error:
@@ -1714,7 +1714,7 @@ struct p9_stat_dotl *p9_client_getattr_dotl(struct p9_fid *fid,
err = p9pdu_readf(&req->rc, clnt->proto_version, "A", ret);
if (err) {
trace_9p_protocol_dump(clnt, &req->rc);
- p9_free_req(clnt, req);
+ p9_tag_remove(clnt, req);
goto error;
}
@@ -1739,7 +1739,7 @@ struct p9_stat_dotl *p9_client_getattr_dotl(struct p9_fid *fid,
ret->st_ctime_nsec, ret->st_btime_sec, ret->st_btime_nsec,
ret->st_gen, ret->st_data_version);
- p9_free_req(clnt, req);
+ p9_tag_remove(clnt, req);
return ret;
error:
@@ -1808,7 +1808,7 @@ int p9_client_wstat(struct p9_fid *fid, struct p9_wstat *wst)
p9_debug(P9_DEBUG_9P, "<<< RWSTAT fid %d\n", fid->fid);
- p9_free_req(clnt, req);
+ p9_tag_remove(clnt, req);
error:
return err;
}
@@ -1840,7 +1840,7 @@ int p9_client_setattr(struct p9_fid *fid, struct p9_iattr_dotl *p9attr)
goto error;
}
p9_debug(P9_DEBUG_9P, "<<< RSETATTR fid %d\n", fid->fid);
- p9_free_req(clnt, req);
+ p9_tag_remove(clnt, req);
error:
return err;
}
@@ -1868,7 +1868,7 @@ int p9_client_statfs(struct p9_fid *fid, struct p9_rstatfs *sb)
&sb->files, &sb->ffree, &sb->fsid, &sb->namelen);
if (err) {
trace_9p_protocol_dump(clnt, &req->rc);
- p9_free_req(clnt, req);
+ p9_tag_remove(clnt, req);
goto error;
}
@@ -1879,7 +1879,7 @@ int p9_client_statfs(struct p9_fid *fid, struct p9_rstatfs *sb)
sb->blocks, sb->bfree, sb->bavail, sb->files, sb->ffree,
sb->fsid, (long int)sb->namelen);
- p9_free_req(clnt, req);
+ p9_tag_remove(clnt, req);
error:
return err;
}
@@ -1907,7 +1907,7 @@ int p9_client_rename(struct p9_fid *fid,
p9_debug(P9_DEBUG_9P, "<<< RRENAME fid %d\n", fid->fid);
- p9_free_req(clnt, req);
+ p9_tag_remove(clnt, req);
error:
return err;
}
@@ -1937,7 +1937,7 @@ int p9_client_renameat(struct p9_fid *olddirfid, const char *old_name,
p9_debug(P9_DEBUG_9P, "<<< RRENAMEAT newdirfid %d new name %s\n",
newdirfid->fid, new_name);
- p9_free_req(clnt, req);
+ p9_tag_remove(clnt, req);
error:
return err;
}
@@ -1974,10 +1974,10 @@ struct p9_fid *p9_client_xattrwalk(struct p9_fid *file_fid,
err = p9pdu_readf(&req->rc, clnt->proto_version, "q", attr_size);
if (err) {
trace_9p_protocol_dump(clnt, &req->rc);
- p9_free_req(clnt, req);
+ p9_tag_remove(clnt, req);
goto clunk_fid;
}
- p9_free_req(clnt, req);
+ p9_tag_remove(clnt, req);
p9_debug(P9_DEBUG_9P, "<<< RXATTRWALK fid %d size %llu\n",
attr_fid->fid, *attr_size);
return attr_fid;
@@ -2011,7 +2011,7 @@ int p9_client_xattrcreate(struct p9_fid *fid, const char *name,
goto error;
}
p9_debug(P9_DEBUG_9P, "<<< RXATTRCREATE fid %d\n", fid->fid);
- p9_free_req(clnt, req);
+ p9_tag_remove(clnt, req);
error:
return err;
}
@@ -2074,11 +2074,11 @@ int p9_client_readdir(struct p9_fid *fid, char *data, u32 count, u64 offset)
if (non_zc)
memmove(data, dataptr, count);
- p9_free_req(clnt, req);
+ p9_tag_remove(clnt, req);
return count;
free_and_error:
- p9_free_req(clnt, req);
+ p9_tag_remove(clnt, req);
error:
return err;
}
@@ -2109,7 +2109,7 @@ int p9_client_mknod_dotl(struct p9_fid *fid, const char *name, int mode,
(unsigned long long)qid->path, qid->version);
error:
- p9_free_req(clnt, req);
+ p9_tag_remove(clnt, req);
return err;
}
@@ -2140,7 +2140,7 @@ int p9_client_mkdir_dotl(struct p9_fid *fid, const char *name, int mode,
(unsigned long long)qid->path, qid->version);
error:
- p9_free_req(clnt, req);
+ p9_tag_remove(clnt, req);
return err;
}
@@ -2173,7 +2173,7 @@ int p9_client_lock_dotl(struct p9_fid *fid, struct p9_flock *flock, u8 *status)
}
p9_debug(P9_DEBUG_9P, "<<< RLOCK status %i\n", *status);
error:
- p9_free_req(clnt, req);
+ p9_tag_remove(clnt, req);
return err;
}
@@ -2208,7 +2208,7 @@ int p9_client_getlock_dotl(struct p9_fid *fid, struct p9_getlock *glock)
"proc_id %d client_id %s\n", glock->type, glock->start,
glock->length, glock->proc_id, glock->client_id);
error:
- p9_free_req(clnt, req);
+ p9_tag_remove(clnt, req);
return err;
}
EXPORT_SYMBOL(p9_client_getlock_dotl);
@@ -2234,7 +2234,7 @@ int p9_client_readlink(struct p9_fid *fid, char **target)
}
p9_debug(P9_DEBUG_9P, "<<< RREADLINK target %s\n", *target);
error:
- p9_free_req(clnt, req);
+ p9_tag_remove(clnt, req);
return err;
}
EXPORT_SYMBOL(p9_client_readlink);
--
2.11.0
^ permalink raw reply related
* Re: possible deadlock in rhashtable_lookup_insert_fast
From: Cong Wang @ 2018-08-14 17:39 UTC (permalink / raw)
To: syzbot+b66a5a554991a8ed027c
Cc: David Miller, Greg KH, Kees Cook, Kirill Tkhai, Alexey Kuznetsov,
LKML, Linux Kernel Network Developers, Philippe Ombredanne,
Stephen Hemminger, syzkaller-bugs, Tom Herbert, Hideaki YOSHIFUJI
In-Reply-To: <00000000000043975f05735f33f0@google.com>
On Mon, Aug 13, 2018 at 11:21 PM syzbot
<syzbot+b66a5a554991a8ed027c@syzkaller.appspotmail.com> wrote:
>
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit: 36d2f761b5aa cxgb4: update 1.20.8.0 as the latest firmware..
> git tree: net-next
> console output: https://syzkaller.appspot.com/x/log.txt?x=134b323c400000
> kernel config: https://syzkaller.appspot.com/x/.config?x=dbf55ebfa6bfd517
> dashboard link: https://syzkaller.appspot.com/bug?extid=b66a5a554991a8ed027c
> compiler: gcc (GCC) 8.0.1 20180413 (experimental)
> syzkaller repro:https://syzkaller.appspot.com/x/repro.syz?x=103285f0400000
> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=12859d72400000
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+b66a5a554991a8ed027c@syzkaller.appspotmail.com
>
> random: sshd: uninitialized urandom read (32 bytes read)
> random: sshd: uninitialized urandom read (32 bytes read)
> netlink: 'syz-executor077': attribute type 8 has an invalid length.
>
> ============================================
> WARNING: possible recursive locking detected
> 4.18.0-rc8+ #179 Not tainted
> --------------------------------------------
> syz-executor077/4391 is trying to acquire lock:
> (____ptrval____) (&(&tlocks[i])->rlock){+.+.}, at: spin_lock_bh
> include/linux/spinlock.h:315 [inline]
> (____ptrval____) (&(&tlocks[i])->rlock){+.+.}, at: __rhashtable_insert_fast
> include/linux/rhashtable.h:596 [inline]
> (____ptrval____) (&(&tlocks[i])->rlock){+.+.}, at:
> rhashtable_lookup_insert_fast.constprop.26+0x436/0x13a0
> include/linux/rhashtable.h:784
>
> but task is already holding lock:
> (____ptrval____) (&(&tlocks[i])->rlock){+.+.}, at: spin_lock
> include/linux/spinlock.h:310 [inline]
> (____ptrval____) (&(&tlocks[i])->rlock){+.+.}, at: ila_add_mapping
> net/ipv6/ila/ila_xlat.c:233 [inline]
> (____ptrval____) (&(&tlocks[i])->rlock){+.+.}, at:
> ila_xlat_nl_cmd_add_mapping+0x6bb/0x17e0 net/ipv6/ila/ila_xlat.c:355
>
> other info that might help us debug this:
> Possible unsafe locking scenario:
>
> CPU0
> ----
> lock(&(&tlocks[i])->rlock);
> lock(&(&tlocks[i])->rlock);
>
> *** DEADLOCK ***
>
> May be due to missing lock nesting notation
>
> 3 locks held by syz-executor077/4391:
> #0: (____ptrval____) (cb_lock){++++}, at: genl_rcv+0x19/0x40
> net/netlink/genetlink.c:636
> #1: (____ptrval____) (&(&tlocks[i])->rlock){+.+.}, at: spin_lock
> include/linux/spinlock.h:310 [inline]
> #1: (____ptrval____) (&(&tlocks[i])->rlock){+.+.}, at: ila_add_mapping
> net/ipv6/ila/ila_xlat.c:233 [inline]
> #1: (____ptrval____) (&(&tlocks[i])->rlock){+.+.}, at:
> ila_xlat_nl_cmd_add_mapping+0x6bb/0x17e0 net/ipv6/ila/ila_xlat.c:355
> #2: (____ptrval____) (rcu_read_lock){....}, at: __rhashtable_insert_fast
> include/linux/rhashtable.h:579 [inline]
> #2: (____ptrval____) (rcu_read_lock){....}, at:
> rhashtable_lookup_insert_fast.constprop.26+0x1d7/0x13a0
> include/linux/rhashtable.h:784
>
> stack backtrace:
> CPU: 1 PID: 4391 Comm: syz-executor077 Not tainted 4.18.0-rc8+ #179
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> Google 01/01/2011
> Call Trace:
> __dump_stack lib/dump_stack.c:77 [inline]
> dump_stack+0x1c9/0x2b4 lib/dump_stack.c:113
> print_deadlock_bug kernel/locking/lockdep.c:1765 [inline]
> check_deadlock kernel/locking/lockdep.c:1809 [inline]
> validate_chain kernel/locking/lockdep.c:2405 [inline]
> __lock_acquire.cold.65+0x1fb/0x486 kernel/locking/lockdep.c:3435
> lock_acquire+0x1e4/0x540 kernel/locking/lockdep.c:3924
> __raw_spin_lock_bh include/linux/spinlock_api_smp.h:135 [inline]
> _raw_spin_lock_bh+0x31/0x40 kernel/locking/spinlock.c:168
> spin_lock_bh include/linux/spinlock.h:315 [inline]
> __rhashtable_insert_fast include/linux/rhashtable.h:596 [inline]
> rhashtable_lookup_insert_fast.constprop.26+0x436/0x13a0
> include/linux/rhashtable.h:784
> ila_add_mapping net/ipv6/ila/ila_xlat.c:240 [inline]
> ila_xlat_nl_cmd_add_mapping+0xafe/0x17e0 net/ipv6/ila/ila_xlat.c:355
Purely bogus warning introduced by commit
b893281715ab ("ila: Call library function alloc_bucket_locks").
> genl_family_rcv_msg+0x8a3/0x1140 net/netlink/genetlink.c:601
> genl_rcv_msg+0xc6/0x168 net/netlink/genetlink.c:626
> netlink_rcv_skb+0x172/0x440 net/netlink/af_netlink.c:2454
> genl_rcv+0x28/0x40 net/netlink/genetlink.c:637
> netlink_unicast_kernel net/netlink/af_netlink.c:1317 [inline]
> netlink_unicast+0x5a0/0x760 net/netlink/af_netlink.c:1343
> netlink_sendmsg+0xa18/0xfc0 net/netlink/af_netlink.c:1908
> sock_sendmsg_nosec net/socket.c:640 [inline]
> sock_sendmsg+0xd5/0x120 net/socket.c:650
> ___sys_sendmsg+0x7fd/0x930 net/socket.c:2133
> __sys_sendmsg+0x11d/0x290 net/socket.c:2171
> __do_sys_sendmsg net/socket.c:2180 [inline]
> __se_sys_sendmsg net/socket.c:2178 [inline]
> __x64_sys_sendmsg+0x78/0xb0 net/socket.c:2178
> do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
> entry_SYSCALL_64_after_hwframe+0x49/0xbe
> RIP: 0033:0x4400e9
> Code: 18 89 d0 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 00 48 89 f8 48 89 f7
> 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff
> ff 0f 83 fb 13 fc ff c3 66 2e 0f 1f 84 00 00 00 00
> RSP: 002b:00007ffd14495758 EFLAG
>
>
> ---
> This bug is generated by a bot. It may contain errors.
> See https://goo.gl/tpsmEJ for more information about syzbot.
> syzbot engineers can be reached at syzkaller@googlegroups.com.
>
> syzbot will keep track of this bug report. See:
> https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with
> syzbot.
> syzbot can test patches for this bug, for details see:
> https://goo.gl/tpsmEJ#testing-patches
^ permalink raw reply
* Re: [PATCH] dt-bindings: net: ravb: Add support for r8a774a1 SoC
From: Sergei Shtylyov @ 2018-08-14 17:38 UTC (permalink / raw)
To: Fabrizio Castro, Rob Herring, Mark Rutland
Cc: David S. Miller, Geert Uytterhoeven, Simon Horman, Biju Das,
Yoshihiro Shimoda, netdev, linux-renesas-soc, devicetree,
linux-kernel, Simon Horman, Chris Paterson
In-Reply-To: <1534250017-15725-1-git-send-email-fabrizio.castro@bp.renesas.com>
Hello!
On 08/14/2018 03:33 PM, Fabrizio Castro wrote:
> 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>
[...]
Reviewed-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
MBR, Sergei
^ 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