Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net-next 06/19] net: usb: aqc111: Introduce link management
From: Andrew Lunn @ 2018-10-05 17:11 UTC (permalink / raw)
  To: Igor Russkikh
  Cc: David S . Miller, linux-usb@vger.kernel.org,
	netdev@vger.kernel.org, Dmitry Bezrukov
In-Reply-To: <cec5cd88988e0985e3fdd1343906ef649b01f646.1538734658.git.igor.russkikh@aquantia.com>

On Fri, Oct 05, 2018 at 10:24:55AM +0000, Igor Russkikh wrote:
> From: Dmitry Bezrukov <dmitry.bezrukov@aquantia.com>
> 
> Add full hardware initialization sequence and link configuration logic

Hi Igor, Dmitry

Please could you explain why you decided to not use drivers/net/phy?
The previous patch introduced basically what you need to export a
standard Linux MDIO bus. From that you can use a standard Linux PHY
driver.

	Andrew

^ permalink raw reply

* Re: [PATCH iproute2 net-next v2 2/6] include: Add helper to retrieve a __s64 from a netlink msg
From: Vinicius Costa Gomes @ 2018-10-05 17:08 UTC (permalink / raw)
  To: Ilias Apalodimas
  Cc: netdev, jhs, xiyou.wangcong, jiri, jesus.sanchez-palencia,
	simon.fok
In-Reply-To: <20181005084107.GA16936@apalos>

Hi Ilias,

Ilias Apalodimas <ilias.apalodimas@linaro.org> writes:

> On Thu, Oct 04, 2018 at 04:17:07PM -0700, Vinicius Costa Gomes wrote:
>> This allows signed 64-bit integers to be retrieved from a netlink
>> message.
>> ---
>>  include/libnetlink.h | 7 +++++++
>>  1 file changed, 7 insertions(+)
>> 
>> diff --git a/include/libnetlink.h b/include/libnetlink.h
>> index 9d9249e6..88164975 100644
>> --- a/include/libnetlink.h
>> +++ b/include/libnetlink.h
>> @@ -185,6 +185,13 @@ static inline __u64 rta_getattr_u64(const struct rtattr *rta)
>>  	memcpy(&tmp, RTA_DATA(rta), sizeof(__u64));
>>  	return tmp;
>>  }
>> +static inline __s64 rta_getattr_s64(const struct rtattr *rta)
>> +{
>> +	__s64 tmp;
>> +
>> +	memcpy(&tmp, RTA_DATA(rta), sizeof(__s64));
> Maybe change it to sizeof(tmp)?

Will fix. Thanks.

>> +	return tmp;
>> +}
>>  static inline const char *rta_getattr_str(const struct rtattr *rta)
>>  {
>>  	return (const char *)RTA_DATA(rta);
>> -- 
>> 2.19.0
>> 


Cheers,

^ permalink raw reply

* [PATCH net-next 6/6] net: hns3: Fix for rx vlan id handle to support Rev 0x21 hardware
From: Salil Mehta @ 2018-10-05 17:03 UTC (permalink / raw)
  To: davem
  Cc: salil.mehta, yisen.zhuang, lipeng321, mehta.salil, netdev,
	linux-kernel, linuxarm, Jian Shen
In-Reply-To: <20181005170329.6512-1-salil.mehta@huawei.com>

From: Jian Shen <shenjian15@huawei.com>

In revision 0x20, we use vlan id != 0 to check whether a vlan tag
has been offloaded, so vlan id 0 is not supported.

In revision 0x21, rx buffer descriptor adds two bits to indicate
whether one or more vlan tags have been offloaded, so vlan id 0
is valid now.

This patch seperates the handle for vlan id 0, add vlan id 0 support
for revision 0x21.

Fixes: 5b5455a9ed5a ("net: hns3: Add STRP_TAGP field support for hardware revision 0x21")
Signed-off-by: Jian Shen <shenjian15@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
 drivers/net/ethernet/hisilicon/hns3/hns3_enet.c | 30 +++++++++++--------------
 1 file changed, 13 insertions(+), 17 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
index 3f36c38..e9d4564 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
@@ -2200,18 +2200,18 @@ static void hns3_rx_skb(struct hns3_enet_ring *ring, struct sk_buff *skb)
 	napi_gro_receive(&ring->tqp_vector->napi, skb);
 }
 
-static u16 hns3_parse_vlan_tag(struct hns3_enet_ring *ring,
-			       struct hns3_desc *desc, u32 l234info)
+static bool hns3_parse_vlan_tag(struct hns3_enet_ring *ring,
+				struct hns3_desc *desc, u32 l234info,
+				u16 *vlan_tag)
 {
 	struct pci_dev *pdev = ring->tqp->handle->pdev;
-	u16 vlan_tag;
 
 	if (pdev->revision == 0x20) {
-		vlan_tag = le16_to_cpu(desc->rx.ot_vlan_tag);
-		if (!(vlan_tag & VLAN_VID_MASK))
-			vlan_tag = le16_to_cpu(desc->rx.vlan_tag);
+		*vlan_tag = le16_to_cpu(desc->rx.ot_vlan_tag);
+		if (!(*vlan_tag & VLAN_VID_MASK))
+			*vlan_tag = le16_to_cpu(desc->rx.vlan_tag);
 
-		return vlan_tag;
+		return (*vlan_tag != 0);
 	}
 
 #define HNS3_STRP_OUTER_VLAN	0x1
@@ -2220,17 +2220,14 @@ static u16 hns3_parse_vlan_tag(struct hns3_enet_ring *ring,
 	switch (hnae3_get_field(l234info, HNS3_RXD_STRP_TAGP_M,
 				HNS3_RXD_STRP_TAGP_S)) {
 	case HNS3_STRP_OUTER_VLAN:
-		vlan_tag = le16_to_cpu(desc->rx.ot_vlan_tag);
-		break;
+		*vlan_tag = le16_to_cpu(desc->rx.ot_vlan_tag);
+		return true;
 	case HNS3_STRP_INNER_VLAN:
-		vlan_tag = le16_to_cpu(desc->rx.vlan_tag);
-		break;
+		*vlan_tag = le16_to_cpu(desc->rx.vlan_tag);
+		return true;
 	default:
-		vlan_tag = 0;
-		break;
+		return false;
 	}
-
-	return vlan_tag;
 }
 
 static int hns3_handle_rx_bd(struct hns3_enet_ring *ring,
@@ -2332,8 +2329,7 @@ static int hns3_handle_rx_bd(struct hns3_enet_ring *ring,
 	if (netdev->features & NETIF_F_HW_VLAN_CTAG_RX) {
 		u16 vlan_tag;
 
-		vlan_tag = hns3_parse_vlan_tag(ring, desc, l234info);
-		if (vlan_tag & VLAN_VID_MASK)
+		if (hns3_parse_vlan_tag(ring, desc, l234info, &vlan_tag))
 			__vlan_hwaccel_put_tag(skb,
 					       htons(ETH_P_8021Q),
 					       vlan_tag);
-- 
2.7.4

^ permalink raw reply related

* [PATCH net-next 5/6] net: hns3: Add egress/ingress vlan filter for revision 0x21
From: Salil Mehta @ 2018-10-05 17:03 UTC (permalink / raw)
  To: davem
  Cc: salil.mehta, yisen.zhuang, lipeng321, mehta.salil, netdev,
	linux-kernel, linuxarm, Zhongzhu Liu, Jian Shen
In-Reply-To: <20181005170329.6512-1-salil.mehta@huawei.com>

From: Zhongzhu Liu <liuzhongzhu@huawei.com>

In revision 0x21, hw supports both ingress and egress vlan filter.
This patch adds support for it.

Signed-off-by: Zhongzhu Liu <liuzhongzhu@huawei.com>
Signed-off-by: Jian Shen <shenjian15@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
 .../ethernet/hisilicon/hns3/hns3pf/hclge_main.c    | 45 +++++++++++++++++-----
 1 file changed, 36 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
index d95ea9b..ca1a936 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
@@ -5523,7 +5523,7 @@ static int hclge_do_ioctl(struct hnae3_handle *handle, struct ifreq *ifr,
 }
 
 static int hclge_set_vlan_filter_ctrl(struct hclge_dev *hdev, u8 vlan_type,
-				      bool filter_en)
+				      u8 fe_type, bool filter_en)
 {
 	struct hclge_vlan_filter_ctrl_cmd *req;
 	struct hclge_desc desc;
@@ -5533,7 +5533,7 @@ static int hclge_set_vlan_filter_ctrl(struct hclge_dev *hdev, u8 vlan_type,
 
 	req = (struct hclge_vlan_filter_ctrl_cmd *)desc.data;
 	req->vlan_type = vlan_type;
-	req->vlan_fe = filter_en;
+	req->vlan_fe = filter_en ? fe_type : 0;
 
 	ret = hclge_cmd_send(&hdev->hw, &desc, 1);
 	if (ret)
@@ -5545,13 +5545,30 @@ static int hclge_set_vlan_filter_ctrl(struct hclge_dev *hdev, u8 vlan_type,
 
 #define HCLGE_FILTER_TYPE_VF		0
 #define HCLGE_FILTER_TYPE_PORT		1
+#define HCLGE_FILTER_FE_EGRESS_V1_B	BIT(0)
+#define HCLGE_FILTER_FE_NIC_INGRESS_B	BIT(0)
+#define HCLGE_FILTER_FE_NIC_EGRESS_B	BIT(1)
+#define HCLGE_FILTER_FE_ROCE_INGRESS_B	BIT(2)
+#define HCLGE_FILTER_FE_ROCE_EGRESS_B	BIT(3)
+#define HCLGE_FILTER_FE_EGRESS		(HCLGE_FILTER_FE_NIC_EGRESS_B \
+					| HCLGE_FILTER_FE_ROCE_EGRESS_B)
+#define HCLGE_FILTER_FE_INGRESS		(HCLGE_FILTER_FE_NIC_INGRESS_B \
+					| HCLGE_FILTER_FE_ROCE_INGRESS_B)
 
 static void hclge_enable_vlan_filter(struct hnae3_handle *handle, bool enable)
 {
 	struct hclge_vport *vport = hclge_get_vport(handle);
 	struct hclge_dev *hdev = vport->back;
 
-	hclge_set_vlan_filter_ctrl(hdev, HCLGE_FILTER_TYPE_VF, enable);
+	if (hdev->pdev->revision >= 0x21) {
+		hclge_set_vlan_filter_ctrl(hdev, HCLGE_FILTER_TYPE_VF,
+					   HCLGE_FILTER_FE_EGRESS, enable);
+		hclge_set_vlan_filter_ctrl(hdev, HCLGE_FILTER_TYPE_PORT,
+					   HCLGE_FILTER_FE_INGRESS, enable);
+	} else {
+		hclge_set_vlan_filter_ctrl(hdev, HCLGE_FILTER_TYPE_VF,
+					   HCLGE_FILTER_FE_EGRESS_V1_B, enable);
+	}
 }
 
 static int hclge_set_vf_vlan_common(struct hclge_dev *hdev, int vfid,
@@ -5853,13 +5870,23 @@ static int hclge_init_vlan_config(struct hclge_dev *hdev)
 	int ret;
 	int i;
 
-	ret = hclge_set_vlan_filter_ctrl(hdev, HCLGE_FILTER_TYPE_VF, true);
-	if (ret)
-		return ret;
+	if (hdev->pdev->revision >= 0x21) {
+		ret = hclge_set_vlan_filter_ctrl(hdev, HCLGE_FILTER_TYPE_VF,
+						 HCLGE_FILTER_FE_EGRESS, true);
+		if (ret)
+			return ret;
 
-	ret = hclge_set_vlan_filter_ctrl(hdev, HCLGE_FILTER_TYPE_PORT, true);
-	if (ret)
-		return ret;
+		ret = hclge_set_vlan_filter_ctrl(hdev, HCLGE_FILTER_TYPE_PORT,
+						 HCLGE_FILTER_FE_INGRESS, true);
+		if (ret)
+			return ret;
+	} else {
+		ret = hclge_set_vlan_filter_ctrl(hdev, HCLGE_FILTER_TYPE_VF,
+						 HCLGE_FILTER_FE_EGRESS_V1_B,
+						 true);
+		if (ret)
+			return ret;
+	}
 
 	hdev->vlan_type_cfg.rx_in_fst_vlan_type = HCLGE_DEF_VLAN_TYPE;
 	hdev->vlan_type_cfg.rx_in_sec_vlan_type = HCLGE_DEF_VLAN_TYPE;
-- 
2.7.4

^ permalink raw reply related

* [PATCH net-next] fib_tests: Add tests for invalid metric on route
From: David Ahern @ 2018-10-05 17:01 UTC (permalink / raw)
  To: netdev, davem; +Cc: eric.dumazet, David Ahern

From: David Ahern <dsahern@gmail.com>

Add ipv4 and ipv6 test cases with an invalid metrics option causing
ip_metrics_convert to fail. Tests clean up path during route add.

Also, add nodad to to ipv6 address add. When running ipv6_route_metrics
directly seeing an occasional failure on the "Using route with mtu metric"
test case.

Signed-off-by: David Ahern <dsahern@gmail.com>
---
This test catches the error Eric reported and verifies the fix.

 tools/testing/selftests/net/fib_tests.sh | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/tools/testing/selftests/net/fib_tests.sh b/tools/testing/selftests/net/fib_tests.sh
index 491332713dd9..802b4af18729 100755
--- a/tools/testing/selftests/net/fib_tests.sh
+++ b/tools/testing/selftests/net/fib_tests.sh
@@ -711,14 +711,14 @@ route_setup()
 	ip -netns ns2 li add dummy1 type dummy
 	ip -netns ns2 li set dummy1 up
 
-	$IP -6 addr add 2001:db8:101::1/64 dev veth1
-	$IP -6 addr add 2001:db8:103::1/64 dev veth3
+	$IP -6 addr add 2001:db8:101::1/64 dev veth1 nodad
+	$IP -6 addr add 2001:db8:103::1/64 dev veth3 nodad
 	$IP addr add 172.16.101.1/24 dev veth1
 	$IP addr add 172.16.103.1/24 dev veth3
 
-	ip -netns ns2 -6 addr add 2001:db8:101::2/64 dev veth2
-	ip -netns ns2 -6 addr add 2001:db8:103::2/64 dev veth4
-	ip -netns ns2 -6 addr add 2001:db8:104::1/64 dev dummy1
+	ip -netns ns2 -6 addr add 2001:db8:101::2/64 dev veth2 nodad
+	ip -netns ns2 -6 addr add 2001:db8:103::2/64 dev veth4 nodad
+	ip -netns ns2 -6 addr add 2001:db8:104::1/64 dev dummy1 nodad
 
 	ip -netns ns2 addr add 172.16.101.2/24 dev veth2
 	ip -netns ns2 addr add 172.16.103.2/24 dev veth4
@@ -1043,6 +1043,9 @@ ipv6_route_metrics_test()
 	run_cmd "ip netns exec ns1 ping6 -w1 -c1 -s 1500 2001:db8:104::1"
 	log_test $? 0 "Using route with mtu metric"
 
+	run_cmd "$IP -6 ro add 2001:db8:114::/64 via  2001:db8:101::2  congctl lock foo"
+	log_test $? 2 "Invalid metric (fails metric_convert)"
+
 	route_cleanup
 }
 
@@ -1432,6 +1435,9 @@ ipv4_route_metrics_test()
 	run_cmd "ip netns exec ns1 ping -w1 -c1 -s 1500 172.16.104.1"
 	log_test $? 0 "Using route with mtu metric"
 
+	run_cmd "$IP ro add 172.16.111.0/24 via 172.16.101.2 congctl lock foo"
+	log_test $? 2 "Invalid metric (fails metric_convert)"
+
 	route_cleanup
 }
 
-- 
2.11.0

^ permalink raw reply related

* RE: [RFC PATCH] skb: Define NET_IP_ALIGN based on CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
From: David Laight @ 2018-10-05 16:59 UTC (permalink / raw)
  To: 'Ben Hutchings', netdev@vger.kernel.org
  Cc: Ben Dooks, linux-kernel@lists.codethink.co.uk,
	linux-arm-kernel@lists.infradead.org, linux-s390@vger.kernel.org
In-Reply-To: <20181004173631.3nchegr6rm3jgz24@xylophone.i.decadent.org.uk>

From: Ben Hutchings
> Sent: 04 October 2018 18:37
> 
> NET_IP_ALIGN is supposed to be defined as 0 if DMA writes to an
> unaligned buffer would be more expensive than CPU access to unaligned
> header fields, and otherwise defined as 2.
> 
> Currently only ppc64 and x86 configurations define it to be 0.
> However several other architectures (conditionally) define
> CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS, which seems to imply that
> NET_IP_ALIGN should be 0.
> 
> Remove the overriding definitions for ppc64 and x86 and define
> NET_IP_ALIGN solely based on CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS.

Even if CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS is set unaligned
accesses are likely to be slightly slower than aligned ones.
So having NET_IP_ALIGN set to 2 might make sense even on x86.
(ISTR DM saying why this isn't done.)

I've also met systems when misaligned DMA transfers (for ethernet receive)
were so bad that is was necessary to DMA to a 4n aligned buffer and
then do a misaligned copy to the real rx buffer (skb equiv) for the
network stack - which required the buffer be 4n+2 aligned.
(sparc sbus with the original DMA part.)

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

^ permalink raw reply

* Re: [PATCH bpf-next 1/6] bpf: introduce BPF_PROG_TYPE_FILE_FILTER
From: Al Viro @ 2018-10-05 23:47 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Andy Lutomirski, Alexei Starovoitov, David S. Miller,
	Daniel Borkmann, Network Development, LKML, kernel-team
In-Reply-To: <20181005222752.l5da54rpww6tlyfy@ast-mbp.dhcp.thefacebook.com>

On Fri, Oct 05, 2018 at 03:27:54PM -0700, Alexei Starovoitov wrote:
> On Fri, Oct 05, 2018 at 03:09:20PM -0700, Andy Lutomirski wrote:
> > On Fri, Oct 5, 2018 at 3:05 PM Alexei Starovoitov
> > <alexei.starovoitov@gmail.com> wrote:
> > >
> > > On Fri, Oct 05, 2018 at 05:46:59AM +0100, Al Viro wrote:
> > >
> > > > Another problem is your direct poking in ->i_ino.  It's not
> > > > something directly exposed to userland at the moment and it should
> > > > not become such.
> > >
> > > The patch is not making i_ino directly exposed.
> > > Only 'struct bpf_file_info' is exposed to user space / bpf programs.
> > 
> > I think Al is saying that the valie of i_ino is not something that
> > user code is permitted to know regardless of how you format it because
> > it may or may not actually match the value returned by stat().
> > Another way of saying that is that your patch is digging into an
> > internal data structure and is doing it wrong.
> 
> several fs implementation I've looked at just do generic_fillattr()
> for these fields. Are you saying some FS don't use inode->i_ino at all?
> And it's bogus, hence shouldn't be read?

Bloody wonderful...  "Several instances use a library function and do not
override that part of its results; ergo, let's assume that all of them
do the same".

generic_fillattr() is a library function.  In a lot of cases this is all
->getattr() instance needs to do.  And yes, use of ->i_dev and ->i_ino
to intialize ->st_dev and ->st_ino happens to be the default.  However,
this is entirely up to the filesystem in question.  These fields are
fs-private; whether to use them for stat(2) (or anything userland-visible,
really) or to calculate some other value is up to individual filesystem.

FWIW, finding which instances do that is as simple as
grep -n '[-]>[[:space:]]*ino[[:space:]]*=' `git grep -l -w generic_fillattr`
on the plausible theory that ->getattr() instances will be using that helper
at least for some of the fields.  Discarding fs/stat.c, where generic_fillattr()
itself lives, we are left with
fs/ceph/inode.c:558:            if (realm->ino == ci->i_vino.ino)
fs/ceph/inode.c:2268:           stat->ino = ceph_translate_ino(inode->i_sb, inode->i_ino);
fs/cifs/inode.c:2067:   stat->ino = CIFS_I(inode)->uniqueid;
fs/fat/file.c:410:              stat->ino = fat_i_pos_read(MSDOS_SB(inode->i_sb), inode);
fs/fuse/dir.c:866:      stat->ino = attr->ino;
fs/fuse/dir.c:954:              stat->ino = fi->orig_ino;
fs/nfs/inode.c:841:     stat->ino = nfs_compat_user_ino64(NFS_FILEID(inode));
Trivial examination shows that all matches except the first one *are* in ->getattr()
instances of the filesystems in question or are called from such.

And no, it's not that each of those filesystems does not use inode->i_ino at all.
There's a bunch of library helpers in fs/*.c that happen to use the value filesystem
has seen fit to store there.  Whether to use those helpers or not, what to store in
that field, etc. is, again, entirely up to the filesystem in question.
generic_fillattr() is one of those, that's all there is to it.

That's precisely why I really do not like the idea of hooks poking in the internals
of kernel data structures.  Especially since not even "it's not visible outside of
a subsystem-internal header" appears to slow you down.

The same goes for tracepoints, etc. - turning random details of implementation into
a carved in stone ABI is actively harmful.

NAK.

PS: If anything, visibility to hooks should be opt-in.  Sure, we can start actively hiding
the things, but that's a winless arms race  - even if we bloody went and encrypted the
private stuff, you'd still be able to pull decryption key from where it would be accessed
by legitimate users, etc.  Нахуй нам это надо?

^ permalink raw reply

* Re: [PATCH bpf-next] xsk: proper AF_XDP socket teardown ordering
From: Song Liu @ 2018-10-05 16:41 UTC (permalink / raw)
  To: bjorn.topel
  Cc: Alexei Starovoitov, Daniel Borkmann, Networking,
	Jesper Dangaard Brouer, Björn Töpel, Magnus Karlsson,
	magnus.karlsson
In-Reply-To: <20181005112515.3009-1-bjorn.topel@gmail.com>

On Fri, Oct 5, 2018 at 4:28 AM Björn Töpel <bjorn.topel@gmail.com> wrote:
>
> From: Björn Töpel <bjorn.topel@intel.com>
>
> The AF_XDP socket struct can exist in three different, implicit
> states: setup, bound and released. Setup is prior the socket has been
> bound to a device. Bound is when the socket is active for receive and
> send. Released is when the process/userspace side of the socket is
> released, but the sock object is still lingering, e.g. when there is a
> reference to the socket in an XSKMAP after process termination.
>
> The Rx fast-path code uses the "dev" member of struct xdp_sock to
> check whether a socket is bound or relased, and the Tx code uses the
> struct xdp_umem "xsk_list" member in conjunction with "dev" to
> determine the state of a socket.
>
> However, the transition from bound to released did not tear the socket
> down in correct order.
>
> On the Rx side "dev" was cleared after synchronize_net() making the
> synchronization useless. On the Tx side, the internal queues were
> destroyed prior removing them from the "xsk_list".
>
> This commit corrects the cleanup order, and by doing so
> xdp_del_sk_umem() can be simplified and one synchronize_net() can be
> removed.
>
> Fixes: 965a99098443 ("xsk: add support for bind for Rx")
> Fixes: ac98d8aab61b ("xsk: wire upp Tx zero-copy functions")
> Reported-by: Jesper Dangaard Brouer <brouer@redhat.com>
> Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
Acked-by: Song Liu <songliubraving@fb.com>



> ---
>  net/xdp/xdp_umem.c | 11 +++--------
>  net/xdp/xsk.c      | 13 ++++++++-----
>  2 files changed, 11 insertions(+), 13 deletions(-)
>
> diff --git a/net/xdp/xdp_umem.c b/net/xdp/xdp_umem.c
> index c6007c58231c..a264cf2accd0 100644
> --- a/net/xdp/xdp_umem.c
> +++ b/net/xdp/xdp_umem.c
> @@ -32,14 +32,9 @@ void xdp_del_sk_umem(struct xdp_umem *umem, struct xdp_sock *xs)
>  {
>         unsigned long flags;
>
> -       if (xs->dev) {
> -               spin_lock_irqsave(&umem->xsk_list_lock, flags);
> -               list_del_rcu(&xs->list);
> -               spin_unlock_irqrestore(&umem->xsk_list_lock, flags);
> -
> -               if (umem->zc)
> -                       synchronize_net();
> -       }
> +       spin_lock_irqsave(&umem->xsk_list_lock, flags);
> +       list_del_rcu(&xs->list);
> +       spin_unlock_irqrestore(&umem->xsk_list_lock, flags);
>  }
>
>  /* The umem is stored both in the _rx struct and the _tx struct as we do
> diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> index caeddad15b7c..0577cd49aa72 100644
> --- a/net/xdp/xsk.c
> +++ b/net/xdp/xsk.c
> @@ -355,12 +355,18 @@ static int xsk_release(struct socket *sock)
>         local_bh_enable();
>
>         if (xs->dev) {
> +               struct net_device *dev = xs->dev;
> +
>                 /* Wait for driver to stop using the xdp socket. */
nit: I guess we should move this comment together with synchronize_net().

> -               synchronize_net();
> -               dev_put(xs->dev);
> +               xdp_del_sk_umem(xs->umem, xs);
>                 xs->dev = NULL;
> +               synchronize_net();
> +               dev_put(dev);
>         }
>
> +       xskq_destroy(xs->rx);
> +       xskq_destroy(xs->tx);
> +
>         sock_orphan(sk);
>         sock->sk = NULL;
>
> @@ -714,9 +720,6 @@ static void xsk_destruct(struct sock *sk)
>         if (!sock_flag(sk, SOCK_DEAD))
>                 return;
>
> -       xskq_destroy(xs->rx);
> -       xskq_destroy(xs->tx);
> -       xdp_del_sk_umem(xs->umem, xs);
>         xdp_put_umem(xs->umem);
>
>         sk_refcnt_debug_dec(sk);
> --
> 2.17.1
>

^ permalink raw reply

* [PATCH bpf-next] libbpf: relicense libbpf as LGPL-2.1 OR BSD-2-Clause
From: Alexei Starovoitov @ 2018-10-05 23:40 UTC (permalink / raw)
  To: davem; +Cc: daniel, acme, netdev, linux-kernel, kernel-team

libbpf is maturing as a library and gaining features that no other bpf libraries support
(BPF Type Format, bpf to bpf calls, etc)
Many Apache2 licensed projects (like bcc, bpftrace, gobpf, cilium, etc)
would like to use libbpf, but cannot do this yet, since Apache Foundation explicitly
states that LGPL is incompatible with Apache2.
Hence let's relicense libbpf as dual license LGPL-2.1 or BSD-2-Clause,
since BSD-2 is compatible with Apache2.
Dual LGPL or Apache2 is invalid combination.
Fix license mistake in Makefile as well.

Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Acked-by: Andrey Ignatov <rdna@fb.com>
Acked-by: Arnaldo Carvalho de Melo <acme@kernel.org>
Acked-by: Björn Töpel <bjorn.topel@intel.com>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: David Beckett <david.beckett@netronome.com>
Acked-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Acked-by: Joe Stringer <joe@ovn.org>
Acked-by: John Fastabend <john.fastabend@gmail.com>
Acked-by: Martin KaFai Lau <kafai@fb.com>
Acked-by: Quentin Monnet <quentin.monnet@netronome.com>
Acked-by: Thomas Graf <tgraf@suug.ch>
Acked-by: Roman Gushchin <guro@fb.com>
Acked-by: Wang Nan <wangnan0@huawei.com>
Acked-by: Yonghong Song <yhs@fb.com>
---
vger rejected previous patch. resending with shorter cc list.

 tools/lib/bpf/Makefile       |  2 +-
 tools/lib/bpf/bpf.c          |  2 +-
 tools/lib/bpf/bpf.h          |  2 +-
 tools/lib/bpf/btf.c          |  2 +-
 tools/lib/bpf/btf.h          |  2 +-
 tools/lib/bpf/libbpf.c       | 15 +--------------
 tools/lib/bpf/libbpf.h       | 15 +--------------
 tools/lib/bpf/libbpf_errno.c | 15 +--------------
 tools/lib/bpf/netlink.c      |  2 +-
 tools/lib/bpf/nlattr.c       |  7 +------
 tools/lib/bpf/nlattr.h       |  7 +------
 tools/lib/bpf/str_error.c    |  2 +-
 tools/lib/bpf/str_error.h    |  2 +-
 13 files changed, 13 insertions(+), 62 deletions(-)

diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile
index d49902e818b5..6ad27257fd67 100644
--- a/tools/lib/bpf/Makefile
+++ b/tools/lib/bpf/Makefile
@@ -1,4 +1,4 @@
-# SPDX-License-Identifier: GPL-2.0
+# SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause)
 # Most of this file is copied from tools/lib/traceevent/Makefile
 
 BPF_VERSION = 0
diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
index 3878a26a2071..d70a255cb05e 100644
--- a/tools/lib/bpf/bpf.c
+++ b/tools/lib/bpf/bpf.c
@@ -1,4 +1,4 @@
-// SPDX-License-Identifier: LGPL-2.1
+// SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause)
 
 /*
  * common eBPF ELF operations.
diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
index 4c78f61b7c71..87520a87a75f 100644
--- a/tools/lib/bpf/bpf.h
+++ b/tools/lib/bpf/bpf.h
@@ -1,4 +1,4 @@
-/* SPDX-License-Identifier: LGPL-2.1 */
+/* SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause) */
 
 /*
  * common eBPF ELF operations.
diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
index cf94b0770522..449591aa9900 100644
--- a/tools/lib/bpf/btf.c
+++ b/tools/lib/bpf/btf.c
@@ -1,4 +1,4 @@
-// SPDX-License-Identifier: LGPL-2.1
+// SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause)
 /* Copyright (c) 2018 Facebook */
 
 #include <stdlib.h>
diff --git a/tools/lib/bpf/btf.h b/tools/lib/bpf/btf.h
index d5d20682eeb6..6db5462bb2ef 100644
--- a/tools/lib/bpf/btf.h
+++ b/tools/lib/bpf/btf.h
@@ -1,4 +1,4 @@
-/* SPDX-License-Identifier: LGPL-2.1 */
+/* SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause) */
 /* Copyright (c) 2018 Facebook */
 
 #ifndef __LIBBPF_BTF_H
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 85de1ebd4cb0..ceb918c14d80 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -1,4 +1,4 @@
-// SPDX-License-Identifier: LGPL-2.1
+// SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause)
 
 /*
  * Common eBPF ELF object loading operations.
@@ -7,19 +7,6 @@
  * Copyright (C) 2015 Wang Nan <wangnan0@huawei.com>
  * Copyright (C) 2015 Huawei Inc.
  * Copyright (C) 2017 Nicira, Inc.
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Lesser General Public
- * License as published by the Free Software Foundation;
- * version 2.1 of the License (not later!)
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public
- * License along with this program; if not,  see <http://www.gnu.org/licenses>
  */
 
 #define _GNU_SOURCE
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index fbfc2aec0f0d..8af8d3663991 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -1,4 +1,4 @@
-/* SPDX-License-Identifier: LGPL-2.1 */
+/* SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause) */
 
 /*
  * Common eBPF ELF object loading operations.
@@ -6,19 +6,6 @@
  * Copyright (C) 2013-2015 Alexei Starovoitov <ast@kernel.org>
  * Copyright (C) 2015 Wang Nan <wangnan0@huawei.com>
  * Copyright (C) 2015 Huawei Inc.
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Lesser General Public
- * License as published by the Free Software Foundation;
- * version 2.1 of the License (not later!)
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public
- * License along with this program; if not,  see <http://www.gnu.org/licenses>
  */
 #ifndef __LIBBPF_LIBBPF_H
 #define __LIBBPF_LIBBPF_H
diff --git a/tools/lib/bpf/libbpf_errno.c b/tools/lib/bpf/libbpf_errno.c
index 2464ade3b326..d83b17f8435c 100644
--- a/tools/lib/bpf/libbpf_errno.c
+++ b/tools/lib/bpf/libbpf_errno.c
@@ -1,23 +1,10 @@
-// SPDX-License-Identifier: LGPL-2.1
+// SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause)
 
 /*
  * Copyright (C) 2013-2015 Alexei Starovoitov <ast@kernel.org>
  * Copyright (C) 2015 Wang Nan <wangnan0@huawei.com>
  * Copyright (C) 2015 Huawei Inc.
  * Copyright (C) 2017 Nicira, Inc.
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Lesser General Public
- * License as published by the Free Software Foundation;
- * version 2.1 of the License (not later!)
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public
- * License along with this program; if not,  see <http://www.gnu.org/licenses>
  */
 
 #include <stdio.h>
diff --git a/tools/lib/bpf/netlink.c b/tools/lib/bpf/netlink.c
index 2d2edbbd8ae8..0ce67aea8f3b 100644
--- a/tools/lib/bpf/netlink.c
+++ b/tools/lib/bpf/netlink.c
@@ -1,4 +1,4 @@
-// SPDX-License-Identifier: LGPL-2.1
+// SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause)
 /* Copyright (c) 2018 Facebook */
 
 #include <stdlib.h>
diff --git a/tools/lib/bpf/nlattr.c b/tools/lib/bpf/nlattr.c
index e52257a7367a..1e69c0c8d413 100644
--- a/tools/lib/bpf/nlattr.c
+++ b/tools/lib/bpf/nlattr.c
@@ -1,13 +1,8 @@
-// SPDX-License-Identifier: LGPL-2.1
+// SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause)
 
 /*
  * NETLINK      Netlink attributes
  *
- *	This library is free software; you can redistribute it and/or
- *	modify it under the terms of the GNU Lesser General Public
- *	License as published by the Free Software Foundation version 2.1
- *	of the License.
- *
  * Copyright (c) 2003-2013 Thomas Graf <tgraf@suug.ch>
  */
 
diff --git a/tools/lib/bpf/nlattr.h b/tools/lib/bpf/nlattr.h
index 7198584a3040..6cc3ac91690f 100644
--- a/tools/lib/bpf/nlattr.h
+++ b/tools/lib/bpf/nlattr.h
@@ -1,13 +1,8 @@
-/* SPDX-License-Identifier: LGPL-2.1 */
+/* SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause) */
 
 /*
  * NETLINK      Netlink attributes
  *
- *	This library is free software; you can redistribute it and/or
- *	modify it under the terms of the GNU Lesser General Public
- *	License as published by the Free Software Foundation version 2.1
- *	of the License.
- *
  * Copyright (c) 2003-2013 Thomas Graf <tgraf@suug.ch>
  */
 
diff --git a/tools/lib/bpf/str_error.c b/tools/lib/bpf/str_error.c
index 3d211b642cb5..00e48ac5b806 100644
--- a/tools/lib/bpf/str_error.c
+++ b/tools/lib/bpf/str_error.c
@@ -1,4 +1,4 @@
-// SPDX-License-Identifier: LGPL-2.1
+// SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause)
 #undef _GNU_SOURCE
 #include <string.h>
 #include <stdio.h>
diff --git a/tools/lib/bpf/str_error.h b/tools/lib/bpf/str_error.h
index b9157f5eebde..a139334d57b6 100644
--- a/tools/lib/bpf/str_error.h
+++ b/tools/lib/bpf/str_error.h
@@ -1,4 +1,4 @@
-// SPDX-License-Identifier: LGPL-2.1
+/* SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause) */
 #ifndef __LIBBPF_STR_ERROR_H
 #define __LIBBPF_STR_ERROR_H
 
-- 
2.17.1

^ permalink raw reply related

* Re: [PATCH net] ipv6: do not leave garbage in rt->fib6_metrics
From: Eric Dumazet @ 2018-10-05 16:33 UTC (permalink / raw)
  To: David Ahern; +Cc: David Miller, netdev, Eric Dumazet
In-Reply-To: <5c82ffec-bb35-9bc9-38f4-95962691641a@gmail.com>

On Fri, Oct 5, 2018 at 9:29 AM David Ahern <dsahern@gmail.com> wrote:
>
> On 10/5/18 10:17 AM, Eric Dumazet wrote:
> > In case ip_fib_metrics_init() returns an error, we better
> > rewrite rt->fib6_metrics with &dst_default_metrics so that
> > we do not crash later in ip_fib_metrics_put()
> >
> > Fixes: 767a2217533f ("net: common metrics init helper for FIB entries")
> > Signed-off-by: Eric Dumazet <edumazet@google.com>
> > Reported-by: syzbot <syzkaller@googlegroups.com>
>
> Where is syzbot sending the reports? I don't see it on netdev. I would
> like to understand how it triggered the failure of a 68-byte malloc.
>

Not a public syzbot instance.

And this is not a 68-byte malloc error, simply a ip_metrics_convert()
being cautious with user provided attributes.

You can trivially trigger this with ip command

>
> > ---
> >  net/ipv6/route.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> > index 6c1d817151cae45421dc976c5ea082b4115650be..74d97addf1af20dda0c2b6a2018e88696f9f7d5a 100644
> > --- a/net/ipv6/route.c
> > +++ b/net/ipv6/route.c
> > @@ -2976,6 +2976,8 @@ static struct fib6_info *ip6_route_info_create(struct fib6_config *cfg,
> >       rt->fib6_metrics = ip_fib_metrics_init(net, cfg->fc_mx, cfg->fc_mx_len);
> >       if (IS_ERR(rt->fib6_metrics)) {
> >               err = PTR_ERR(rt->fib6_metrics);
> > +             /* Do not leave garbage there. */
> > +             rt->fib6_metrics = (struct dst_metrics *)&dst_default_metrics;
> >               goto out;
> >       }
> >
> >
>
> Reviewed-by: David Ahern <dsahern@gmail.com>
>
> Thanks, Eric.

^ permalink raw reply

* Re: [PATCH net] ipv6: do not leave garbage in rt->fib6_metrics
From: David Ahern @ 2018-10-05 16:29 UTC (permalink / raw)
  To: Eric Dumazet, David S . Miller; +Cc: netdev, Eric Dumazet
In-Reply-To: <20181005161750.179275-1-edumazet@google.com>

On 10/5/18 10:17 AM, Eric Dumazet wrote:
> In case ip_fib_metrics_init() returns an error, we better
> rewrite rt->fib6_metrics with &dst_default_metrics so that
> we do not crash later in ip_fib_metrics_put()
> 
> Fixes: 767a2217533f ("net: common metrics init helper for FIB entries")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: syzbot <syzkaller@googlegroups.com>

Where is syzbot sending the reports? I don't see it on netdev. I would
like to understand how it triggered the failure of a 68-byte malloc.


> ---
>  net/ipv6/route.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index 6c1d817151cae45421dc976c5ea082b4115650be..74d97addf1af20dda0c2b6a2018e88696f9f7d5a 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -2976,6 +2976,8 @@ static struct fib6_info *ip6_route_info_create(struct fib6_config *cfg,
>  	rt->fib6_metrics = ip_fib_metrics_init(net, cfg->fc_mx, cfg->fc_mx_len);
>  	if (IS_ERR(rt->fib6_metrics)) {
>  		err = PTR_ERR(rt->fib6_metrics);
> +		/* Do not leave garbage there. */
> +		rt->fib6_metrics = (struct dst_metrics *)&dst_default_metrics;
>  		goto out;
>  	}
>  
> 

Reviewed-by: David Ahern <dsahern@gmail.com>

Thanks, Eric.

^ permalink raw reply

* [PATCH net] ipv6: do not leave garbage in rt->fib6_metrics
From: Eric Dumazet @ 2018-10-05 16:17 UTC (permalink / raw)
  To: David S . Miller; +Cc: netdev, Eric Dumazet, Eric Dumazet, David Ahern

In case ip_fib_metrics_init() returns an error, we better
rewrite rt->fib6_metrics with &dst_default_metrics so that
we do not crash later in ip_fib_metrics_put()

Fixes: 767a2217533f ("net: common metrics init helper for FIB entries")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: syzbot <syzkaller@googlegroups.com>
---
 net/ipv6/route.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 6c1d817151cae45421dc976c5ea082b4115650be..74d97addf1af20dda0c2b6a2018e88696f9f7d5a 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -2976,6 +2976,8 @@ static struct fib6_info *ip6_route_info_create(struct fib6_config *cfg,
 	rt->fib6_metrics = ip_fib_metrics_init(net, cfg->fc_mx, cfg->fc_mx_len);
 	if (IS_ERR(rt->fib6_metrics)) {
 		err = PTR_ERR(rt->fib6_metrics);
+		/* Do not leave garbage there. */
+		rt->fib6_metrics = (struct dst_metrics *)&dst_default_metrics;
 		goto out;
 	}
 
-- 
2.19.0.605.g01d371f741-goog

^ permalink raw reply related

* Re: [PATCH net-next RFC 0/8] udp and configurable gro
From: Willem de Bruijn @ 2018-10-05 16:12 UTC (permalink / raw)
  To: Paolo Abeni
  Cc: Network Development, steffen.klassert, David Miller,
	Willem de Bruijn
In-Reply-To: <ac9a3fde11d8615e946cf48bdbb7b8f410baeea0.camel@redhat.com>

On Fri, Oct 5, 2018 at 12:05 PM Paolo Abeni <pabeni@redhat.com> wrote:
>
> On Fri, 2018-10-05 at 11:45 -0400, Willem de Bruijn wrote:
> > On Fri, Oct 5, 2018 at 11:30 AM Paolo Abeni <pabeni@redhat.com> wrote:
> > > Would love that. We need to care of key decr, too (and possibly don't
> > > be fooled by encap_rcv() users).
> >
> > I just sent  http://patchwork.ozlabs.org/patch/979525/
> >
> > Right now all users are those that call setup_udp_tunnel_sock
> > to register encap_rcv.
>
> plus setsockopt(UDP_ENCAP)
>
> > If accepted, I'll add a separate patch to decrement the key. That's
> > probably in udp_tunnel_sock_release, but I need to take a closer
> > look.
>
> l2tp calls setup_udp_tunnel_sock() but don't use
> udp_tunnel_sock_release(). Possibly it would be safer checking for:
>
> up->encap_type || up(sk)->gro_receive
>
> in udp_destroy_sock()

Ah indeed. And gtp might be another example.

Thanks!

^ permalink raw reply

* Re: [PATCH net-next RFC 0/8] udp and configurable gro
From: Paolo Abeni @ 2018-10-05 16:05 UTC (permalink / raw)
  To: Willem de Bruijn
  Cc: Network Development, steffen.klassert, David Miller,
	Willem de Bruijn
In-Reply-To: <CAF=yD-K3vAEepu8Ri4tYrJ7+Hw7rYWKoe7Y8QRzOovcDZDqu+A@mail.gmail.com>

On Fri, 2018-10-05 at 11:45 -0400, Willem de Bruijn wrote:
> On Fri, Oct 5, 2018 at 11:30 AM Paolo Abeni <pabeni@redhat.com> wrote:
> > Would love that. We need to care of key decr, too (and possibly don't
> > be fooled by encap_rcv() users).
> 
> I just sent  http://patchwork.ozlabs.org/patch/979525/
> 
> Right now all users are those that call setup_udp_tunnel_sock
> to register encap_rcv.

plus setsockopt(UDP_ENCAP)

> If accepted, I'll add a separate patch to decrement the key. That's
> probably in udp_tunnel_sock_release, but I need to take a closer
> look.

l2tp calls setup_udp_tunnel_sock() but don't use
udp_tunnel_sock_release(). Possibly it would be safer checking for:

up->encap_type || up(sk)->gro_receive

in udp_destroy_sock()

Cheers,

Paolo

^ permalink raw reply

* [PATCH] yam: fix a missing-check bug
From: Wenwen Wang @ 2018-10-05 15:59 UTC (permalink / raw)
  To: Wenwen Wang
  Cc: Kangjie Lu, Jean-Paul Roubelat, David S. Miller,
	open list:YAM DRIVER FOR AX.25, open list:NETWORKING DRIVERS,
	open list

In yam_ioctl(), the concrete ioctl command is firstly copied from the
user-space buffer 'ifr->ifr_data' to 'ioctl_cmd' and checked through the
following switch statement. If the command is not as expected, an error
code EINVAL is returned. In the following execution the buffer
'ifr->ifr_data' is copied again in the cases of the switch statement to
specific data structures according to what kind of ioctl command is
requested. However, after the second copy, no re-check is enforced on the
newly-copied command. Given that the buffer 'ifr->ifr_data' is in the user
space, a malicious user can race to change the command between the two
copies. This way, the attacker can inject inconsistent data and cause
undefined behavior.

This patch adds a re-check in each case of the switch statement if there is
a second copy in that case, to re-check whether the command obtained in the
second copy is the same as the one in the first copy. If not, an error code
EINVAL will be returned.

Signed-off-by: Wenwen Wang <wang6495@umn.edu>
---
 drivers/net/hamradio/yam.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/hamradio/yam.c b/drivers/net/hamradio/yam.c
index 16ec7af..ba9df43 100644
--- a/drivers/net/hamradio/yam.c
+++ b/drivers/net/hamradio/yam.c
@@ -966,6 +966,8 @@ static int yam_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
 				 sizeof(struct yamdrv_ioctl_mcs));
 		if (IS_ERR(ym))
 			return PTR_ERR(ym);
+		if (ym->cmd != SIOCYAMSMCS)
+			return -EINVAL;
 		if (ym->bitrate > YAM_MAXBITRATE) {
 			kfree(ym);
 			return -EINVAL;
@@ -981,6 +983,8 @@ static int yam_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
 		if (copy_from_user(&yi, ifr->ifr_data, sizeof(struct yamdrv_ioctl_cfg)))
 			 return -EFAULT;
 
+		if (yi.cmd != SIOCYAMSCFG)
+			return -EINVAL;
 		if ((yi.cfg.mask & YAM_IOBASE) && netif_running(dev))
 			return -EINVAL;		/* Cannot change this parameter when up */
 		if ((yi.cfg.mask & YAM_IRQ) && netif_running(dev))
-- 
2.7.4

^ permalink raw reply related

* Re: [PATCH net-next] udp: gro behind static key
From: Paolo Abeni @ 2018-10-05 15:55 UTC (permalink / raw)
  To: Willem de Bruijn, netdev; +Cc: tom, davem, steffen.klassert, Willem de Bruijn
In-Reply-To: <20181005153140.205743-1-willemdebruijn.kernel@gmail.com>

On Fri, 2018-10-05 at 11:31 -0400, Willem de Bruijn wrote:
> From: Willem de Bruijn <willemb@google.com>
> 
> Avoid the socket lookup cost in udp_gro_receive if no socket has a
> udp tunnel callback configured.
> 
> udp_sk(sk)->gro_receive requires a registration with
> setup_udp_tunnel_sock, which enables the static key.
> 
> Signed-off-by: Willem de Bruijn <willemb@google.com>
> ---
>  include/net/udp.h      | 2 ++
>  net/ipv4/udp.c         | 2 +-
>  net/ipv4/udp_offload.c | 2 +-
>  net/ipv6/udp.c         | 2 +-
>  net/ipv6/udp_offload.c | 2 +-
>  5 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/include/net/udp.h b/include/net/udp.h
> index 8482a990b0bb..9e82cb391dea 100644
> --- a/include/net/udp.h
> +++ b/include/net/udp.h
> @@ -443,8 +443,10 @@ int udpv4_offload_init(void);
>  
>  void udp_init(void);
>  
> +DECLARE_STATIC_KEY_FALSE(udp_encap_needed_key);
>  void udp_encap_enable(void);
>  #if IS_ENABLED(CONFIG_IPV6)
> +DECLARE_STATIC_KEY_FALSE(udpv6_encap_needed_key);
>  void udpv6_encap_enable(void);
>  #endif
>  
> diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
> index 5fc4beb1c336..1bec2203d558 100644
> --- a/net/ipv4/udp.c
> +++ b/net/ipv4/udp.c
> @@ -1889,7 +1889,7 @@ static int __udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
>  	return 0;
>  }
>  
> -static DEFINE_STATIC_KEY_FALSE(udp_encap_needed_key);
> +DEFINE_STATIC_KEY_FALSE(udp_encap_needed_key);
>  void udp_encap_enable(void)
>  {
>  	static_branch_enable(&udp_encap_needed_key);
> diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
> index 0c0522b79b43..802f2bc00d69 100644
> --- a/net/ipv4/udp_offload.c
> +++ b/net/ipv4/udp_offload.c
> @@ -405,7 +405,7 @@ static struct sk_buff *udp4_gro_receive(struct list_head *head,
>  {
>  	struct udphdr *uh = udp_gro_udphdr(skb);
>  
> -	if (unlikely(!uh))
> +	if (unlikely(!uh) || !static_branch_unlikely(&udp_encap_needed_key))
>  		goto flush;
>  
>  	/* Don't bother verifying checksum if we're going to flush anyway. */
> diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
> index 28c4aa5078fc..374e7d302f26 100644
> --- a/net/ipv6/udp.c
> +++ b/net/ipv6/udp.c
> @@ -548,7 +548,7 @@ static __inline__ void udpv6_err(struct sk_buff *skb,
>  	__udp6_lib_err(skb, opt, type, code, offset, info, &udp_table);
>  }
>  
> -static DEFINE_STATIC_KEY_FALSE(udpv6_encap_needed_key);
> +DEFINE_STATIC_KEY_FALSE(udpv6_encap_needed_key);
>  void udpv6_encap_enable(void)
>  {
>  	static_branch_enable(&udpv6_encap_needed_key);
> diff --git a/net/ipv6/udp_offload.c b/net/ipv6/udp_offload.c
> index 95dee9ca8d22..1b8e161ac527 100644
> --- a/net/ipv6/udp_offload.c
> +++ b/net/ipv6/udp_offload.c
> @@ -119,7 +119,7 @@ static struct sk_buff *udp6_gro_receive(struct list_head *head,
>  {
>  	struct udphdr *uh = udp_gro_udphdr(skb);
>  
> -	if (unlikely(!uh))
> +	if (unlikely(!uh) || !static_branch_unlikely(&udpv6_encap_needed_key))
>  		goto flush;
>  
>  	/* Don't bother verifying checksum if we're going to flush anyway. */

Acked-by: Paolo Abeni <pabeni@redhat.com>

^ permalink raw reply

* Re: [PATCH net-next 20/20] net/bridge: Update br_mdb_dump for strict data checking
From: David Ahern @ 2018-10-05 15:49 UTC (permalink / raw)
  To: David Miller, dsahern; +Cc: netdev, christian, jbenc, stephen
In-Reply-To: <20181005.003445.1297966279801053451.davem@davemloft.net>

On 10/5/18 1:34 AM, David Miller wrote:
> From: David Ahern <dsahern@kernel.org>
> Date: Thu,  4 Oct 2018 14:33:55 -0700
> 
>> @@ -162,6 +162,28 @@ static int br_mdb_fill_info(struct sk_buff *skb, struct netlink_callback *cb,
>>  	return err;
>>  }
>>  
>> +static int br_mdb_valid_dump_req(const struct nlmsghdr *nlh,
>> +				 struct netlink_ext_ack *extack)
>> +{
>> +	struct br_port_msg *bpm;
>> +
>> +	if (nlh->nlmsg_len < nlmsg_msg_size(sizeof(*bpm))) {
>> +		NL_SET_ERR_MSG(extack, "Invalid header");
>> +		return -EINVAL;
>> +	}
>> +	if (bpm->ifindex) {
> 
> 'bpm' is never initialized.
> 

Thanks.

I had not updated the bridge command for strict checking. Doing so and
bridge mdb show generates a trace. Will fix.

^ permalink raw reply

* Re: [PATCH net-next RFC 0/8] udp and configurable gro
From: Willem de Bruijn @ 2018-10-05 15:45 UTC (permalink / raw)
  To: Paolo Abeni
  Cc: Network Development, steffen.klassert, David Miller,
	Willem de Bruijn
In-Reply-To: <17ebaef357cda6696808259c1e42843a37c15f37.camel@redhat.com>

On Fri, Oct 5, 2018 at 11:30 AM Paolo Abeni <pabeni@redhat.com> wrote:
>
> Hi,
>
> Thank you for the prompt reply!
>

Not at all. Thanks for moving this forward :)

> On Fri, 2018-10-05 at 10:41 -0400, Willem de Bruijn wrote:
> > On Fri, Oct 5, 2018 at 9:53 AM Paolo Abeni <pabeni@redhat.com> wrote:
> > >
> > > Hi all,
> > >
> > > On Fri, 2018-09-14 at 13:59 -0400, Willem de Bruijn wrote:
> > > > This is a *very rough* draft. Mainly for discussion while we also
> > > > look at another partially overlapping approach [1].
> > >
> > > I'm wondering how we go on from this ? I'm fine with either approaches.
> >
> > Let me send the udp gro static_key patch.
>
> Would love that. We need to care of key decr, too (and possibly don't
> be fooled by encap_rcv() users).

I just sent  http://patchwork.ozlabs.org/patch/979525/

Right now all users are those that call setup_udp_tunnel_sock
to register encap_rcv.

If accepted, I'll add a separate patch to decrement the key. That's
probably in udp_tunnel_sock_release, but I need to take a closer
look.

> > Then we don't need the enable udp on demand logic (patch 2/4).
>
> ok.
>
> > Your implementation of GRO is more fleshed out (patch 3/4) than
> > my quick hack. My only request would be to use a separate
> > UDP_GRO socket option instead of adding this to the existing
> > UDP_SEGMENT.
> >
> > Sounds good?
>
> Indeed!
> I need also to add a cmsg to expose to the user the skb gro_size, and
> some test cases. Locally I'm [ab-]using the GRO functionality
> introduced recently on veth to test the code in a namespace pair
> (attaching a dummy XDP program to the RX-side veth). I'm not sure if
> that could fit a selftest.

Very nice. Yes, veth only implements napi in xdp mode.

>
> > > Also, I'm interested in [try to] enable GRO/GSO batching in the
> > > forwarding path, as you outlined initially in the GSO series
> > > submission. That should cover Steffen use-case, too, right?
> >
> > Great. Indeed. Though there is some unresolved discussion on
> > one large gso skb vs frag list. There has been various concerns
> > around the use of frag lists for GSO in the past, and it does not
> > match h/w offload. So I think the answer would be the first unless
> > the second proves considerably faster (in which case it could also
> > be added later as optimization).
>
> Agreed.
>
> Let's try the first step first ;)
>
> Final but relevant note: I'll try my best to avoid delaying this, but
> lately I tend to be pre-empted by other tasks, it's difficult for me to
> assure a deadline here.
>
> Cheers,
>
> Paolo
>

^ permalink raw reply

* Re: [PATCH net-next 0/5] net: Consolidate metrics handling for ipv4 and ipv6
From: David Ahern @ 2018-10-05 15:37 UTC (permalink / raw)
  To: Eric Dumazet, David Miller, dsahern; +Cc: netdev, weiwan, sd, xiyou.wangcong
In-Reply-To: <d6cd6b12-9e7c-b84b-7dfb-20e47b77b8f6@gmail.com>

On 10/5/18 7:08 AM, Eric Dumazet wrote:
> Commit 767a2217533fed6 ("net: common metrics init helper for FIB entries")
> is not correct because we need to better deal with error paths.
> 
> I will submit this more formally when I can reach my workstation in a few minutes :
> 
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index 6c1d817151cae45421dc976c5ea082b4115650be..74d97addf1af20dda0c2b6a2018e88696f9f7d5a 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -2976,6 +2976,8 @@ static struct fib6_info *ip6_route_info_create(struct fib6_config *cfg,
>         rt->fib6_metrics = ip_fib_metrics_init(net, cfg->fc_mx, cfg->fc_mx_len);
>         if (IS_ERR(rt->fib6_metrics)) {
>                 err = PTR_ERR(rt->fib6_metrics);
> +               /* Do not leave garbage there. */
> +               rt->fib6_metrics = (struct dst_metrics *)&dst_default_metrics;
>                 goto out;
>         }
>  
> 

Yes, I was focused on the memory leaks and cleanup and forgot to
purposely fail the alloc for the metrics. The above is one way to fix
it. Thanks for spotting it.

^ permalink raw reply

* [PATCH net-next] udp: gro behind static key
From: Willem de Bruijn @ 2018-10-05 15:31 UTC (permalink / raw)
  To: netdev; +Cc: pabeni, tom, davem, steffen.klassert, Willem de Bruijn

From: Willem de Bruijn <willemb@google.com>

Avoid the socket lookup cost in udp_gro_receive if no socket has a
udp tunnel callback configured.

udp_sk(sk)->gro_receive requires a registration with
setup_udp_tunnel_sock, which enables the static key.

Signed-off-by: Willem de Bruijn <willemb@google.com>
---
 include/net/udp.h      | 2 ++
 net/ipv4/udp.c         | 2 +-
 net/ipv4/udp_offload.c | 2 +-
 net/ipv6/udp.c         | 2 +-
 net/ipv6/udp_offload.c | 2 +-
 5 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/include/net/udp.h b/include/net/udp.h
index 8482a990b0bb..9e82cb391dea 100644
--- a/include/net/udp.h
+++ b/include/net/udp.h
@@ -443,8 +443,10 @@ int udpv4_offload_init(void);
 
 void udp_init(void);
 
+DECLARE_STATIC_KEY_FALSE(udp_encap_needed_key);
 void udp_encap_enable(void);
 #if IS_ENABLED(CONFIG_IPV6)
+DECLARE_STATIC_KEY_FALSE(udpv6_encap_needed_key);
 void udpv6_encap_enable(void);
 #endif
 
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 5fc4beb1c336..1bec2203d558 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1889,7 +1889,7 @@ static int __udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
 	return 0;
 }
 
-static DEFINE_STATIC_KEY_FALSE(udp_encap_needed_key);
+DEFINE_STATIC_KEY_FALSE(udp_encap_needed_key);
 void udp_encap_enable(void)
 {
 	static_branch_enable(&udp_encap_needed_key);
diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
index 0c0522b79b43..802f2bc00d69 100644
--- a/net/ipv4/udp_offload.c
+++ b/net/ipv4/udp_offload.c
@@ -405,7 +405,7 @@ static struct sk_buff *udp4_gro_receive(struct list_head *head,
 {
 	struct udphdr *uh = udp_gro_udphdr(skb);
 
-	if (unlikely(!uh))
+	if (unlikely(!uh) || !static_branch_unlikely(&udp_encap_needed_key))
 		goto flush;
 
 	/* Don't bother verifying checksum if we're going to flush anyway. */
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 28c4aa5078fc..374e7d302f26 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -548,7 +548,7 @@ static __inline__ void udpv6_err(struct sk_buff *skb,
 	__udp6_lib_err(skb, opt, type, code, offset, info, &udp_table);
 }
 
-static DEFINE_STATIC_KEY_FALSE(udpv6_encap_needed_key);
+DEFINE_STATIC_KEY_FALSE(udpv6_encap_needed_key);
 void udpv6_encap_enable(void)
 {
 	static_branch_enable(&udpv6_encap_needed_key);
diff --git a/net/ipv6/udp_offload.c b/net/ipv6/udp_offload.c
index 95dee9ca8d22..1b8e161ac527 100644
--- a/net/ipv6/udp_offload.c
+++ b/net/ipv6/udp_offload.c
@@ -119,7 +119,7 @@ static struct sk_buff *udp6_gro_receive(struct list_head *head,
 {
 	struct udphdr *uh = udp_gro_udphdr(skb);
 
-	if (unlikely(!uh))
+	if (unlikely(!uh) || !static_branch_unlikely(&udpv6_encap_needed_key))
 		goto flush;
 
 	/* Don't bother verifying checksum if we're going to flush anyway. */
-- 
2.19.0.605.g01d371f741-goog

^ permalink raw reply related

* Re: [PATCH net-next RFC 0/8] udp and configurable gro
From: Paolo Abeni @ 2018-10-05 15:30 UTC (permalink / raw)
  To: Willem de Bruijn
  Cc: Network Development, steffen.klassert, David Miller,
	Willem de Bruijn
In-Reply-To: <CAF=yD-LHKr4bWd7uHLxRiYb=rjsJbptQAae_RDxtpRT+Z2-=CQ@mail.gmail.com>

Hi,

Thank you for the prompt reply!

On Fri, 2018-10-05 at 10:41 -0400, Willem de Bruijn wrote:
> On Fri, Oct 5, 2018 at 9:53 AM Paolo Abeni <pabeni@redhat.com> wrote:
> > 
> > Hi all,
> > 
> > On Fri, 2018-09-14 at 13:59 -0400, Willem de Bruijn wrote:
> > > This is a *very rough* draft. Mainly for discussion while we also
> > > look at another partially overlapping approach [1].
> > 
> > I'm wondering how we go on from this ? I'm fine with either approaches.
> 
> Let me send the udp gro static_key patch.

Would love that. We need to care of key decr, too (and possibly don't
be fooled by encap_rcv() users).

> Then we don't need the enable udp on demand logic (patch 2/4).

ok.

> Your implementation of GRO is more fleshed out (patch 3/4) than
> my quick hack. My only request would be to use a separate
> UDP_GRO socket option instead of adding this to the existing
> UDP_SEGMENT.
> 
> Sounds good?

Indeed!
I need also to add a cmsg to expose to the user the skb gro_size, and
some test cases. Locally I'm [ab-]using the GRO functionality
introduced recently on veth to test the code in a namespace pair
(attaching a dummy XDP program to the RX-side veth). I'm not sure if
that could fit a selftest.

> > Also, I'm interested in [try to] enable GRO/GSO batching in the
> > forwarding path, as you outlined initially in the GSO series
> > submission. That should cover Steffen use-case, too, right?
> 
> Great. Indeed. Though there is some unresolved discussion on
> one large gso skb vs frag list. There has been various concerns
> around the use of frag lists for GSO in the past, and it does not
> match h/w offload. So I think the answer would be the first unless
> the second proves considerably faster (in which case it could also
> be added later as optimization).

Agreed.

Let's try the first step first ;)

Final but relevant note: I'll try my best to avoid delaying this, but
lately I tend to be pre-empted by other tasks, it's difficult for me to
assure a deadline here.

Cheers,

Paolo

^ permalink raw reply

* Re: [PATCH bpf-next 1/6] bpf: introduce BPF_PROG_TYPE_FILE_FILTER
From: Alexei Starovoitov @ 2018-10-05 22:27 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Al Viro, Alexei Starovoitov, David S. Miller, Daniel Borkmann,
	Network Development, LKML, kernel-team
In-Reply-To: <CALCETrVURVaqJChL=RwZPe74+JfX-6Zzp9kLR9-mLiownSOexQ@mail.gmail.com>

On Fri, Oct 05, 2018 at 03:09:20PM -0700, Andy Lutomirski wrote:
> On Fri, Oct 5, 2018 at 3:05 PM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> >
> > On Fri, Oct 05, 2018 at 05:46:59AM +0100, Al Viro wrote:
> >
> > > Another problem is your direct poking in ->i_ino.  It's not
> > > something directly exposed to userland at the moment and it should
> > > not become such.
> >
> > The patch is not making i_ino directly exposed.
> > Only 'struct bpf_file_info' is exposed to user space / bpf programs.
> 
> I think Al is saying that the valie of i_ino is not something that
> user code is permitted to know regardless of how you format it because
> it may or may not actually match the value returned by stat().
> Another way of saying that is that your patch is digging into an
> internal data structure and is doing it wrong.

several fs implementation I've looked at just do generic_fillattr()
for these fields. Are you saying some FS don't use inode->i_ino at all?
And it's bogus, hence shouldn't be read?

^ permalink raw reply

* general protection fault in __handle_mm_fault
From: syzbot @ 2018-10-05 22:24 UTC (permalink / raw)
  To: davem, edumazet, kuznet, linux-kernel, netdev, syzkaller-bugs,
	yoshfuji

Hello,

syzbot found the following crash on:

HEAD commit:    25bcda3e8b9f Add linux-next specific files for 20181004
git tree:       linux-next
console output: https://syzkaller.appspot.com/x/log.txt?x=130e3bf1400000
kernel config:  https://syzkaller.appspot.com/x/.config?x=603d7f9140c3368a
dashboard link: https://syzkaller.appspot.com/bug?extid=1577fbe983d20fe2e88f
compiler:       gcc (GCC) 8.0.1 20180413 (experimental)
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=127e88d6400000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=13cdb67e400000

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+1577fbe983d20fe2e88f@syzkaller.appspotmail.com

8021q: adding VLAN 0 to HW filter on device team0
IPv6: ADDRCONF(NETDEV_CHANGE): team0: link becomes ready
nf_conntrack: default automatic helper assignment has been turned off for  
security reasons and CT-based  firewall rule not found. Use the iptables CT  
target to attach helpers instead.
kasan: CONFIG_KASAN_INLINE enabled
kasan: GPF could be caused by NULL-ptr deref or user memory access
general protection fault: 0000 [#1] PREEMPT SMP KASAN
CPU: 0 PID: 6015 Comm: syz-executor196 Not tainted  
4.19.0-rc6-next-20181004+ #87
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
Google 01/01/2011
RIP: 0010:do_fault mm/memory.c:3524 [inline]
RIP: 0010:handle_pte_fault mm/memory.c:3762 [inline]
RIP: 0010:__handle_mm_fault+0x2d72/0x55c0 mm/memory.c:3886
Code: 48 89 fa 48 c1 ea 03 80 3c 02 00 0f 85 87 20 00 00 49 8b 9c 24 b0 fe  
ff ff 48 b8 00 00 00 00 00 fc ff df 48 89 da 48 c1 ea 03 <80> 3c 02 00 0f  
85 57 20 00 00 48 8b 1b 31 ff 48 83 e3 9f 48 89 de
RSP: 0018:ffff8801bbe66fc0 EFLAGS: 00010246
RAX: dffffc0000000000 RBX: 0000000000000000 RCX: ffffffff8160f791
RDX: 0000000000000000 RSI: 0000000000000004 RDI: ffff8801bbe670f8
RBP: ffff8801bbe67270 R08: ffffed0039d485ae R09: ffffed0039d485ad
R10: ffffed0039d485ad R11: ffff8801cea42d6b R12: ffff8801bbe67248
R13: ffff8801bbc7e800 R14: ffff8801d1759f40 R15: 0000000000000067
FS:  0000000001c6a880(0000) GS:ffff8801dae00000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000020077000 CR3: 00000001d79be000 CR4: 00000000001406f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
  handle_mm_fault+0x54f/0xc70 mm/memory.c:3923
  __do_page_fault+0x567/0xd10 arch/x86/mm/fault.c:1355
  do_page_fault+0xed/0x7d1 arch/x86/mm/fault.c:1430
  page_fault+0x1e/0x30 arch/x86/entry/entry_64.S:1139
RIP: 0010:copy_user_enhanced_fast_string+0xe/0x20  
arch/x86/lib/copy_user_64.S:180
Code: 89 d1 c1 e9 03 83 e2 07 f3 48 a5 89 d1 f3 a4 31 c0 0f 1f 00 c3 0f 1f  
80 00 00 00 00 0f 1f 00 83 fa 40 0f 82 70 ff ff ff 89 d1 <f3> a4 31 c0 0f  
1f 00 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 00 83
RSP: 0018:ffff8801bbe675b8 EFLAGS: 00010202
RAX: 0000000000000000 RBX: 0000000000007a50 RCX: 0000000000001b40
RDX: 0000000000007a50 RSI: 0000000020077000 RDI: ffff8801ce615f10
RBP: ffff8801bbe675f0 R08: ffffed0039cc2f4a R09: ffffed0039cc2f4a
R10: ffffed0039cc2f49 R11: ffff8801ce617a4f R12: 0000000020078b40
R13: 00000000200710f0 R14: ffff8801ce610000 R15: 00007ffffffff000
  _copy_from_iter_full+0x263/0xc20 lib/iov_iter.c:724
  copy_from_iter_full include/linux/uio.h:124 [inline]
  skb_do_copy_data_nocache include/net/sock.h:1951 [inline]
  skb_copy_to_page_nocache include/net/sock.h:1977 [inline]
  tcp_sendmsg_locked+0x159e/0x3f90 net/ipv4/tcp.c:1338
  tcp_sendmsg+0x2f/0x50 net/ipv4/tcp.c:1443
  inet_sendmsg+0x19c/0x690 net/ipv4/af_inet.c:798
  sock_sendmsg_nosec net/socket.c:622 [inline]
  sock_sendmsg+0xd5/0x120 net/socket.c:632
  __sys_sendto+0x3d7/0x670 net/socket.c:1789
  __do_sys_sendto net/socket.c:1801 [inline]
  __se_sys_sendto net/socket.c:1797 [inline]
  __x64_sys_sendto+0xe1/0x1a0 net/socket.c:1797
  do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
  entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x441159
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 db 0a fc ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007ffc1def2948 EFLAGS: 00000217 ORIG_RAX: 000000000000002c
RAX: ffffffffffffffda RBX: 00000000004002c8 RCX: 0000000000441159
RDX: fffffffffffffee3 RSI: 0000000020000b40 RDI: 0000000000000003
RBP: 00000000006cc018 R08: 0000000000000000 R09: ffffffffffffff61
R10: 00000000040000cb R11: 0000000000000217 R12: 00000000004020c0
R13: 0000000000402150 R14: 0000000000000000 R15: 0000000000000000
Modules linked in:
---[ end trace d2b868b55677c602 ]---
RIP: 0010:do_fault mm/memory.c:3524 [inline]
RIP: 0010:handle_pte_fault mm/memory.c:3762 [inline]
RIP: 0010:__handle_mm_fault+0x2d72/0x55c0 mm/memory.c:3886
Code: 48 89 fa 48 c1 ea 03 80 3c 02 00 0f 85 87 20 00 00 49 8b 9c 24 b0 fe  
ff ff 48 b8 00 00 00 00 00 fc ff df 48 89 da 48 c1 ea 03 <80> 3c 02 00 0f  
85 57 20 00 00 48 8b 1b 31 ff 48 83 e3 9f 48 89 de
RSP: 0018:ffff8801bbe66fc0 EFLAGS: 00010246
RAX: dffffc0000000000 RBX: 0000000000000000 RCX: ffffffff8160f791
RDX: 0000000000000000 RSI: 0000000000000004 RDI: ffff8801bbe670f8
RBP: ffff8801bbe67270 R08: ffffed0039d485ae R09: ffffed0039d485ad
R10: ffffed0039d485ad R11: ffff8801cea42d6b R12: ffff8801bbe67248
R13: ffff8801bbc7e800 R14: ffff8801d1759f40 R15: 0000000000000067
FS:  0000000001c6a880(0000) GS:ffff8801dae00000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000020077000 CR3: 00000001d79be000 CR4: 00000000001406f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400


---
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 net-next v6 19/23] zinc: Curve25519 ARM implementation
From: Ard Biesheuvel @ 2018-10-05 15:16 UTC (permalink / raw)
  To: Jason A. Donenfeld, Ard Biesheuvel, Linux Kernel Mailing List,
	<netdev@vger.kernel.org>,
	open list:HARDWARE RANDOM NUMBER GENERATOR CORE, David S. Miller,
	Greg Kroah-Hartman, Samuel Neves, Andy Lutomirski,
	Jean-Philippe Aumasson, Russell King, linux-arm-kernel, peter
In-Reply-To: <20181005150538.17006.qmail@cr.yp.to>

On 5 October 2018 at 17:05, D. J. Bernstein <djb@cr.yp.to> wrote:
> For the in-order ARM Cortex-A8 (the target for this code), adjacent
> multiply-add instructions forward summands quickly. A simple in-order
> dot-product computation has no latency problems, while interleaving
> computations, as suggested in this thread, creates problems. Also, on
> this microarchitecture, occasional ARM instructions run in parallel with
> NEON, so trying to manually eliminate ARM instructions through global
> pointer tracking wouldn't gain speed; it would simply create unnecessary
> code-maintenance problems.
>
> See https://cr.yp.to/papers.html#neoncrypto for analysis of the
> performance of---and remaining bottlenecks in---this code. Further
> speedups should be possible on this microarchitecture, but, for anyone
> interested in this, I recommend focusing on building a cycle-accurate
> simulator (e.g., fixing inaccuracies in the Sobole simulator) first.
>
> Of course, there are other ARM microarchitectures, and there are many
> cases where different microarchitectures prefer different optimizations.
> The kernel already has boot-time benchmarks for different optimizations
> for raid6, and should do the same for crypto code, so that implementors
> can focus on each microarchitecture separately rather than living in the
> barbaric world of having to choose which CPUs to favor.
>

Thanks Dan for the insight.

We have already established in a separate discussion that Cortex-A7,
which is main optimization target for future development, does not
have the microarchitectural peculiarity that you are referring to that
ARM instructions are essentially free when interleaved with NEON code.

But I take your point re benchmarking (as I already indicated in my
reply to Jason): if we optimize towards speed, we should ideally reuse
the existing benchmarking infrastructure we have to select the fastest
implementation at runtime. For instance, it turns out that scalar
ChaCha20 is almost as fast as NEON (or even faster?) on A7, and using
NEON in the kernel has some issues of its own.

^ permalink raw reply

* Re: [PATCH net-next v6 19/23] zinc: Curve25519 ARM implementation
From: D. J. Bernstein @ 2018-10-05 15:05 UTC (permalink / raw)
  To: Jason A. Donenfeld, Ard Biesheuvel, LKML, Netdev,
	Linux Crypto Mailing List, David Miller, Greg Kroah-Hartman,
	Samuel Neves, Andrew Lutomirski, Jean-Philippe Aumasson,
	Russell King - ARM Linux, linux-arm-kernel, Peter Schwabe
In-Reply-To: <CAHmME9rp0Fi5ObK5oi8FHj1_nK5hP4T2Bq7_dAmzq4OQ0mp0uw@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 1286 bytes --]

For the in-order ARM Cortex-A8 (the target for this code), adjacent
multiply-add instructions forward summands quickly. A simple in-order
dot-product computation has no latency problems, while interleaving
computations, as suggested in this thread, creates problems. Also, on
this microarchitecture, occasional ARM instructions run in parallel with
NEON, so trying to manually eliminate ARM instructions through global
pointer tracking wouldn't gain speed; it would simply create unnecessary
code-maintenance problems.

See https://cr.yp.to/papers.html#neoncrypto for analysis of the
performance of---and remaining bottlenecks in---this code. Further
speedups should be possible on this microarchitecture, but, for anyone
interested in this, I recommend focusing on building a cycle-accurate
simulator (e.g., fixing inaccuracies in the Sobole simulator) first.

Of course, there are other ARM microarchitectures, and there are many
cases where different microarchitectures prefer different optimizations.
The kernel already has boot-time benchmarks for different optimizations
for raid6, and should do the same for crypto code, so that implementors
can focus on each microarchitecture separately rather than living in the
barbaric world of having to choose which CPUs to favor.

---Dan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox