* Re: [PATCH net-next] openvswitch: add ct_clear action
From: Joe Stringer @ 2017-10-10 17:24 UTC (permalink / raw)
To: Eric Garver, Joe Stringer, Pravin Shelar,
Linux Kernel Network Developers, ovs dev
In-Reply-To: <20171010150949.GG26353@dev-rhel7>
On 10 October 2017 at 08:09, Eric Garver <e@erig.me> wrote:
> On Tue, Oct 10, 2017 at 05:33:48AM -0700, Joe Stringer wrote:
>> On 9 October 2017 at 21:41, Pravin Shelar <pshelar-LZ6Gd1LRuIk@public.gmane.org> wrote:
>> > On Fri, Oct 6, 2017 at 9:44 AM, Eric Garver <e@erig.me> wrote:
>> >> This adds a ct_clear action for clearing conntrack state. ct_clear is
>> >> currently implemented in OVS userspace, but is not backed by an action
>> >> in the kernel datapath. This is useful for flows that may modify a
>> >> packet tuple after a ct lookup has already occurred.
>> >>
>> >> Signed-off-by: Eric Garver <e@erig.me>
>> > Patch mostly looks good. I have following comments.
>> >
>> >> ---
>> >> include/uapi/linux/openvswitch.h | 2 ++
>> >> net/openvswitch/actions.c | 5 +++++
>> >> net/openvswitch/conntrack.c | 12 ++++++++++++
>> >> net/openvswitch/conntrack.h | 7 +++++++
>> >> net/openvswitch/flow_netlink.c | 5 +++++
>> >> 5 files changed, 31 insertions(+)
>> >>
>> >> diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h
>> >> index 156ee4cab82e..1b6e510e2cc6 100644
>> >> --- a/include/uapi/linux/openvswitch.h
>> >> +++ b/include/uapi/linux/openvswitch.h
>> >> @@ -806,6 +806,7 @@ struct ovs_action_push_eth {
>> >> * packet.
>> >> * @OVS_ACTION_ATTR_POP_ETH: Pop the outermost Ethernet header off the
>> >> * packet.
>> >> + * @OVS_ACTION_ATTR_CT_CLEAR: Clear conntrack state from the packet.
>> >> *
>> >> * Only a single header can be set with a single %OVS_ACTION_ATTR_SET. Not all
>> >> * fields within a header are modifiable, e.g. the IPv4 protocol and fragment
>> >> @@ -835,6 +836,7 @@ enum ovs_action_attr {
>> >> OVS_ACTION_ATTR_TRUNC, /* u32 struct ovs_action_trunc. */
>> >> OVS_ACTION_ATTR_PUSH_ETH, /* struct ovs_action_push_eth. */
>> >> OVS_ACTION_ATTR_POP_ETH, /* No argument. */
>> >> + OVS_ACTION_ATTR_CT_CLEAR, /* No argument. */
>> >>
>> >> __OVS_ACTION_ATTR_MAX, /* Nothing past this will be accepted
>> >> * from userspace. */
>> >> diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
>> >> index a54a556fcdb5..db9c7f2e662b 100644
>> >> --- a/net/openvswitch/actions.c
>> >> +++ b/net/openvswitch/actions.c
>> >> @@ -1203,6 +1203,10 @@ static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,
>> >> return err == -EINPROGRESS ? 0 : err;
>> >> break;
>> >>
>> >> + case OVS_ACTION_ATTR_CT_CLEAR:
>> >> + err = ovs_ct_clear(skb, key);
>> >> + break;
>> >> +
>> >> case OVS_ACTION_ATTR_PUSH_ETH:
>> >> err = push_eth(skb, key, nla_data(a));
>> >> break;
>> >> @@ -1210,6 +1214,7 @@ static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,
>> >> case OVS_ACTION_ATTR_POP_ETH:
>> >> err = pop_eth(skb, key);
>> >> break;
>> >> +
>> >> }
>> > Unrelated change.
>> >
>> >>
>> >> if (unlikely(err)) {
>> >> diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
>> >> index d558e882ca0c..f9b73c726ad7 100644
>> >> --- a/net/openvswitch/conntrack.c
>> >> +++ b/net/openvswitch/conntrack.c
>> >> @@ -1129,6 +1129,18 @@ int ovs_ct_execute(struct net *net, struct sk_buff *skb,
>> >> return err;
>> >> }
>> >>
>> >> +int ovs_ct_clear(struct sk_buff *skb, struct sw_flow_key *key)
>> >> +{
>> >> + if (skb_nfct(skb)) {
>> >> + nf_conntrack_put(skb_nfct(skb));
>> >> + nf_ct_set(skb, NULL, 0);
>> > Can the new conntract state be appropriate? may be IP_CT_UNTRACKED?
>> >
>> >> + }
>> >> +
>> >> + ovs_ct_fill_key(skb, key);
>> >> +
>> > I do not see need to refill the key if there is no skb-nf-ct.
>>
>> Really this is trying to just zero the CT key fields, but reuses
>> existing functions, right? This means that subsequent upcalls, for
>
> Right.
>
>> instance, won't have the outdated view of the CT state from the
>> previous lookup (that was prior to the ct_clear). I'd expect these key
>> fields to be cleared.
>
> I assumed Pravin was saying that we don't need to clear them if there is
> no conntrack state. They should already be zero.
The conntrack calls aren't going to clear it, so I don't see what else
would clear it?
If you execute ct(),ct_clear(), then the first ct will set the
values.. what will zero them?
^ permalink raw reply
* Re: [patch net-next 0/4] net: sched: get rid of cls_flower->egress_dev
From: Or Gerlitz @ 2017-10-10 17:25 UTC (permalink / raw)
To: Jiri Pirko
Cc: Linux Netdev List, David Miller, Jamal Hadi Salim, Cong Wang,
Saeed Mahameed, Matan Barak, Leon Romanovsky, mlxsw
In-Reply-To: <20171010073016.3682-1-jiri@resnulli.us>
On Tue, Oct 10, 2017 at 10:30 AM, Jiri Pirko <jiri@resnulli.us> wrote:
>
>
> drivers/net/ethernet/mellanox/mlx5/core/en.h | 3 +
> drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 4 +-
> drivers/net/ethernet/mellanox/mlx5/core/en_rep.c | 31 ++--
Jiri,
FWIW, as I reported to you earlier, I was playing with tc encap/decap
rules on 4.14-rc+ (net) before
applying any patch of this series, and something is messy w.r.t to
decap rules. I don't see
them removed at all when user space attempts to do so. It might
(probably) mlx5 bug, which
we will have to fix for net and later rebase net-next over net. We
have short WW here so
I will not be able to do RCA this week.
^ permalink raw reply
* Re: [PATCH][net-next] ipv6: fix dereference of rt6_ex before null check error
From: Eric Dumazet @ 2017-10-10 17:26 UTC (permalink / raw)
To: Colin King
Cc: David S . Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, netdev,
kernel-janitors, linux-kernel
In-Reply-To: <20171010170116.21124-1-colin.king@canonical.com>
On Tue, 2017-10-10 at 18:01 +0100, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> Currently rt6_ex is being dereferenced before it is null checked
> hence there is a possible null dereference bug. Fix this by only
> dereferencing rt6_ex after it has been null checked.
>
> Detected by CoverityScan, CID#1457749 ("Dereference before null check")
>
> Fixes: 81eb8447daae ("ipv6: take care of rt6_stats")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
Reviewed-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply
* Re: [Patch net-next] tcp: add a tracepoint for tcp_retransmit_skb()
From: Alexei Starovoitov @ 2017-10-10 17:38 UTC (permalink / raw)
To: Cong Wang
Cc: netdev, Eric Dumazet, Yuchung Cheng, Neal Cardwell,
Martin KaFai Lau, Brendan Gregg, hannes, Song Liu
In-Reply-To: <20171010053547.21162-1-xiyou.wangcong@gmail.com>
On Mon, Oct 09, 2017 at 10:35:47PM -0700, Cong Wang wrote:
> We need a real-time notification for tcp retransmission
> for monitoring.
>
> Of course we could use ftrace to dynamically instrument this
> kernel function too, however we can't retrieve the connection
> information at the same time, for example perf-tools [1] reads
> /proc/net/tcp for socket details, which is slow when we have
> a lots of connections.
>
> Therefore, this patch adds a tracepoint for tcp_retransmit_skb()
> and exposes src/dst IP addresses and ports of the connection.
> This also makes it easier to integrate into perf.
>
> Note, I expose both IPv4 and IPv6 addresses at the same time:
> for a IPv4 socket, v4 mapped address is used as IPv6 addresses,
> for a IPv6 socket, LOOPBACK4_IPV6 is already filled by kernel.
>
> Perhaps there are other interfaces to use (for example netlink),
> but tracepoint is the quickest way I can think of.
>
> 1. https://github.com/brendangregg/perf-tools/blob/master/net/tcpretrans
>
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: Yuchung Cheng <ycheng@google.com>
> Cc: Neal Cardwell <ncardwell@google.com>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
> ---
> include/trace/events/tcp.h | 63 ++++++++++++++++++++++++++++++++++++++++++++++
> net/core/net-traces.c | 1 +
> net/ipv4/tcp_output.c | 3 +++
> 3 files changed, 67 insertions(+)
> create mode 100644 include/trace/events/tcp.h
>
> diff --git a/include/trace/events/tcp.h b/include/trace/events/tcp.h
> new file mode 100644
> index 000000000000..cb22acc8aacd
> --- /dev/null
> +++ b/include/trace/events/tcp.h
> @@ -0,0 +1,63 @@
> +#undef TRACE_SYSTEM
> +#define TRACE_SYSTEM tcp
> +
> +#if !defined(_TRACE_TCP_H) || defined(TRACE_HEADER_MULTI_READ)
> +#define _TRACE_TCP_H
> +
> +#include <linux/ipv6.h>
> +#include <linux/tcp.h>
> +#include <linux/tracepoint.h>
> +#include <net/ipv6.h>
> +
> +TRACE_EVENT(tcp_retransmit_skb,
> +
> + TP_PROTO(const struct sock *sk, struct sk_buff *skb, int segs),
> +
> + TP_ARGS(sk, skb, segs),
> +
> + TP_STRUCT__entry(
> + __field(__u16, sport)
> + __field(__u16, dport)
> + __array(__u8, saddr, 4)
> + __array(__u8, daddr, 4)
> + __array(__u8, saddr_v6, 16)
> + __array(__u8, daddr_v6, 16)
> + ),
> +
> + TP_fast_assign(
> + struct ipv6_pinfo *np = inet6_sk(sk);
> + struct inet_sock *inet = inet_sk(sk);
> + struct in6_addr *pin6;
> + __be32 *p32;
> +
> + __entry->sport = ntohs(inet->inet_sport);
> + __entry->dport = ntohs(inet->inet_dport);
> +
> + p32 = (__be32 *) __entry->saddr;
> + *p32 = inet->inet_saddr;
> +
> + p32 = (__be32 *) __entry->daddr;
> + *p32 = inet->inet_daddr;
> +
> + if (np) {
> + pin6 = (struct in6_addr *)__entry->saddr_v6;
> + *pin6 = np->saddr;
> + pin6 = (struct in6_addr *)__entry->daddr_v6;
> + *pin6 = *(np->daddr_cache);
> + } else {
> + pin6 = (struct in6_addr *)__entry->saddr_v6;
> + ipv6_addr_set_v4mapped(inet->inet_saddr, pin6);
> + pin6 = (struct in6_addr *)__entry->daddr_v6;
> + ipv6_addr_set_v4mapped(inet->inet_daddr, pin6);
> + }
> + ),
> +
> + TP_printk("sport=%hu, dport=%hu, saddr=%pI4, daddr=%pI4, saddrv6=%pI6, daddrv6=%pI6",
> + __entry->sport, __entry->dport, __entry->saddr, __entry->daddr,
> + __entry->saddr_v6, __entry->daddr_v6)
> +);
> +
> +#endif /* _TRACE_TCP_H */
> +
> +/* This part must be outside protection */
> +#include <trace/define_trace.h>
> diff --git a/net/core/net-traces.c b/net/core/net-traces.c
> index 1132820c8e62..f4e4fa2db505 100644
> --- a/net/core/net-traces.c
> +++ b/net/core/net-traces.c
> @@ -31,6 +31,7 @@
> #include <trace/events/napi.h>
> #include <trace/events/sock.h>
> #include <trace/events/udp.h>
> +#include <trace/events/tcp.h>
> #include <trace/events/fib.h>
> #include <trace/events/qdisc.h>
> #if IS_ENABLED(CONFIG_IPV6)
> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> index 696b0a168f16..e6d6e1393578 100644
> --- a/net/ipv4/tcp_output.c
> +++ b/net/ipv4/tcp_output.c
> @@ -42,6 +42,8 @@
> #include <linux/gfp.h>
> #include <linux/module.h>
>
> +#include <trace/events/tcp.h>
> +
> /* People can turn this off for buggy TCP's found in printers etc. */
> int sysctl_tcp_retrans_collapse __read_mostly = 1;
>
> @@ -2899,6 +2901,7 @@ int tcp_retransmit_skb(struct sock *sk, struct sk_buff *skb, int segs)
> if (!tp->retrans_stamp)
> tp->retrans_stamp = tcp_skb_timestamp(skb);
>
> + trace_tcp_retransmit_skb(sk, skb, segs);
I'm happy to see new tracepoints being added to tcp stack, but I'm concerned
with practical usability of them.
Like the above tracepoint definition makes it not very useful from bpf point of view,
since 'sk' pointer is not recored by as part of the tracepoint.
In bpf/tracing world we prefer tracepoints to have raw pointers recorded
in TP_STRUCT__entry() and _not_ printed in TP_printk()
(since pointers are useless for userspace).
Like trace_kfree_skb() tracepoint records raw 'skb' pointer and we can
walk whatever sk_buff fields we need inside the program.
Such approach allows tracepoint to be usable in many more scenarios, since
bpf program can examine kernel datastructures.
Over the last few years we've been running tcp statistics framework (similar to web10g)
using 8 kprobes in tcp stack with bpf programs extracting the data and now we're
ready to share this experience with the community. Right now we're working on a set
of tracepoints for tcp stack to make the interface more accurate, faster and more stable.
We're planning to send an RFC patch with these new tracepoints in the comming weeks.
More concrete, if you can make this trace_tcp_retransmit_skb() to record
sk, skb pointers and err code at the end of __tcp_retransmit_skb() it will solve
our need as well.
So far our list of kprobes is:
int kprobe__tcp_validate_incoming
int kprobe__tcp_send_active_reset
int kprobe__tcp_v4_send_reset
int kprobe__tcp_v6_send_reset
int kprobe__tcp_v4_destroy_sock
int kprobe__tcp_set_state
int kprobe__tcp_retransmit_skb
int kprobe__tcp_rtx_synack
with tracepoints we can consolidate two of them into one and drop
another one for sure. Notice that tcp_retransmit_skb is on our list too
and currently we're doing extra work inside the program to make it more
accurate which will be unnecessary if this tracepoint is at the end
of __tcp_retransmit_skb().
Thanks
^ permalink raw reply
* Re: [PATCH RFC 0/3] tun zerocopy stats
From: David Miller @ 2017-10-10 17:39 UTC (permalink / raw)
To: willemdebruijn.kernel; +Cc: netdev, mst, jasowang, willemb
In-Reply-To: <CAF=yD-Jc_ikdYLydYwkw+h_+izVEg2Qp66=5+URokyqM=E=zJg@mail.gmail.com>
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Date: Tue, 10 Oct 2017 11:29:33 -0400
> If there is a way to expose these stats through vhost_net directly,
> instead of through tun, that may be better. But I did not see a
> suitable interface. Perhaps debugfs.
Please don't use debugfs, thank you :-)
^ permalink raw reply
* Re: [PATCH][net-next] ipv6: fix dereference of rt6_ex before null check error
From: Martin KaFai Lau @ 2017-10-10 17:40 UTC (permalink / raw)
To: Colin King
Cc: David S . Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, netdev,
kernel-janitors, linux-kernel
In-Reply-To: <20171010170116.21124-1-colin.king@canonical.com>
On Tue, Oct 10, 2017 at 05:01:16PM +0000, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> Currently rt6_ex is being dereferenced before it is null checked
> hence there is a possible null dereference bug. Fix this by only
> dereferencing rt6_ex after it has been null checked.
>
> Detected by CoverityScan, CID#1457749 ("Dereference before null check")
>
> Fixes: 81eb8447daae ("ipv6: take care of rt6_stats")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Acked-by: Martin KaFai Lau <kafai@fb.com>
> ---
> net/ipv6/route.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index 606e80325b21..6db1541eaa7b 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -1152,10 +1152,12 @@ static DEFINE_SPINLOCK(rt6_exception_lock);
> static void rt6_remove_exception(struct rt6_exception_bucket *bucket,
> struct rt6_exception *rt6_ex)
> {
> - struct net *net = dev_net(rt6_ex->rt6i->dst.dev);
> + struct net *net;
>
> if (!bucket || !rt6_ex)
> return;
> +
> + net = dev_net(rt6_ex->rt6i->dst.dev);
> rt6_ex->rt6i->rt6i_node = NULL;
> hlist_del_rcu(&rt6_ex->hlist);
> rt6_release(rt6_ex->rt6i);
> --
> 2.14.1
>
^ permalink raw reply
* Re: [PATCH v5 2/2] net: phy: at803x: Change error to EINVAL for invalid MAC
From: Florian Fainelli @ 2017-10-10 17:43 UTC (permalink / raw)
To: Dan Murphy, andrew; +Cc: netdev, Woojung.Huh, afd
In-Reply-To: <20171010174256.21930-2-dmurphy@ti.com>
On 10/10/2017 10:42 AM, Dan Murphy wrote:
> Change the return error code to EINVAL if the MAC
> address is not valid in the set_wol function.
>
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply
* [PATCH v5 2/2] net: phy: at803x: Change error to EINVAL for invalid MAC
From: Dan Murphy @ 2017-10-10 17:42 UTC (permalink / raw)
To: andrew, f.fainelli; +Cc: netdev, Woojung.Huh, afd, Dan Murphy
In-Reply-To: <20171010174256.21930-1-dmurphy@ti.com>
Change the return error code to EINVAL if the MAC
address is not valid in the set_wol function.
Signed-off-by: Dan Murphy <dmurphy@ti.com>
---
v5 - No changes made
v4 - Updated $subject to include the part number -
https://www.mail-archive.com/netdev@vger.kernel.org/msg192424.html
v3 - No changes made
v2 - There was no v1 on this patch this is new.
drivers/net/phy/at803x.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index c1e52b9dc58d..5f93e6add563 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -167,7 +167,7 @@ static int at803x_set_wol(struct phy_device *phydev,
mac = (const u8 *) ndev->dev_addr;
if (!is_valid_ether_addr(mac))
- return -EFAULT;
+ return -EINVAL;
for (i = 0; i < 3; i++) {
phy_write(phydev, AT803X_MMD_ACCESS_CONTROL,
--
2.14.0
^ permalink raw reply related
* [PATCH v5 1/2] net: phy: DP83822 initial driver submission
From: Dan Murphy @ 2017-10-10 17:42 UTC (permalink / raw)
To: andrew, f.fainelli; +Cc: netdev, Woojung.Huh, afd, Dan Murphy
Add support for the TI DP83822 10/100Mbit ethernet phy.
The DP83822 provides flexibility to connect to a MAC through a
standard MII, RMII or RGMII interface.
In addition the DP83822 needs to be removed from the DP83848 driver
as the WoL support is added here for this device.
Datasheet:
http://www.ti.com/product/DP83822I/datasheet
Signed-off-by: Dan Murphy <dmurphy@ti.com>
---
v5 - Fixed bit mask, added config_init to set WOL register bits, fixed spacing
issues, moved password storage under secure on if statement, and added INT_EN bit
setting for interrupt enable.
https://www.mail-archive.com/netdev@vger.kernel.org/msg192495.html
v4 - Squash DP83822 removal patch into this patch -
https://www.mail-archive.com/netdev@vger.kernel.org/msg192424.html
v3 - Fixed WoL indication bit and removed mutex for suspend/resume -
https://www.mail-archive.com/netdev@vger.kernel.org/msg191891.html and
https://www.mail-archive.com/netdev@vger.kernel.org/msg191665.html
v2 - Updated per comments. Removed unnessary parantheis, called genphy_suspend/
resume routines and then performing WoL changes, reworked sopass storage and reduced
the number of phy reads, and moved WOL_SECURE_ON -
https://www.mail-archive.com/netdev@vger.kernel.org/msg191392.html
drivers/net/phy/Kconfig | 5 +
drivers/net/phy/Makefile | 1 +
drivers/net/phy/dp83822.c | 344 ++++++++++++++++++++++++++++++++++++++++++++++
drivers/net/phy/dp83848.c | 3 -
4 files changed, 350 insertions(+), 3 deletions(-)
create mode 100644 drivers/net/phy/dp83822.c
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index cd931cf9dcc2..8e78a482e09e 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -277,6 +277,11 @@ config DAVICOM_PHY
---help---
Currently supports dm9161e and dm9131
+config DP83822_PHY
+ tristate "Texas Instruments DP83822 PHY"
+ ---help---
+ Supports the DP83822 PHY.
+
config DP83848_PHY
tristate "Texas Instruments DP83848 PHY"
---help---
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index 416df92fbf4f..df3b82ba8550 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -55,6 +55,7 @@ obj-$(CONFIG_CICADA_PHY) += cicada.o
obj-$(CONFIG_CORTINA_PHY) += cortina.o
obj-$(CONFIG_DAVICOM_PHY) += davicom.o
obj-$(CONFIG_DP83640_PHY) += dp83640.o
+obj-$(CONFIG_DP83822_PHY) += dp83822.o
obj-$(CONFIG_DP83848_PHY) += dp83848.o
obj-$(CONFIG_DP83867_PHY) += dp83867.o
obj-$(CONFIG_FIXED_PHY) += fixed_phy.o
diff --git a/drivers/net/phy/dp83822.c b/drivers/net/phy/dp83822.c
new file mode 100644
index 000000000000..14335d14e9e4
--- /dev/null
+++ b/drivers/net/phy/dp83822.c
@@ -0,0 +1,344 @@
+/*
+ * Driver for the Texas Instruments DP83822 PHY
+ *
+ * Copyright (C) 2017 Texas Instruments Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License.
+ *
+ * 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 General Public License for more details.
+ */
+
+#include <linux/ethtool.h>
+#include <linux/etherdevice.h>
+#include <linux/kernel.h>
+#include <linux/mii.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/phy.h>
+#include <linux/netdevice.h>
+
+#define DP83822_PHY_ID 0x2000a240
+#define DP83822_DEVADDR 0x1f
+
+#define MII_DP83822_PHYSCR 0x11
+#define MII_DP83822_MISR1 0x12
+#define MII_DP83822_MISR2 0x13
+#define MII_DP83822_RESET_CTRL 0x1f
+
+#define DP83822_HW_RESET BIT(15)
+#define DP83822_SW_RESET BIT(14)
+
+/* PHYSCR Register Fields */
+#define DP83822_PHYSCR_INT_OE BIT(0) /* Interrupt Output Enable */
+#define DP83822_PHYSCR_INTEN BIT(1) /* Interrupt Enable */
+
+/* MISR1 bits */
+#define DP83822_RX_ERR_HF_INT_EN BIT(0)
+#define DP83822_FALSE_CARRIER_HF_INT_EN BIT(1)
+#define DP83822_ANEG_COMPLETE_INT_EN BIT(2)
+#define DP83822_DUP_MODE_CHANGE_INT_EN BIT(3)
+#define DP83822_SPEED_CHANGED_INT_EN BIT(4)
+#define DP83822_LINK_STAT_INT_EN BIT(5)
+#define DP83822_ENERGY_DET_INT_EN BIT(6)
+#define DP83822_LINK_QUAL_INT_EN BIT(7)
+
+/* MISR2 bits */
+#define DP83822_JABBER_DET_INT_EN BIT(0)
+#define DP83822_WOL_PKT_INT_EN BIT(1)
+#define DP83822_SLEEP_MODE_INT_EN BIT(2)
+#define DP83822_MDI_XOVER_INT_EN BIT(3)
+#define DP83822_LB_FIFO_INT_EN BIT(4)
+#define DP83822_PAGE_RX_INT_EN BIT(5)
+#define DP83822_ANEG_ERR_INT_EN BIT(6)
+#define DP83822_EEE_ERROR_CHANGE_INT_EN BIT(7)
+
+/* INT_STAT1 bits */
+#define DP83822_WOL_INT_EN BIT(4)
+#define DP83822_WOL_INT_STAT BIT(12)
+
+#define MII_DP83822_RXSOP1 0x04a5
+#define MII_DP83822_RXSOP2 0x04a6
+#define MII_DP83822_RXSOP3 0x04a7
+
+/* WoL Registers */
+#define MII_DP83822_WOL_CFG 0x04a0
+#define MII_DP83822_WOL_STAT 0x04a1
+#define MII_DP83822_WOL_DA1 0x04a2
+#define MII_DP83822_WOL_DA2 0x04a3
+#define MII_DP83822_WOL_DA3 0x04a4
+
+/* WoL bits */
+#define DP83822_WOL_MAGIC_EN BIT(0)
+#define DP83822_WOL_SECURE_ON BIT(5)
+#define DP83822_WOL_EN BIT(7)
+#define DP83822_WOL_INDICATION_SEL BIT(8)
+#define DP83822_WOL_CLR_INDICATION BIT(11)
+
+static int dp83822_ack_interrupt(struct phy_device *phydev)
+{
+ int err;
+
+ err = phy_read(phydev, MII_DP83822_MISR1);
+ if (err < 0)
+ return err;
+
+ err = phy_read(phydev, MII_DP83822_MISR2);
+ if (err < 0)
+ return err;
+
+ return 0;
+}
+
+static int dp83822_set_wol(struct phy_device *phydev,
+ struct ethtool_wolinfo *wol)
+{
+ struct net_device *ndev = phydev->attached_dev;
+ u16 value;
+ const u8 *mac;
+
+ if (wol->wolopts & (WAKE_MAGIC | WAKE_MAGICSECURE)) {
+ mac = (const u8 *)ndev->dev_addr;
+
+ if (!is_valid_ether_addr(mac))
+ return -EINVAL;
+
+ /* MAC addresses start with byte 5, but stored in mac[0].
+ * 822 PHYs store bytes 4|5, 2|3, 0|1
+ */
+ phy_write_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_DA1,
+ (mac[1] << 8) | mac[0]);
+ phy_write_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_DA2,
+ (mac[3] << 8) | mac[2]);
+ phy_write_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_DA3,
+ (mac[5] << 8) | mac[4]);
+
+ value = phy_read_mmd(phydev, DP83822_DEVADDR,
+ MII_DP83822_WOL_CFG);
+ if (wol->wolopts & WAKE_MAGIC)
+ value |= DP83822_WOL_MAGIC_EN;
+ else
+ value &= ~DP83822_WOL_MAGIC_EN;
+
+ if (wol->wolopts & WAKE_MAGICSECURE) {
+ phy_write_mmd(phydev, DP83822_DEVADDR,
+ MII_DP83822_RXSOP1,
+ (wol->sopass[1] << 8) | wol->sopass[0]);
+ phy_write_mmd(phydev, DP83822_DEVADDR,
+ MII_DP83822_RXSOP2,
+ (wol->sopass[3] << 8) | wol->sopass[2]);
+ phy_write_mmd(phydev, DP83822_DEVADDR,
+ MII_DP83822_RXSOP3,
+ (wol->sopass[5] << 8) | wol->sopass[4]);
+ value |= DP83822_WOL_SECURE_ON;
+ } else {
+ value &= ~DP83822_WOL_SECURE_ON;
+ }
+
+ value |= (DP83822_WOL_EN | DP83822_WOL_INDICATION_SEL |
+ DP83822_WOL_CLR_INDICATION);
+ phy_write_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_CFG,
+ value);
+ } else {
+ value = phy_read_mmd(phydev, DP83822_DEVADDR,
+ MII_DP83822_WOL_CFG);
+ value &= ~DP83822_WOL_EN;
+ phy_write_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_CFG,
+ value);
+ }
+
+ return 0;
+}
+
+static void dp83822_get_wol(struct phy_device *phydev,
+ struct ethtool_wolinfo *wol)
+{
+ int value;
+ u16 sopass_val;
+
+ wol->supported = (WAKE_MAGIC | WAKE_MAGICSECURE);
+ wol->wolopts = 0;
+
+ value = phy_read_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_CFG);
+
+ if (value & DP83822_WOL_MAGIC_EN)
+ wol->wolopts |= WAKE_MAGIC;
+
+ if (value & DP83822_WOL_SECURE_ON) {
+ sopass_val = phy_read_mmd(phydev, DP83822_DEVADDR,
+ MII_DP83822_RXSOP1);
+ wol->sopass[0] = (sopass_val & 0xff);
+ wol->sopass[1] = (sopass_val >> 8);
+
+ sopass_val = phy_read_mmd(phydev, DP83822_DEVADDR,
+ MII_DP83822_RXSOP2);
+ wol->sopass[2] = (sopass_val & 0xff);
+ wol->sopass[3] = (sopass_val >> 8);
+
+ sopass_val = phy_read_mmd(phydev, DP83822_DEVADDR,
+ MII_DP83822_RXSOP3);
+ wol->sopass[4] = (sopass_val & 0xff);
+ wol->sopass[5] = (sopass_val >> 8);
+
+ wol->wolopts |= WAKE_MAGICSECURE;
+ }
+
+ /* WoL is not enabled so set wolopts to 0 */
+ if (!(value & DP83822_WOL_EN))
+ wol->wolopts = 0;
+}
+
+static int dp83822_config_intr(struct phy_device *phydev)
+{
+ int misr_status;
+ int physcr_status;
+ int err;
+
+ if (phydev->interrupts == PHY_INTERRUPT_ENABLED) {
+ misr_status = phy_read(phydev, MII_DP83822_MISR1);
+ if (misr_status < 0)
+ return misr_status;
+
+ misr_status |= (DP83822_RX_ERR_HF_INT_EN |
+ DP83822_FALSE_CARRIER_HF_INT_EN |
+ DP83822_ANEG_COMPLETE_INT_EN |
+ DP83822_DUP_MODE_CHANGE_INT_EN |
+ DP83822_SPEED_CHANGED_INT_EN |
+ DP83822_LINK_STAT_INT_EN |
+ DP83822_ENERGY_DET_INT_EN |
+ DP83822_LINK_QUAL_INT_EN);
+
+ err = phy_write(phydev, MII_DP83822_MISR1, misr_status);
+ if (err < 0)
+ return err;
+
+ misr_status = phy_read(phydev, MII_DP83822_MISR2);
+ if (misr_status < 0)
+ return misr_status;
+
+ misr_status |= (DP83822_JABBER_DET_INT_EN |
+ DP83822_WOL_PKT_INT_EN |
+ DP83822_SLEEP_MODE_INT_EN |
+ DP83822_MDI_XOVER_INT_EN |
+ DP83822_LB_FIFO_INT_EN |
+ DP83822_PAGE_RX_INT_EN |
+ DP83822_ANEG_ERR_INT_EN |
+ DP83822_EEE_ERROR_CHANGE_INT_EN);
+
+ err = phy_write(phydev, MII_DP83822_MISR2, misr_status);
+ if (err < 0)
+ return err;
+
+ physcr_status = phy_read(phydev, MII_DP83822_PHYSCR);
+ if (physcr_status < 0)
+ return physcr_status;
+
+ physcr_status |= DP83822_PHYSCR_INT_OE | DP83822_PHYSCR_INTEN;
+
+ } else {
+ err = phy_write(phydev, MII_DP83822_MISR1, 0);
+ if (err < 0)
+ return err;
+
+ err = phy_write(phydev, MII_DP83822_MISR1, 0);
+ if (err < 0)
+ return err;
+
+ physcr_status = phy_read(phydev, MII_DP83822_PHYSCR);
+ if (physcr_status < 0)
+ return physcr_status;
+
+ physcr_status &= ~DP83822_PHYSCR_INTEN;
+ }
+
+ return phy_write(phydev, MII_DP83822_PHYSCR, physcr_status);
+}
+
+static int dp83822_config_init(struct phy_device *phydev)
+{
+ int err;
+ int value;
+
+ err = genphy_config_init(phydev);
+ if (err < 0)
+ return err;
+
+ value = DP83822_WOL_MAGIC_EN | DP83822_WOL_SECURE_ON | DP83822_WOL_EN;
+
+ return phy_write_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_CFG,
+ value);
+}
+
+static int dp83822_phy_reset(struct phy_device *phydev)
+{
+ int err;
+
+ err = phy_write(phydev, MII_DP83822_RESET_CTRL, DP83822_HW_RESET);
+ if (err < 0)
+ return err;
+
+ dp83822_config_init(phydev);
+
+ return 0;
+}
+
+static int dp83822_suspend(struct phy_device *phydev)
+{
+ int value;
+
+ value = phy_read_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_CFG);
+
+ if (!(value & DP83822_WOL_EN))
+ genphy_suspend(phydev);
+
+ return 0;
+}
+
+static int dp83822_resume(struct phy_device *phydev)
+{
+ int value;
+
+ genphy_resume(phydev);
+
+ value = phy_read_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_CFG);
+
+ phy_write_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_CFG, value |
+ DP83822_WOL_CLR_INDICATION);
+
+ return 0;
+}
+
+static struct phy_driver dp83822_driver[] = {
+ {
+ .phy_id = DP83822_PHY_ID,
+ .phy_id_mask = 0xfffffff0,
+ .name = "TI DP83822",
+ .features = PHY_BASIC_FEATURES,
+ .flags = PHY_HAS_INTERRUPT,
+ .config_init = dp83822_config_init,
+ .soft_reset = dp83822_phy_reset,
+ .get_wol = dp83822_get_wol,
+ .set_wol = dp83822_set_wol,
+ .ack_interrupt = dp83822_ack_interrupt,
+ .config_intr = dp83822_config_intr,
+ .config_aneg = genphy_config_aneg,
+ .read_status = genphy_read_status,
+ .suspend = dp83822_suspend,
+ .resume = dp83822_resume,
+ },
+};
+module_phy_driver(dp83822_driver);
+
+static struct mdio_device_id __maybe_unused dp83822_tbl[] = {
+ { DP83822_PHY_ID, 0xfffffff0 },
+ { },
+};
+MODULE_DEVICE_TABLE(mdio, dp83822_tbl);
+
+MODULE_DESCRIPTION("Texas Instruments DP83822 PHY driver");
+MODULE_AUTHOR("Dan Murphy <dmurphy@ti.com");
+MODULE_LICENSE("GPL");
diff --git a/drivers/net/phy/dp83848.c b/drivers/net/phy/dp83848.c
index 3de4fe4dda77..3966d43c5146 100644
--- a/drivers/net/phy/dp83848.c
+++ b/drivers/net/phy/dp83848.c
@@ -20,7 +20,6 @@
#define TI_DP83620_PHY_ID 0x20005ce0
#define NS_DP83848C_PHY_ID 0x20005c90
#define TLK10X_PHY_ID 0x2000a210
-#define TI_DP83822_PHY_ID 0x2000a240
/* Registers */
#define DP83848_MICR 0x11 /* MII Interrupt Control Register */
@@ -80,7 +79,6 @@ static struct mdio_device_id __maybe_unused dp83848_tbl[] = {
{ NS_DP83848C_PHY_ID, 0xfffffff0 },
{ TI_DP83620_PHY_ID, 0xfffffff0 },
{ TLK10X_PHY_ID, 0xfffffff0 },
- { TI_DP83822_PHY_ID, 0xfffffff0 },
{ }
};
MODULE_DEVICE_TABLE(mdio, dp83848_tbl);
@@ -110,7 +108,6 @@ static struct phy_driver dp83848_driver[] = {
DP83848_PHY_DRIVER(NS_DP83848C_PHY_ID, "NS DP83848C 10/100 Mbps PHY"),
DP83848_PHY_DRIVER(TI_DP83620_PHY_ID, "TI DP83620 10/100 Mbps PHY"),
DP83848_PHY_DRIVER(TLK10X_PHY_ID, "TI TLK10X 10/100 Mbps PHY"),
- DP83848_PHY_DRIVER(TI_DP83822_PHY_ID, "TI DP83822 10/100 Mbps PHY"),
};
module_phy_driver(dp83848_driver);
--
2.14.0
^ permalink raw reply related
* Re: [patch net-next 1/4] net: sched: make tc_action_ops->get_dev return dev and avoid passing net
From: Cong Wang @ 2017-10-10 17:44 UTC (permalink / raw)
To: Jiri Pirko
Cc: Linux Kernel Network Developers, David Miller, Jamal Hadi Salim,
Saeed Mahameed, matanb, leonro, mlxsw
In-Reply-To: <20171010073016.3682-2-jiri@resnulli.us>
On Tue, Oct 10, 2017 at 12:30 AM, Jiri Pirko <jiri@resnulli.us> wrote:
> -static int tcf_mirred_device(const struct tc_action *a, struct net *net,
> - struct net_device **mirred_dev)
> +static struct net_device *tcf_mirred_get_dev(const struct tc_action *a)
> {
> - int ifindex = tcf_mirred_ifindex(a);
> + struct tcf_mirred *m = to_mirred(a);
>
> - *mirred_dev = __dev_get_by_index(net, ifindex);
> - if (!*mirred_dev)
> - return -EINVAL;
> - return 0;
> + return __dev_get_by_index(m->net, m->tcfm_ifindex);
Hmm, why not just return m->tcfm_dev?
^ permalink raw reply
* Re: [PATCH net-next v2 5/5] selinux: bpf: Add addtional check for bpf object file receive
From: Chenbo Feng @ 2017-10-10 17:48 UTC (permalink / raw)
To: Stephen Smalley
Cc: Chenbo Feng, linux-security-module, netdev, SELinux,
Daniel Borkmann, Alexei Starovoitov, Lorenzo Colitti
In-Reply-To: <1507645461.30616.9.camel@tycho.nsa.gov>
On Tue, Oct 10, 2017 at 7:24 AM, Stephen Smalley <sds@tycho.nsa.gov> wrote:
> On Mon, 2017-10-09 at 15:20 -0700, Chenbo Feng wrote:
>> From: Chenbo Feng <fengc@google.com>
>>
>> Introduce a bpf object related check when sending and receiving files
>> through unix domain socket as well as binder. It checks if the
>> receiving
>> process have privilege to read/write the bpf map or use the bpf
>> program.
>> This check is necessary because the bpf maps and programs are using a
>> anonymous inode as their shared inode so the normal way of checking
>> the
>> files and sockets when passing between processes cannot work properly
>> on
>> eBPF object. This check only works when the BPF_SYSCALL is
>> configured.
>> The information stored inside the file security struct is the same as
>> the information in bpf object security struct.
>>
>> Signed-off-by: Chenbo Feng <fengc@google.com>
>> ---
>> include/linux/bpf.h | 3 +++
>> include/linux/lsm_hooks.h | 17 +++++++++++++
>> include/linux/security.h | 9 +++++++
>> kernel/bpf/syscall.c | 4 ++--
>> security/security.c | 8 +++++++
>> security/selinux/hooks.c | 61
>> +++++++++++++++++++++++++++++++++++++++++++++++
>> 6 files changed, 100 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
>> index 225740688ab7..81d6c01b8825 100644
>> --- a/include/linux/bpf.h
>> +++ b/include/linux/bpf.h
>> @@ -285,6 +285,9 @@ int bpf_prog_array_copy_to_user(struct
>> bpf_prog_array __rcu *progs,
>> #ifdef CONFIG_BPF_SYSCALL
>> DECLARE_PER_CPU(int, bpf_prog_active);
>>
>> +extern const struct file_operations bpf_map_fops;
>> +extern const struct file_operations bpf_prog_fops;
>> +
>> #define BPF_PROG_TYPE(_id, _ops) \
>> extern const struct bpf_verifier_ops _ops;
>> #define BPF_MAP_TYPE(_id, _ops) \
>> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
>> index 7161d8e7ee79..517dea60b87b 100644
>> --- a/include/linux/lsm_hooks.h
>> +++ b/include/linux/lsm_hooks.h
>> @@ -1385,6 +1385,19 @@
>> * @bpf_prog_free_security:
>> * Clean up the security information stored inside bpf prog.
>> *
>> + * @bpf_map_file:
>> + * When creating a bpf map fd, set up the file security
>> information with
>> + * the bpf security information stored in the map struct. So
>> when the map
>> + * fd is passed between processes, the security module can
>> directly read
>> + * the security information from file security struct rather
>> than the bpf
>> + * security struct.
>> + *
>> + * @bpf_prog_file:
>> + * When creating a bpf prog fd, set up the file security
>> information with
>> + * the bpf security information stored in the prog struct. So
>> when the prog
>> + * fd is passed between processes, the security module can
>> directly read
>> + * the security information from file security struct rather
>> than the bpf
>> + * security struct.
>> */
>> union security_list_options {
>> int (*binder_set_context_mgr)(struct task_struct *mgr);
>> @@ -1726,6 +1739,8 @@ union security_list_options {
>> void (*bpf_map_free_security)(struct bpf_map *map);
>> int (*bpf_prog_alloc_security)(struct bpf_prog_aux *aux);
>> void (*bpf_prog_free_security)(struct bpf_prog_aux *aux);
>> + void (*bpf_map_file)(struct bpf_map *map, struct file
>> *file);
>> + void (*bpf_prog_file)(struct bpf_prog_aux *aux, struct file
>> *file);
>> #endif /* CONFIG_BPF_SYSCALL */
>> };
>>
>> @@ -1954,6 +1969,8 @@ struct security_hook_heads {
>> struct list_head bpf_map_free_security;
>> struct list_head bpf_prog_alloc_security;
>> struct list_head bpf_prog_free_security;
>> + struct list_head bpf_map_file;
>> + struct list_head bpf_prog_file;
>> #endif /* CONFIG_BPF_SYSCALL */
>> } __randomize_layout;
>>
>> diff --git a/include/linux/security.h b/include/linux/security.h
>> index 18800b0911e5..57573b794e2d 100644
>> --- a/include/linux/security.h
>> +++ b/include/linux/security.h
>> @@ -1740,6 +1740,8 @@ extern int security_bpf_map_alloc(struct
>> bpf_map *map);
>> extern void security_bpf_map_free(struct bpf_map *map);
>> extern int security_bpf_prog_alloc(struct bpf_prog_aux *aux);
>> extern void security_bpf_prog_free(struct bpf_prog_aux *aux);
>> +extern void security_bpf_map_file(struct bpf_map *map, struct file
>> *file);
>> +extern void security_bpf_prog_file(struct bpf_prog_aux *aux, struct
>> file *file);
>> #else
>> static inline int security_bpf(int cmd, union bpf_attr *attr,
>> unsigned int size)
>> @@ -1772,6 +1774,13 @@ static inline int
>> security_bpf_prog_alloc(struct bpf_prog_aux *aux)
>>
>> static inline void security_bpf_prog_free(struct bpf_prog_aux *aux)
>> { }
>> +
>> +static inline void security_bpf_map_file(struct bpf_map *map, struct
>> file *file)
>> +{ }
>> +
>> +static inline void security_bpf_prog_file(struct bpf_prog_aux *aux,
>> + struct file *file)
>> +{ }
>> #endif /* CONFIG_SECURITY */
>> #endif /* CONFIG_BPF_SYSCALL */
>>
>> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
>> index 1cf31ddd7616..b144181d3f3a 100644
>> --- a/kernel/bpf/syscall.c
>> +++ b/kernel/bpf/syscall.c
>> @@ -313,7 +313,7 @@ static ssize_t bpf_dummy_write(struct file *filp,
>> const char __user *buf,
>> return -EINVAL;
>> }
>>
>> -static const struct file_operations bpf_map_fops = {
>> +const struct file_operations bpf_map_fops = {
>> #ifdef CONFIG_PROC_FS
>> .show_fdinfo = bpf_map_show_fdinfo,
>> #endif
>> @@ -964,7 +964,7 @@ static void bpf_prog_show_fdinfo(struct seq_file
>> *m, struct file *filp)
>> }
>> #endif
>>
>> -static const struct file_operations bpf_prog_fops = {
>> +const struct file_operations bpf_prog_fops = {
>> #ifdef CONFIG_PROC_FS
>> .show_fdinfo = bpf_prog_show_fdinfo,
>> #endif
>> diff --git a/security/security.c b/security/security.c
>> index 1cd8526cb0b7..dacf649b8cfa 100644
>> --- a/security/security.c
>> +++ b/security/security.c
>> @@ -1734,4 +1734,12 @@ void security_bpf_prog_free(struct
>> bpf_prog_aux *aux)
>> {
>> call_void_hook(bpf_prog_free_security, aux);
>> }
>> +void security_bpf_map_file(struct bpf_map *map, struct file *file)
>> +{
>> + call_void_hook(bpf_map_file, map, file);
>> +}
>> +void security_bpf_prog_file(struct bpf_prog_aux *aux, struct file
>> *file)
>> +{
>> + call_void_hook(bpf_prog_file, aux, file);
>> +}
>> #endif /* CONFIG_BPF_SYSCALL */
>> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
>> index 41aba4e3d57c..fea88655e0ee 100644
>> --- a/security/selinux/hooks.c
>> +++ b/security/selinux/hooks.c
>> @@ -1815,6 +1815,10 @@ static inline int file_path_has_perm(const
>> struct cred *cred,
>> return inode_has_perm(cred, file_inode(file), av, &ad);
>> }
>>
>> +#ifdef CONFIG_BPF_SYSCALL
>> +static int bpf_file_check(struct file *file, u32 sid);
>> +#endif
>> +
>> /* Check whether a task can use an open file descriptor to
>> access an inode in a given way. Check access to the
>> descriptor itself, and then use dentry_has_perm to
>> @@ -1845,6 +1849,12 @@ static int file_has_perm(const struct cred
>> *cred,
>> goto out;
>> }
>>
>> +#ifdef CONFIG_BPF_SYSCALL
>> + rc = bpf_file_check(file, cred_sid(cred));
>> + if (rc)
>> + goto out;
>> +#endif
>> +
>> /* av is zero if only checking access to the descriptor. */
>> rc = 0;
>> if (av)
>> @@ -2165,6 +2175,12 @@ static int selinux_binder_transfer_file(struct
>> task_struct *from,
>> return rc;
>> }
>>
>> +#ifdef CONFIG_BPF_SYSCALL
>> + rc = bpf_file_check(file, sid);
>> + if (rc)
>> + return rc;
>> +#endif
>> +
>> if (unlikely(IS_PRIVATE(d_backing_inode(dentry))))
>> return 0;
>>
>> @@ -6288,6 +6304,33 @@ static u32 bpf_map_fmode_to_av(fmode_t fmode)
>> return av;
>> }
>>
>> +/* This function will check the file pass through unix socket or
>> binder to see
>> + * if it is a bpf related object. And apply correspinding checks on
>> the bpf
>> + * object based on the type. The bpf maps and programs, not like
>> other files and
>> + * socket, are using a shared anonymous inode inside the kernel as
>> their inode.
>> + * So checking that inode cannot identify if the process have
>> privilege to
>> + * access the bpf object and that's why we have to add this
>> additional check in
>> + * selinux_file_receive and selinux_binder_transfer_files.
>> + */
>> +static int bpf_file_check(struct file *file, u32 sid)
>> +{
>> + struct file_security_struct *fsec = file->f_security;
>> + int ret;
>> +
>> + if (file->f_op == &bpf_map_fops) {
>> + ret = avc_has_perm(sid, fsec->sid, SECCLASS_BPF_MAP,
>> + bpf_map_fmode_to_av(file-
>> >f_mode), NULL);
>> + if (ret)
>> + return ret;
>> + } else if (file->f_op == &bpf_prog_fops) {
>> + ret = avc_has_perm(sid, fsec->sid,
>> SECCLASS_BPF_PROG,
>> + BPF_PROG__USE, NULL);
>> + if (ret)
>> + return ret;
>> + }
>> + return 0;
>> +}
>> +
>> static int selinux_bpf_map(struct bpf_map *map, fmode_t fmode)
>> {
>> u32 sid = current_sid();
>> @@ -6351,6 +6394,22 @@ static void selinux_bpf_prog_free(struct
>> bpf_prog_aux *aux)
>> aux->security = NULL;
>> kfree(bpfsec);
>> }
>> +
>> +static void selinux_bpf_map_file(struct bpf_map *map, struct file
>> *file)
>> +{
>> + struct bpf_security_struct *bpfsec = map->security;
>> + struct file_security_struct *fsec = file->f_security;
>> +
>> + fsec->sid = bpfsec->sid;
>> +}
>> +
>> +static void selinux_bpf_prog_file(struct bpf_prog_aux *aux, struct
>> file *file)
>> +{
>> + struct bpf_security_struct *bpfsec = aux->security;
>> + struct file_security_struct *fsec = file->f_security;
>> +
>> + fsec->sid = bpfsec->sid;
>
> I could be wrong, but isn't it the case that fsec->sid already will
> equal bpfsec->sid, because they are both created by the same thread
> during the same system call, and they each inherit the SID of the
> current task?
>
This is true when bpf object is created by the same process that
obtains the fd. But there are other ways of getting a bpf object fd
from the kernel such as bpf_obj_get and bpf_get_obj_fd_by_id. These
action will ask the kernel to allocate a new file for the bpf object
and the file sid would be the process ask for fd while the bpfsec->sid
is the sid when bpf object get created. These two could be different.
> What I expected you to do was to add and set a flags field in the
> file_security_struct to indicate that this is a bpf map or prog, and
> then test for that in your bpf_file_check() function instead of having
> to export and test the fops structures.
>
>
>> +}
>> #endif
>>
>> static struct security_hook_list selinux_hooks[] __lsm_ro_after_init
>> = {
>> @@ -6581,6 +6640,8 @@ static struct security_hook_list
>> selinux_hooks[] __lsm_ro_after_init = {
>> LSM_HOOK_INIT(bpf_prog_alloc_security,
>> selinux_bpf_prog_alloc),
>> LSM_HOOK_INIT(bpf_map_free_security, selinux_bpf_map_free),
>> LSM_HOOK_INIT(bpf_prog_free_security,
>> selinux_bpf_prog_free),
>> + LSM_HOOK_INIT(bpf_map_file, selinux_bpf_map_file),
>> + LSM_HOOK_INIT(bpf_prog_file, selinux_bpf_prog_file),
>> #endif
>> };
>>
^ permalink raw reply
* Re: [PATCH v5 1/2] net: phy: DP83822 initial driver submission
From: Andrew F. Davis @ 2017-10-10 17:49 UTC (permalink / raw)
To: Dan Murphy, andrew, f.fainelli; +Cc: netdev, Woojung.Huh
In-Reply-To: <20171010174256.21930-1-dmurphy@ti.com>
On 10/10/2017 12:42 PM, Dan Murphy wrote:
> Add support for the TI DP83822 10/100Mbit ethernet phy.
>
> The DP83822 provides flexibility to connect to a MAC through a
> standard MII, RMII or RGMII interface.
>
> In addition the DP83822 needs to be removed from the DP83848 driver
> as the WoL support is added here for this device.
>
> Datasheet:
> http://www.ti.com/product/DP83822I/datasheet
>
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> ---
Looks much nicer now, thanks for dealing with my nitpicking :)
Acked-by: Andrew F. Davis <afd@ti.com>
>
> v5 - Fixed bit mask, added config_init to set WOL register bits, fixed spacing
> issues, moved password storage under secure on if statement, and added INT_EN bit
> setting for interrupt enable.
> https://www.mail-archive.com/netdev@vger.kernel.org/msg192495.html
>
> v4 - Squash DP83822 removal patch into this patch -
> https://www.mail-archive.com/netdev@vger.kernel.org/msg192424.html
>
> v3 - Fixed WoL indication bit and removed mutex for suspend/resume -
> https://www.mail-archive.com/netdev@vger.kernel.org/msg191891.html and
> https://www.mail-archive.com/netdev@vger.kernel.org/msg191665.html
>
> v2 - Updated per comments. Removed unnessary parantheis, called genphy_suspend/
> resume routines and then performing WoL changes, reworked sopass storage and reduced
> the number of phy reads, and moved WOL_SECURE_ON -
> https://www.mail-archive.com/netdev@vger.kernel.org/msg191392.html
>
> drivers/net/phy/Kconfig | 5 +
> drivers/net/phy/Makefile | 1 +
> drivers/net/phy/dp83822.c | 344 ++++++++++++++++++++++++++++++++++++++++++++++
> drivers/net/phy/dp83848.c | 3 -
> 4 files changed, 350 insertions(+), 3 deletions(-)
> create mode 100644 drivers/net/phy/dp83822.c
>
> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
> index cd931cf9dcc2..8e78a482e09e 100644
> --- a/drivers/net/phy/Kconfig
> +++ b/drivers/net/phy/Kconfig
> @@ -277,6 +277,11 @@ config DAVICOM_PHY
> ---help---
> Currently supports dm9161e and dm9131
>
> +config DP83822_PHY
> + tristate "Texas Instruments DP83822 PHY"
> + ---help---
> + Supports the DP83822 PHY.
> +
> config DP83848_PHY
> tristate "Texas Instruments DP83848 PHY"
> ---help---
> diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
> index 416df92fbf4f..df3b82ba8550 100644
> --- a/drivers/net/phy/Makefile
> +++ b/drivers/net/phy/Makefile
> @@ -55,6 +55,7 @@ obj-$(CONFIG_CICADA_PHY) += cicada.o
> obj-$(CONFIG_CORTINA_PHY) += cortina.o
> obj-$(CONFIG_DAVICOM_PHY) += davicom.o
> obj-$(CONFIG_DP83640_PHY) += dp83640.o
> +obj-$(CONFIG_DP83822_PHY) += dp83822.o
> obj-$(CONFIG_DP83848_PHY) += dp83848.o
> obj-$(CONFIG_DP83867_PHY) += dp83867.o
> obj-$(CONFIG_FIXED_PHY) += fixed_phy.o
> diff --git a/drivers/net/phy/dp83822.c b/drivers/net/phy/dp83822.c
> new file mode 100644
> index 000000000000..14335d14e9e4
> --- /dev/null
> +++ b/drivers/net/phy/dp83822.c
> @@ -0,0 +1,344 @@
> +/*
> + * Driver for the Texas Instruments DP83822 PHY
> + *
> + * Copyright (C) 2017 Texas Instruments Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License.
> + *
> + * 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 General Public License for more details.
> + */
> +
> +#include <linux/ethtool.h>
> +#include <linux/etherdevice.h>
> +#include <linux/kernel.h>
> +#include <linux/mii.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/phy.h>
> +#include <linux/netdevice.h>
> +
> +#define DP83822_PHY_ID 0x2000a240
> +#define DP83822_DEVADDR 0x1f
> +
> +#define MII_DP83822_PHYSCR 0x11
> +#define MII_DP83822_MISR1 0x12
> +#define MII_DP83822_MISR2 0x13
> +#define MII_DP83822_RESET_CTRL 0x1f
> +
> +#define DP83822_HW_RESET BIT(15)
> +#define DP83822_SW_RESET BIT(14)
> +
> +/* PHYSCR Register Fields */
> +#define DP83822_PHYSCR_INT_OE BIT(0) /* Interrupt Output Enable */
> +#define DP83822_PHYSCR_INTEN BIT(1) /* Interrupt Enable */
> +
> +/* MISR1 bits */
> +#define DP83822_RX_ERR_HF_INT_EN BIT(0)
> +#define DP83822_FALSE_CARRIER_HF_INT_EN BIT(1)
> +#define DP83822_ANEG_COMPLETE_INT_EN BIT(2)
> +#define DP83822_DUP_MODE_CHANGE_INT_EN BIT(3)
> +#define DP83822_SPEED_CHANGED_INT_EN BIT(4)
> +#define DP83822_LINK_STAT_INT_EN BIT(5)
> +#define DP83822_ENERGY_DET_INT_EN BIT(6)
> +#define DP83822_LINK_QUAL_INT_EN BIT(7)
> +
> +/* MISR2 bits */
> +#define DP83822_JABBER_DET_INT_EN BIT(0)
> +#define DP83822_WOL_PKT_INT_EN BIT(1)
> +#define DP83822_SLEEP_MODE_INT_EN BIT(2)
> +#define DP83822_MDI_XOVER_INT_EN BIT(3)
> +#define DP83822_LB_FIFO_INT_EN BIT(4)
> +#define DP83822_PAGE_RX_INT_EN BIT(5)
> +#define DP83822_ANEG_ERR_INT_EN BIT(6)
> +#define DP83822_EEE_ERROR_CHANGE_INT_EN BIT(7)
> +
> +/* INT_STAT1 bits */
> +#define DP83822_WOL_INT_EN BIT(4)
> +#define DP83822_WOL_INT_STAT BIT(12)
> +
> +#define MII_DP83822_RXSOP1 0x04a5
> +#define MII_DP83822_RXSOP2 0x04a6
> +#define MII_DP83822_RXSOP3 0x04a7
> +
> +/* WoL Registers */
> +#define MII_DP83822_WOL_CFG 0x04a0
> +#define MII_DP83822_WOL_STAT 0x04a1
> +#define MII_DP83822_WOL_DA1 0x04a2
> +#define MII_DP83822_WOL_DA2 0x04a3
> +#define MII_DP83822_WOL_DA3 0x04a4
> +
> +/* WoL bits */
> +#define DP83822_WOL_MAGIC_EN BIT(0)
> +#define DP83822_WOL_SECURE_ON BIT(5)
> +#define DP83822_WOL_EN BIT(7)
> +#define DP83822_WOL_INDICATION_SEL BIT(8)
> +#define DP83822_WOL_CLR_INDICATION BIT(11)
> +
> +static int dp83822_ack_interrupt(struct phy_device *phydev)
> +{
> + int err;
> +
> + err = phy_read(phydev, MII_DP83822_MISR1);
> + if (err < 0)
> + return err;
> +
> + err = phy_read(phydev, MII_DP83822_MISR2);
> + if (err < 0)
> + return err;
> +
> + return 0;
> +}
> +
> +static int dp83822_set_wol(struct phy_device *phydev,
> + struct ethtool_wolinfo *wol)
> +{
> + struct net_device *ndev = phydev->attached_dev;
> + u16 value;
> + const u8 *mac;
> +
> + if (wol->wolopts & (WAKE_MAGIC | WAKE_MAGICSECURE)) {
> + mac = (const u8 *)ndev->dev_addr;
> +
> + if (!is_valid_ether_addr(mac))
> + return -EINVAL;
> +
> + /* MAC addresses start with byte 5, but stored in mac[0].
> + * 822 PHYs store bytes 4|5, 2|3, 0|1
> + */
> + phy_write_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_DA1,
> + (mac[1] << 8) | mac[0]);
> + phy_write_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_DA2,
> + (mac[3] << 8) | mac[2]);
> + phy_write_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_DA3,
> + (mac[5] << 8) | mac[4]);
> +
> + value = phy_read_mmd(phydev, DP83822_DEVADDR,
> + MII_DP83822_WOL_CFG);
> + if (wol->wolopts & WAKE_MAGIC)
> + value |= DP83822_WOL_MAGIC_EN;
> + else
> + value &= ~DP83822_WOL_MAGIC_EN;
> +
> + if (wol->wolopts & WAKE_MAGICSECURE) {
> + phy_write_mmd(phydev, DP83822_DEVADDR,
> + MII_DP83822_RXSOP1,
> + (wol->sopass[1] << 8) | wol->sopass[0]);
> + phy_write_mmd(phydev, DP83822_DEVADDR,
> + MII_DP83822_RXSOP2,
> + (wol->sopass[3] << 8) | wol->sopass[2]);
> + phy_write_mmd(phydev, DP83822_DEVADDR,
> + MII_DP83822_RXSOP3,
> + (wol->sopass[5] << 8) | wol->sopass[4]);
> + value |= DP83822_WOL_SECURE_ON;
> + } else {
> + value &= ~DP83822_WOL_SECURE_ON;
> + }
> +
> + value |= (DP83822_WOL_EN | DP83822_WOL_INDICATION_SEL |
> + DP83822_WOL_CLR_INDICATION);
> + phy_write_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_CFG,
> + value);
> + } else {
> + value = phy_read_mmd(phydev, DP83822_DEVADDR,
> + MII_DP83822_WOL_CFG);
> + value &= ~DP83822_WOL_EN;
> + phy_write_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_CFG,
> + value);
> + }
> +
> + return 0;
> +}
> +
> +static void dp83822_get_wol(struct phy_device *phydev,
> + struct ethtool_wolinfo *wol)
> +{
> + int value;
> + u16 sopass_val;
> +
> + wol->supported = (WAKE_MAGIC | WAKE_MAGICSECURE);
> + wol->wolopts = 0;
> +
> + value = phy_read_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_CFG);
> +
> + if (value & DP83822_WOL_MAGIC_EN)
> + wol->wolopts |= WAKE_MAGIC;
> +
> + if (value & DP83822_WOL_SECURE_ON) {
> + sopass_val = phy_read_mmd(phydev, DP83822_DEVADDR,
> + MII_DP83822_RXSOP1);
> + wol->sopass[0] = (sopass_val & 0xff);
> + wol->sopass[1] = (sopass_val >> 8);
> +
> + sopass_val = phy_read_mmd(phydev, DP83822_DEVADDR,
> + MII_DP83822_RXSOP2);
> + wol->sopass[2] = (sopass_val & 0xff);
> + wol->sopass[3] = (sopass_val >> 8);
> +
> + sopass_val = phy_read_mmd(phydev, DP83822_DEVADDR,
> + MII_DP83822_RXSOP3);
> + wol->sopass[4] = (sopass_val & 0xff);
> + wol->sopass[5] = (sopass_val >> 8);
> +
> + wol->wolopts |= WAKE_MAGICSECURE;
> + }
> +
> + /* WoL is not enabled so set wolopts to 0 */
> + if (!(value & DP83822_WOL_EN))
> + wol->wolopts = 0;
> +}
> +
> +static int dp83822_config_intr(struct phy_device *phydev)
> +{
> + int misr_status;
> + int physcr_status;
> + int err;
> +
> + if (phydev->interrupts == PHY_INTERRUPT_ENABLED) {
> + misr_status = phy_read(phydev, MII_DP83822_MISR1);
> + if (misr_status < 0)
> + return misr_status;
> +
> + misr_status |= (DP83822_RX_ERR_HF_INT_EN |
> + DP83822_FALSE_CARRIER_HF_INT_EN |
> + DP83822_ANEG_COMPLETE_INT_EN |
> + DP83822_DUP_MODE_CHANGE_INT_EN |
> + DP83822_SPEED_CHANGED_INT_EN |
> + DP83822_LINK_STAT_INT_EN |
> + DP83822_ENERGY_DET_INT_EN |
> + DP83822_LINK_QUAL_INT_EN);
> +
> + err = phy_write(phydev, MII_DP83822_MISR1, misr_status);
> + if (err < 0)
> + return err;
> +
> + misr_status = phy_read(phydev, MII_DP83822_MISR2);
> + if (misr_status < 0)
> + return misr_status;
> +
> + misr_status |= (DP83822_JABBER_DET_INT_EN |
> + DP83822_WOL_PKT_INT_EN |
> + DP83822_SLEEP_MODE_INT_EN |
> + DP83822_MDI_XOVER_INT_EN |
> + DP83822_LB_FIFO_INT_EN |
> + DP83822_PAGE_RX_INT_EN |
> + DP83822_ANEG_ERR_INT_EN |
> + DP83822_EEE_ERROR_CHANGE_INT_EN);
> +
> + err = phy_write(phydev, MII_DP83822_MISR2, misr_status);
> + if (err < 0)
> + return err;
> +
> + physcr_status = phy_read(phydev, MII_DP83822_PHYSCR);
> + if (physcr_status < 0)
> + return physcr_status;
> +
> + physcr_status |= DP83822_PHYSCR_INT_OE | DP83822_PHYSCR_INTEN;
> +
> + } else {
> + err = phy_write(phydev, MII_DP83822_MISR1, 0);
> + if (err < 0)
> + return err;
> +
> + err = phy_write(phydev, MII_DP83822_MISR1, 0);
> + if (err < 0)
> + return err;
> +
> + physcr_status = phy_read(phydev, MII_DP83822_PHYSCR);
> + if (physcr_status < 0)
> + return physcr_status;
> +
> + physcr_status &= ~DP83822_PHYSCR_INTEN;
> + }
> +
> + return phy_write(phydev, MII_DP83822_PHYSCR, physcr_status);
> +}
> +
> +static int dp83822_config_init(struct phy_device *phydev)
> +{
> + int err;
> + int value;
> +
> + err = genphy_config_init(phydev);
> + if (err < 0)
> + return err;
> +
> + value = DP83822_WOL_MAGIC_EN | DP83822_WOL_SECURE_ON | DP83822_WOL_EN;
> +
> + return phy_write_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_CFG,
> + value);
> +}
> +
> +static int dp83822_phy_reset(struct phy_device *phydev)
> +{
> + int err;
> +
> + err = phy_write(phydev, MII_DP83822_RESET_CTRL, DP83822_HW_RESET);
> + if (err < 0)
> + return err;
> +
> + dp83822_config_init(phydev);
> +
> + return 0;
> +}
> +
> +static int dp83822_suspend(struct phy_device *phydev)
> +{
> + int value;
> +
> + value = phy_read_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_CFG);
> +
> + if (!(value & DP83822_WOL_EN))
> + genphy_suspend(phydev);
> +
> + return 0;
> +}
> +
> +static int dp83822_resume(struct phy_device *phydev)
> +{
> + int value;
> +
> + genphy_resume(phydev);
> +
> + value = phy_read_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_CFG);
> +
> + phy_write_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_CFG, value |
> + DP83822_WOL_CLR_INDICATION);
> +
> + return 0;
> +}
> +
> +static struct phy_driver dp83822_driver[] = {
> + {
> + .phy_id = DP83822_PHY_ID,
> + .phy_id_mask = 0xfffffff0,
> + .name = "TI DP83822",
> + .features = PHY_BASIC_FEATURES,
> + .flags = PHY_HAS_INTERRUPT,
> + .config_init = dp83822_config_init,
> + .soft_reset = dp83822_phy_reset,
> + .get_wol = dp83822_get_wol,
> + .set_wol = dp83822_set_wol,
> + .ack_interrupt = dp83822_ack_interrupt,
> + .config_intr = dp83822_config_intr,
> + .config_aneg = genphy_config_aneg,
> + .read_status = genphy_read_status,
> + .suspend = dp83822_suspend,
> + .resume = dp83822_resume,
> + },
> +};
> +module_phy_driver(dp83822_driver);
> +
> +static struct mdio_device_id __maybe_unused dp83822_tbl[] = {
> + { DP83822_PHY_ID, 0xfffffff0 },
> + { },
> +};
> +MODULE_DEVICE_TABLE(mdio, dp83822_tbl);
> +
> +MODULE_DESCRIPTION("Texas Instruments DP83822 PHY driver");
> +MODULE_AUTHOR("Dan Murphy <dmurphy@ti.com");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/net/phy/dp83848.c b/drivers/net/phy/dp83848.c
> index 3de4fe4dda77..3966d43c5146 100644
> --- a/drivers/net/phy/dp83848.c
> +++ b/drivers/net/phy/dp83848.c
> @@ -20,7 +20,6 @@
> #define TI_DP83620_PHY_ID 0x20005ce0
> #define NS_DP83848C_PHY_ID 0x20005c90
> #define TLK10X_PHY_ID 0x2000a210
> -#define TI_DP83822_PHY_ID 0x2000a240
>
> /* Registers */
> #define DP83848_MICR 0x11 /* MII Interrupt Control Register */
> @@ -80,7 +79,6 @@ static struct mdio_device_id __maybe_unused dp83848_tbl[] = {
> { NS_DP83848C_PHY_ID, 0xfffffff0 },
> { TI_DP83620_PHY_ID, 0xfffffff0 },
> { TLK10X_PHY_ID, 0xfffffff0 },
> - { TI_DP83822_PHY_ID, 0xfffffff0 },
> { }
> };
> MODULE_DEVICE_TABLE(mdio, dp83848_tbl);
> @@ -110,7 +108,6 @@ static struct phy_driver dp83848_driver[] = {
> DP83848_PHY_DRIVER(NS_DP83848C_PHY_ID, "NS DP83848C 10/100 Mbps PHY"),
> DP83848_PHY_DRIVER(TI_DP83620_PHY_ID, "TI DP83620 10/100 Mbps PHY"),
> DP83848_PHY_DRIVER(TLK10X_PHY_ID, "TI TLK10X 10/100 Mbps PHY"),
> - DP83848_PHY_DRIVER(TI_DP83822_PHY_ID, "TI DP83822 10/100 Mbps PHY"),
> };
> module_phy_driver(dp83848_driver);
>
>
^ permalink raw reply
* Re: [PATCH v5 1/2] net: phy: DP83822 initial driver submission
From: Florian Fainelli @ 2017-10-10 17:50 UTC (permalink / raw)
To: Dan Murphy, andrew; +Cc: netdev, Woojung.Huh, afd
In-Reply-To: <20171010174256.21930-1-dmurphy@ti.com>
On 10/10/2017 10:42 AM, Dan Murphy wrote:
> Add support for the TI DP83822 10/100Mbit ethernet phy.
>
> The DP83822 provides flexibility to connect to a MAC through a
> standard MII, RMII or RGMII interface.
>
> In addition the DP83822 needs to be removed from the DP83848 driver
> as the WoL support is added here for this device.
>
> Datasheet:
> http://www.ti.com/product/DP83822I/datasheet
>
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Thanks Dan!
--
Florian
^ permalink raw reply
* Re: [PATCH][net-next] ipv6: fix dereference of rt6_ex before null check error
From: David Miller @ 2017-10-10 17:54 UTC (permalink / raw)
To: colin.king; +Cc: kuznet, yoshfuji, netdev, kernel-janitors, linux-kernel
In-Reply-To: <20171010170116.21124-1-colin.king@canonical.com>
From: Colin King <colin.king@canonical.com>
Date: Tue, 10 Oct 2017 18:01:16 +0100
> From: Colin Ian King <colin.king@canonical.com>
>
> Currently rt6_ex is being dereferenced before it is null checked
> hence there is a possible null dereference bug. Fix this by only
> dereferencing rt6_ex after it has been null checked.
>
> Detected by CoverityScan, CID#1457749 ("Dereference before null check")
>
> Fixes: 81eb8447daae ("ipv6: take care of rt6_stats")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Applied, thanks Colin.
^ permalink raw reply
* Re: [PATCH net-next v2 4/5] selinux: bpf: Add selinux check for eBPF syscall operations
From: Chenbo Feng @ 2017-10-10 17:54 UTC (permalink / raw)
To: Stephen Smalley
Cc: Chenbo Feng, linux-security-module, netdev, SELinux,
Alexei Starovoitov, Daniel Borkmann, Lorenzo Colitti
In-Reply-To: <1507647148.30616.14.camel@tycho.nsa.gov>
On Tue, Oct 10, 2017 at 7:52 AM, Stephen Smalley <sds@tycho.nsa.gov> wrote:
> On Tue, 2017-10-10 at 10:18 -0400, Stephen Smalley wrote:
>> On Mon, 2017-10-09 at 15:20 -0700, Chenbo Feng wrote:
>> > From: Chenbo Feng <fengc@google.com>
>> >
>> > Implement the actual checks introduced to eBPF related syscalls.
>> > This
>> > implementation use the security field inside bpf object to store a
>> > sid that
>> > identify the bpf object. And when processes try to access the
>> > object,
>> > selinux will check if processes have the right privileges. The
>> > creation
>> > of eBPF object are also checked at the general bpf check hook and
>> > new
>> > cmd introduced to eBPF domain can also be checked there.
>> >
>> > Signed-off-by: Chenbo Feng <fengc@google.com>
>> > Acked-by: Alexei Starovoitov <ast@kernel.org>
>> > ---
>> > security/selinux/hooks.c | 111
>> > ++++++++++++++++++++++++++++++++++++
>> > security/selinux/include/classmap.h | 2 +
>> > security/selinux/include/objsec.h | 4 ++
>> > 3 files changed, 117 insertions(+)
>> >
>> > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
>> > index f5d304736852..41aba4e3d57c 100644
>> > --- a/security/selinux/hooks.c
>> > +++ b/security/selinux/hooks.c
>> > @@ -85,6 +85,7 @@
>> > #include <linux/export.h>
>> > #include <linux/msg.h>
>> > #include <linux/shm.h>
>> > +#include <linux/bpf.h>
>> >
>> > #include "avc.h"
>> > #include "objsec.h"
>> > @@ -6252,6 +6253,106 @@ static void selinux_ib_free_security(void
>> > *ib_sec)
>> > }
>> > #endif
>> >
>> > +#ifdef CONFIG_BPF_SYSCALL
>> > +static int selinux_bpf(int cmd, union bpf_attr *attr,
>> > + unsigned int size)
>> > +{
>> > + u32 sid = current_sid();
>> > + int ret;
>> > +
>> > + switch (cmd) {
>> > + case BPF_MAP_CREATE:
>> > + ret = avc_has_perm(sid, sid, SECCLASS_BPF_MAP,
>> > BPF_MAP__CREATE,
>> > + NULL);
>> > + break;
>> > + case BPF_PROG_LOAD:
>> > + ret = avc_has_perm(sid, sid, SECCLASS_BPF_PROG,
>> > BPF_PROG__LOAD,
>> > + NULL);
>> > + break;
>> > + default:
>> > + ret = 0;
>> > + break;
>> > + }
>> > +
>> > + return ret;
>> > +}
>> > +
>> > +static u32 bpf_map_fmode_to_av(fmode_t fmode)
>> > +{
>> > + u32 av = 0;
>> > +
>> > + if (f_mode & FMODE_READ)
>> > + av |= BPF_MAP__READ;
>> > + if (f_mode & FMODE_WRITE)
>> > + av |= BPF_MAP__WRITE;
>> > + return av;
>> > +}
>> > +
>> > +static int selinux_bpf_map(struct bpf_map *map, fmode_t fmode)
>> > +{
>> > + u32 sid = current_sid();
>> > + struct bpf_security_struct *bpfsec;
>> > +
>> > + bpfsec = map->security;
>> > + return avc_has_perm(sid, bpfsec->sid, SECCLASS_BPF_MAP,
>> > + bpf_map_fmode_to_av(fmode), NULL);
>> > +}
>> > +
>> > +static int selinux_bpf_prog(struct bpf_prog *prog)
>> > +{
>> > + u32 sid = current_sid();
>> > + struct bpf_security_struct *bpfsec;
>> > +
>> > + bpfsec = prog->aux->security;
>> > + return avc_has_perm(sid, bpfsec->sid, SECCLASS_BPF_PROG,
>> > + BPF_PROG__USE, NULL);
>> > +}
>> > +
>> > +static int selinux_bpf_map_alloc(struct bpf_map *map)
>> > +{
>> > + struct bpf_security_struct *bpfsec;
>> > +
>> > + bpfsec = kzalloc(sizeof(*bpfsec), GFP_KERNEL);
>> > + if (!bpfsec)
>> > + return -ENOMEM;
>> > +
>> > + bpfsec->sid = current_sid();
>> > + map->security = bpfsec;
>> > +
>> > + return 0;
>> > +}
>> > +
>> > +static void selinux_bpf_map_free(struct bpf_map *map)
>> > +{
>> > + struct bpf_security_struct *bpfsec = map->security;
>> > +
>> > + map->security = NULL;
>> > + kfree(bpfsec);
>> > +}
>> > +
>> > +static int selinux_bpf_prog_alloc(struct bpf_prog_aux *aux)
>> > +{
>> > + struct bpf_security_struct *bpfsec;
>> > +
>> > + bpfsec = kzalloc(sizeof(*bpfsec), GFP_KERNEL);
>> > + if (!bpfsec)
>> > + return -ENOMEM;
>> > +
>> > + bpfsec->sid = current_sid();
>> > + aux->security = bpfsec;
>> > +
>> > + return 0;
>> > +}
>> > +
>> > +static void selinux_bpf_prog_free(struct bpf_prog_aux *aux)
>> > +{
>> > + struct bpf_security_struct *bpfsec = aux->security;
>> > +
>> > + aux->security = NULL;
>> > + kfree(bpfsec);
>> > +}
>> > +#endif
>> > +
>> > static struct security_hook_list selinux_hooks[]
>> > __lsm_ro_after_init
>> > = {
>> > LSM_HOOK_INIT(binder_set_context_mgr,
>> > selinux_binder_set_context_mgr),
>> > LSM_HOOK_INIT(binder_transaction,
>> > selinux_binder_transaction),
>> > @@ -6471,6 +6572,16 @@ static struct security_hook_list
>> > selinux_hooks[] __lsm_ro_after_init = {
>> > LSM_HOOK_INIT(audit_rule_match, selinux_audit_rule_match),
>> > LSM_HOOK_INIT(audit_rule_free, selinux_audit_rule_free),
>> > #endif
>> > +
>> > +#ifdef CONFIG_BPF_SYSCALL
>> > + LSM_HOOK_INIT(bpf, selinux_bpf),
>> > + LSM_HOOK_INIT(bpf_map, selinux_bpf_map),
>> > + LSM_HOOK_INIT(bpf_prog, selinux_bpf_prog),
>> > + LSM_HOOK_INIT(bpf_map_alloc_security,
>> > selinux_bpf_map_alloc),
>> > + LSM_HOOK_INIT(bpf_prog_alloc_security,
>> > selinux_bpf_prog_alloc),
>> > + LSM_HOOK_INIT(bpf_map_free_security,
>> > selinux_bpf_map_free),
>> > + LSM_HOOK_INIT(bpf_prog_free_security,
>> > selinux_bpf_prog_free),
>> > +#endif
>> > };
>> >
>> > static __init int selinux_init(void)
>> > diff --git a/security/selinux/include/classmap.h
>> > b/security/selinux/include/classmap.h
>> > index 35ffb29a69cb..7253c5eea59c 100644
>> > --- a/security/selinux/include/classmap.h
>> > +++ b/security/selinux/include/classmap.h
>> > @@ -237,6 +237,8 @@ struct security_class_mapping secclass_map[] =
>> > {
>> > { "access", NULL } },
>> > { "infiniband_endport",
>> > { "manage_subnet", NULL } },
>> > + { "bpf_map", {"create", "read", "write"} },
>> > + { "bpf_prog", {"load", "use"} },
>>
>> Again I have to ask: do you truly need/want two separate classes, or
>> would a single class with distinct permissions suffice, ala:
>> { "bpf", { "create_map", "read_map", "write_map",
>> "prog_load",
>> "prog_use" } },
>>
>> and then allow A self:bpf { create_map read_map write_map prog_load
>> prog_use }; would be stored in a single policy avtab rule, and be
>> cached in a single AVC entry.
>
Sorry I missed to reply on this, I keep it that way because sometimes
we need to grant the permission of accessing eBPF maps and programs
separately. But keep them in a single class definitely works for me.
> I guess for consistency in naming it should be either:
> { "bpf", { "map_create", "map_read", "map_write", "prog_load",
> "prog_use" } },
>
> or:
>
> { "bpf", { "create_map", "read_map", "write_map", "load_prog",
> "use_prog" } },
>
>
>> > { NULL }
>> > };
>> >
>> > diff --git a/security/selinux/include/objsec.h
>> > b/security/selinux/include/objsec.h
>> > index 1649cd18eb0b..3d54468ce334 100644
>> > --- a/security/selinux/include/objsec.h
>> > +++ b/security/selinux/include/objsec.h
>> > @@ -150,6 +150,10 @@ struct pkey_security_struct {
>> > u32 sid; /* SID of pkey */
>> > };
>> >
>> > +struct bpf_security_struct {
>> > + u32 sid; /*SID of bpf obj creater*/
>> > +};
>> > +
>> > extern unsigned int selinux_checkreqprot;
>> >
>> > #endif /* _SELINUX_OBJSEC_H_ */
^ permalink raw reply
* [PATCH][net-next] ipv6: fix incorrect bitwise operator used on rt6i_flags
From: Colin King @ 2017-10-10 17:55 UTC (permalink / raw)
To: David S . Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, netdev
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
The use of the | operator always leads to true on the expression
(rt->rt6i_flags | RTF_CACHE) which looks rather suspect to me. I
believe this is fixed by using & instead to just check the
RTF_CACHE entry bit.
Detected by CoverityScan, CID#1457747 ("Wrong operator used")
Fixes: 35732d01fe31 ("ipv6: introduce a hash table to store dst cache")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
net/ipv6/route.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 6db1541eaa7b..0556d1ee189c 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -1425,7 +1425,7 @@ int rt6_remove_exception_rt(struct rt6_info *rt)
int err;
if (!from ||
- !(rt->rt6i_flags | RTF_CACHE))
+ !(rt->rt6i_flags & RTF_CACHE))
return -EINVAL;
if (!rcu_access_pointer(from->rt6i_exception_bucket))
--
2.14.1
^ permalink raw reply related
* Re: [PATCH net-next v2 4/5] selinux: bpf: Add selinux check for eBPF syscall operations
From: kbuild test robot @ 2017-10-10 17:59 UTC (permalink / raw)
To: Chenbo Feng
Cc: kbuild-all, linux-security-module, netdev, SELinux,
Jeffrey Vander Stoep, Alexei Starovoitov, lorenzo,
Daniel Borkmann, Stephen Smalley, Chenbo Feng
In-Reply-To: <20171009222028.13096-5-chenbofeng.kernel@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1370 bytes --]
Hi Chenbo,
[auto build test ERROR on net-next/master]
url: https://github.com/0day-ci/linux/commits/Chenbo-Feng/bpf-security-New-file-mode-and-LSM-hooks-for-eBPF-object-permission-control/20171011-010349
config: xtensa-allyesconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 4.9.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=xtensa
All errors (new ones prefixed by >>):
security//selinux/hooks.c: In function 'bpf_map_fmode_to_av':
>> security//selinux/hooks.c:6284:6: error: 'f_mode' undeclared (first use in this function)
if (f_mode & FMODE_READ)
^
security//selinux/hooks.c:6284:6: note: each undeclared identifier is reported only once for each function it appears in
vim +/f_mode +6284 security//selinux/hooks.c
6279
6280 static u32 bpf_map_fmode_to_av(fmode_t fmode)
6281 {
6282 u32 av = 0;
6283
> 6284 if (f_mode & FMODE_READ)
6285 av |= BPF_MAP__READ;
6286 if (f_mode & FMODE_WRITE)
6287 av |= BPF_MAP__WRITE;
6288 return av;
6289 }
6290
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 51658 bytes --]
^ permalink raw reply
* NACK: [PATCH][net-next] ipv6: fix incorrect bitwise operator used on rt6i_flags
From: Colin Ian King @ 2017-10-10 18:05 UTC (permalink / raw)
To: David S . Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, netdev
Cc: kernel-janitors, linux-kernel
In-Reply-To: <20171010175527.21982-1-colin.king@canonical.com>
On 10/10/17 18:55, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> The use of the | operator always leads to true on the expression
> (rt->rt6i_flags | RTF_CACHE) which looks rather suspect to me. I
> believe this is fixed by using & instead to just check the
> RTF_CACHE entry bit.
>
> Detected by CoverityScan, CID#1457747 ("Wrong operator used")
>
> Fixes: 35732d01fe31 ("ipv6: introduce a hash table to store dst cache")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
> net/ipv6/route.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index 6db1541eaa7b..0556d1ee189c 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -1425,7 +1425,7 @@ int rt6_remove_exception_rt(struct rt6_info *rt)
> int err;
>
> if (!from ||
> - !(rt->rt6i_flags | RTF_CACHE))
> + !(rt->rt6i_flags & RTF_CACHE))
> return -EINVAL;
>
> if (!rcu_access_pointer(from->rt6i_exception_bucket))
>
Nack that, seems like this occurs more than once and I failed to spot
the others.
^ permalink raw reply
* Re: [RFC net 1/1] net: sched: act: fix rcu race in dump
From: Alexander Aring @ 2017-10-10 18:09 UTC (permalink / raw)
To: Eric Dumazet
Cc: Jamal Hadi Salim, Cong Wang, Jiří Pírko, netdev,
Manish Kurup, Brenda Butler
In-Reply-To: <1507644745.31614.17.camel@edumazet-glaptop3.roam.corp.google.com>
Hi,
On Tue, Oct 10, 2017 at 10:12 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Tue, 2017-10-10 at 08:32 -0400, Alexander Aring wrote:
>> This patch fixes an issue with kfree_rcu which is not protected by RTNL
>> lock. It could be that the current assigned rcu pointer will be freed by
>> kfree_rcu while dump callback is running.
>>
>> To prevent this, we call rcu_synchronize at first. Then we are sure all
>> latest rcu functions e.g. rcu_assign_pointer and kfree_rcu in init are
>> done. After rcu_synchronize we dereference under RTNL lock which is also
>> held in init function, which means no other rcu_assign_pointer or
>> kfree_rcu will occur.
>>
>> To call rcu_synchronize will also prevent weird behaviours by doing over
>> netlink:
>>
>> - set params A
>> - set params B
>> - dump params
>> \--> will dump params A
>>
>> This could be a unlikely case that the last rcu_assign_pointer was not
>> happened before dump callback.
>>
>> Signed-off-by: Alexander Aring <aring@mojatatu.com>
>> ---
>> net/sched/act_skbmod.c | 7 ++++++-
>> 1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/net/sched/act_skbmod.c b/net/sched/act_skbmod.c
>> index b642ad3d39dd..231e07bca384 100644
>> --- a/net/sched/act_skbmod.c
>> +++ b/net/sched/act_skbmod.c
>> @@ -198,7 +198,7 @@ static int tcf_skbmod_dump(struct sk_buff *skb, struct tc_action *a,
>> {
>> struct tcf_skbmod *d = to_skbmod(a);
>> unsigned char *b = skb_tail_pointer(skb);
>> - struct tcf_skbmod_params *p = rtnl_dereference(d->skbmod_p);
>> + struct tcf_skbmod_params *p;
>> struct tc_skbmod opt = {
>> .index = d->tcf_index,
>> .refcnt = d->tcf_refcnt - ref,
>> @@ -207,6 +207,11 @@ static int tcf_skbmod_dump(struct sk_buff *skb, struct tc_action *a,
>> };
>> struct tcf_t t;
>>
>> + /* wait until last rcu_assign_pointer/kfree_rcu is done */
>> + rcu_synchronize();
>> + /* RTNL lock prevents another rcu_assign_pointer/kfree_rcu call */
>> + p = rtnl_dereference(d->skbmod_p);
>> +
>> opt.flags = p->flags;
>> if (nla_put(skb, TCA_SKBMOD_PARMS, sizeof(opt), &opt))
>> goto nla_put_failure;
>
> Sorry but no. This is plainly wrong.
>
> We need to fix this without adding a _very_ expensive rcu_synchronize()
> on a path which does not need such thing.
>
I agree that a rcu synchronize is very expensive while holding RTNL.
Should be handled with rcu_read_lock as you suggested below, but this
will not prevent to show an user space behavior like:
- set_params(A)
- set_params(B)
\---> dump - will dump values A
Because the rcu_read_lock will avoid rcu_assign_pointer to update the
pointer and not wait that the rcu_assign_pointer of set_params(B) is
done before calling dump.
Okay, this issue is maybe something we should not care about it so far
it's not an use after free issue.
> I am confused by this patch, please tell us more what the problem is.
>
The callback "init" is also called by updating parameters for an action.
It use rcu_assign_pointer [0], as well kfree_rcu [1] to swap the
pointers of parameter structures and free the old resource.
This is well protected by rcu_read_lock inside the "run" callback of
tc action, which runs in softirq context. But dump is only protected
by RTNL so far I see.
Sorry when I understood RCU wrong, but so far I understood RCU
handling, it _could_ be that returning of "init" the pointers are not
updated yet. After a "grace" period, which rcu synchronize waits for
it - we can be sure that it's assigned and kfree_rcu completes.
The problem is:
If the deference of parameters inside dump callback using still the
old structure (for my understanding, it can happened because this
callback do nothing against it to protect it) kfree_rcu can free the
resource during accessing this structure. A RCU read lock will of
course preventing RCU to update the pointers in this time (but not
RTNL, so far I understood).
> I suspect rcu_read_lock() is what you need, but isn't a writer supposed
> to hold RTNL in net/sched/* ???
>
Yes a writer holds RTNL, but these writers using RCU to write (as
shown in [0] and [1]). So far I know kfree_rcu: it can occur that
"init" returns and dump is called afterwards - during the dump RCU can
run and free/assign pointers in this time (while dump still holds
references). So far I understand a RTNL lock will not prevent RCU to
do that.
I wrote this mail also to get an answer if there exists a problem or
not. If you say me, the resource cannot be freed by kfree_rcu if RTNL
lock is hold, then I know more about how RCU is working now.
- Alex
[0] http://elixir.free-electrons.com/linux/latest/source/net/sched/act_skbmod.c#L177
[1] http://elixir.free-electrons.com/linux/latest/source/net/sched/act_skbmod.c#L182
^ permalink raw reply
* [PATCH][V2] ipv6: fix incorrect bitwise operator used on rt6i_flags
From: Colin King @ 2017-10-10 18:10 UTC (permalink / raw)
To: David S . Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, netdev
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
The use of the | operator always leads to true which looks rather
suspect to me. Fix this by using & instead to just check the
RTF_CACHE entry bit.
Detected by CoverityScan, CID#1457734, #1457747 ("Wrong operator used")
Fixes: 35732d01fe31 ("ipv6: introduce a hash table to store dst cache")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
net/ipv6/route.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 6db1541eaa7b..dd9ba1192dbc 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -1425,7 +1425,7 @@ int rt6_remove_exception_rt(struct rt6_info *rt)
int err;
if (!from ||
- !(rt->rt6i_flags | RTF_CACHE))
+ !(rt->rt6i_flags & RTF_CACHE))
return -EINVAL;
if (!rcu_access_pointer(from->rt6i_exception_bucket))
@@ -1469,7 +1469,7 @@ static void rt6_update_exception_stamp_rt(struct rt6_info *rt)
struct rt6_exception *rt6_ex;
if (!from ||
- !(rt->rt6i_flags | RTF_CACHE))
+ !(rt->rt6i_flags & RTF_CACHE))
return;
rcu_read_lock();
--
2.14.1
^ permalink raw reply related
* Re: [PATCH][net-next] ipv6: fix incorrect bitwise operator used on rt6i_flags
From: Martin KaFai Lau @ 2017-10-10 18:10 UTC (permalink / raw)
To: Colin King
Cc: David S . Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, netdev,
kernel-janitors, linux-kernel, Wei Wang
In-Reply-To: <20171010175527.21982-1-colin.king@canonical.com>
On Tue, Oct 10, 2017 at 05:55:27PM +0000, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> The use of the | operator always leads to true on the expression
> (rt->rt6i_flags | RTF_CACHE) which looks rather suspect to me. I
> believe this is fixed by using & instead to just check the
> RTF_CACHE entry bit.
Good catch. LGTM. If rt does not have RTF_CACHE set, it should not be in the
exception table.
Acked-by: Martin KaFai Lau <kafai@fb.com>
>
> Detected by CoverityScan, CID#1457747 ("Wrong operator used")
>
> Fixes: 35732d01fe31 ("ipv6: introduce a hash table to store dst cache")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
> net/ipv6/route.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index 6db1541eaa7b..0556d1ee189c 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -1425,7 +1425,7 @@ int rt6_remove_exception_rt(struct rt6_info *rt)
> int err;
>
> if (!from ||
> - !(rt->rt6i_flags | RTF_CACHE))
> + !(rt->rt6i_flags & RTF_CACHE))
> return -EINVAL;
>
> if (!rcu_access_pointer(from->rt6i_exception_bucket))
> --
> 2.14.1
>
^ permalink raw reply
* Re: [PATCH][net-next] ipv6: fix incorrect bitwise operator used on rt6i_flags
From: Wei Wang @ 2017-10-10 18:23 UTC (permalink / raw)
To: Martin KaFai Lau, Colin King, Eric Dumazet
Cc: David S . Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI,
Linux Kernel Network Developers, kernel-janitors, lkml
In-Reply-To: <20171010181054.4snwnyc3q7rf2wbl@kafai-mbp.dhcp.thefacebook.com>
On Tue, Oct 10, 2017 at 11:10 AM, Martin KaFai Lau <kafai@fb.com> wrote:
> On Tue, Oct 10, 2017 at 05:55:27PM +0000, Colin King wrote:
>> From: Colin Ian King <colin.king@canonical.com>
>>
>> The use of the | operator always leads to true on the expression
>> (rt->rt6i_flags | RTF_CACHE) which looks rather suspect to me. I
>> believe this is fixed by using & instead to just check the
>> RTF_CACHE entry bit.
> Good catch. LGTM. If rt does not have RTF_CACHE set, it should not be in the
> exception table.
>
> Acked-by: Martin KaFai Lau <kafai@fb.com>
>
Thanks a lot for catching this. Yes. It should have been '&' instead of '|'.
Acked-by: Wei Wang <weiwan@google.com>
>>
>> Detected by CoverityScan, CID#1457747 ("Wrong operator used")
>>
>> Fixes: 35732d01fe31 ("ipv6: introduce a hash table to store dst cache")
>> Signed-off-by: Colin Ian King <colin.king@canonical.com>
>> ---
>> net/ipv6/route.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
>> index 6db1541eaa7b..0556d1ee189c 100644
>> --- a/net/ipv6/route.c
>> +++ b/net/ipv6/route.c
>> @@ -1425,7 +1425,7 @@ int rt6_remove_exception_rt(struct rt6_info *rt)
>> int err;
>>
>> if (!from ||
>> - !(rt->rt6i_flags | RTF_CACHE))
>> + !(rt->rt6i_flags & RTF_CACHE))
>> return -EINVAL;
>>
>> if (!rcu_access_pointer(from->rt6i_exception_bucket))
>> --
>> 2.14.1
>>
^ permalink raw reply
* Re: [PATCH][net-next] ipv6: fix incorrect bitwise operator used on rt6i_flags
From: Colin Ian King @ 2017-10-10 18:24 UTC (permalink / raw)
To: Wei Wang, Martin KaFai Lau, Eric Dumazet
Cc: David S . Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI,
Linux Kernel Network Developers, kernel-janitors, lkml
In-Reply-To: <CAEA6p_BDQ_RaF7-9eBnhstMY_w-i91cJECXuvSHntkdDQo-YUw@mail.gmail.com>
On 10/10/17 19:23, Wei Wang wrote:
> On Tue, Oct 10, 2017 at 11:10 AM, Martin KaFai Lau <kafai@fb.com> wrote:
>> On Tue, Oct 10, 2017 at 05:55:27PM +0000, Colin King wrote:
>>> From: Colin Ian King <colin.king@canonical.com>
>>>
>>> The use of the | operator always leads to true on the expression
>>> (rt->rt6i_flags | RTF_CACHE) which looks rather suspect to me. I
>>> believe this is fixed by using & instead to just check the
>>> RTF_CACHE entry bit.
>> Good catch. LGTM. If rt does not have RTF_CACHE set, it should not be in the
>> exception table.
>>
>> Acked-by: Martin KaFai Lau <kafai@fb.com>
>>
>
> Thanks a lot for catching this. Yes. It should have been '&' instead of '|'.
>
> Acked-by: Wei Wang <weiwan@google.com>
Sorry, can you look at V2 of this patch; there is one more occurrence
that needed fixing.
>
>>>
>>> Detected by CoverityScan, CID#1457747 ("Wrong operator used")
>>>
>>> Fixes: 35732d01fe31 ("ipv6: introduce a hash table to store dst cache")
>>> Signed-off-by: Colin Ian King <colin.king@canonical.com>
>>> ---
>>> net/ipv6/route.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
>>> index 6db1541eaa7b..0556d1ee189c 100644
>>> --- a/net/ipv6/route.c
>>> +++ b/net/ipv6/route.c
>>> @@ -1425,7 +1425,7 @@ int rt6_remove_exception_rt(struct rt6_info *rt)
>>> int err;
>>>
>>> if (!from ||
>>> - !(rt->rt6i_flags | RTF_CACHE))
>>> + !(rt->rt6i_flags & RTF_CACHE))
>>> return -EINVAL;
>>>
>>> if (!rcu_access_pointer(from->rt6i_exception_bucket))
>>> --
>>> 2.14.1
>>>
^ permalink raw reply
* Re: [PATCH][V2] ipv6: fix incorrect bitwise operator used on rt6i_flags
From: Wei Wang @ 2017-10-10 18:38 UTC (permalink / raw)
To: Colin King
Cc: David S . Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI,
Linux Kernel Network Developers, kernel-janitors, linux-kernel
In-Reply-To: <20171010181030.22290-1-colin.king@canonical.com>
On Tue, Oct 10, 2017 at 11:10 AM, Colin King <colin.king@canonical.com> wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> The use of the | operator always leads to true which looks rather
> suspect to me. Fix this by using & instead to just check the
> RTF_CACHE entry bit.
>
> Detected by CoverityScan, CID#1457734, #1457747 ("Wrong operator used")
>
> Fixes: 35732d01fe31 ("ipv6: introduce a hash table to store dst cache")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
Acked-by: Wei Wang <weiwan@google.com>
> net/ipv6/route.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index 6db1541eaa7b..dd9ba1192dbc 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -1425,7 +1425,7 @@ int rt6_remove_exception_rt(struct rt6_info *rt)
> int err;
>
> if (!from ||
> - !(rt->rt6i_flags | RTF_CACHE))
> + !(rt->rt6i_flags & RTF_CACHE))
> return -EINVAL;
>
> if (!rcu_access_pointer(from->rt6i_exception_bucket))
> @@ -1469,7 +1469,7 @@ static void rt6_update_exception_stamp_rt(struct rt6_info *rt)
> struct rt6_exception *rt6_ex;
>
> if (!from ||
> - !(rt->rt6i_flags | RTF_CACHE))
> + !(rt->rt6i_flags & RTF_CACHE))
> return;
>
> rcu_read_lock();
> --
> 2.14.1
>
^ permalink raw reply
* RE: [PATCH v5 1/2] net: phy: DP83822 initial driver submission
From: Woojung.Huh @ 2017-10-10 18:56 UTC (permalink / raw)
To: dmurphy, andrew, f.fainelli; +Cc: netdev, afd
In-Reply-To: <20171010174256.21930-1-dmurphy@ti.com>
> +static int dp83822_config_intr(struct phy_device *phydev)
> +{
> + int misr_status;
> + int physcr_status;
> + int err;
> +
> + if (phydev->interrupts == PHY_INTERRUPT_ENABLED) {
> + misr_status = phy_read(phydev, MII_DP83822_MISR1);
> + if (misr_status < 0)
> + return misr_status;
> +
> + misr_status |= (DP83822_RX_ERR_HF_INT_EN |
> + DP83822_FALSE_CARRIER_HF_INT_EN |
> + DP83822_ANEG_COMPLETE_INT_EN |
> + DP83822_DUP_MODE_CHANGE_INT_EN |
> + DP83822_SPEED_CHANGED_INT_EN |
> + DP83822_LINK_STAT_INT_EN |
> + DP83822_ENERGY_DET_INT_EN |
> + DP83822_LINK_QUAL_INT_EN);
> +
> + err = phy_write(phydev, MII_DP83822_MISR1, misr_status);
> + if (err < 0)
> + return err;
> +
> + misr_status = phy_read(phydev, MII_DP83822_MISR2);
> + if (misr_status < 0)
> + return misr_status;
> +
> + misr_status |= (DP83822_JABBER_DET_INT_EN |
> + DP83822_WOL_PKT_INT_EN |
> + DP83822_SLEEP_MODE_INT_EN |
> + DP83822_MDI_XOVER_INT_EN |
> + DP83822_LB_FIFO_INT_EN |
> + DP83822_PAGE_RX_INT_EN |
> + DP83822_ANEG_ERR_INT_EN |
> + DP83822_EEE_ERROR_CHANGE_INT_EN);
> +
> + err = phy_write(phydev, MII_DP83822_MISR2, misr_status);
> + if (err < 0)
> + return err;
> +
> + physcr_status = phy_read(phydev, MII_DP83822_PHYSCR);
> + if (physcr_status < 0)
> + return physcr_status;
> +
> + physcr_status |= DP83822_PHYSCR_INT_OE |
> DP83822_PHYSCR_INTEN;
> +
Don't want to be picky, but seeing extra blank line here.
> + } else {
> + err = phy_write(phydev, MII_DP83822_MISR1, 0);
> + if (err < 0)
> + return err;
> +
> + err = phy_write(phydev, MII_DP83822_MISR1, 0);
> + if (err < 0)
> + return err;
> +
> + physcr_status = phy_read(phydev, MII_DP83822_PHYSCR);
> + if (physcr_status < 0)
> + return physcr_status;
> +
> + physcr_status &= ~DP83822_PHYSCR_INTEN;
> + }
^ 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