* Re: [PATCH net] net: socket: set sock->sk to NULL after calling proto_ops::release()
From: Eric Biggers @ 2019-02-22 17:57 UTC (permalink / raw)
To: Eric Dumazet
Cc: netdev, David S . Miller, linux-kernel, Mao Wenan, Cong Wang,
Lorenzo Colitti, Tetsuo Handa, Al Viro
In-Reply-To: <e4d0f1e8-68ff-512e-ad69-480523ff300f@gmail.com>
Hi Eric,
On Fri, Feb 22, 2019 at 09:45:35AM -0800, Eric Dumazet wrote:
>
>
> On 02/21/2019 02:13 PM, Eric Biggers wrote:
> > From: Eric Biggers <ebiggers@google.com>
> >
> > Commit 9060cb719e61 ("net: crypto set sk to NULL when af_alg_release.")
> > fixed a use-after-free in sockfs_setattr() when an AF_ALG socket is
> > closed concurrently with fchownat(). However, it ignored that many
> > other proto_ops::release() methods don't set sock->sk to NULL and
> > therefore allow the same use-after-free:
> >
>
> I fail to see how setting a pointer to NULL can avoid races.
>
>
> We lack some kind of protection, rcu or something, if another thread can change sock->sk at anytime
> while sockfs_setattr() is used.
>
> sockfs_setattr()
> ...
> if (sock->sk)
>
> // even if sock->sk was not NULL for the if (...).
>
> // it can be NULL right now, compiler could read sock->sk a second time and catch a NULL.
>
> sock->sk->sk_uid = iattr->ia_uid;
>
>
->setattr() is called under inode_lock(), which __sock_release() also takes. So
the uses of sock->sk are serialized. See commit 6d8c50dcb029 ("socket: close
race condition between sock_close() and sockfs_setattr()").
The issue now is that if ->setattr() happens *after* __sock_release() (which is
possible if fchownat() gets the reference to the file's 'struct path', then the
file is close()d by another thread, then fchownat() continues), it will see
stale sock->sk because for many socket types it wasn't set to NULL earlier.
- Eric
^ permalink raw reply
* Re: [PATCH] net: dsa: mv88e6xxx: Add lockdep classes to fix false positive splat
From: Andrew Lunn @ 2019-02-22 17:59 UTC (permalink / raw)
To: Russell King - ARM Linux admin; +Cc: netdev, Vivien Didelot
In-Reply-To: <20190222174856.tmisqn7ih3ea3xvz@shell.armlinux.org.uk>
> Hi Andrew,
>
> Do we have a way forward for this issue?
Hi Russell
Yes. I tested releasing the mutex around the request for the
interrupt. That works. So i will submit a patchset adding both the
lockdep class, and the unlock/lock pair.
Andrew
^ permalink raw reply
* Re: [PATCH v2 iproute2-next 04/11] devlink: Add helper functions for name and value separately
From: Stephen Hemminger @ 2019-02-22 18:01 UTC (permalink / raw)
To: Jiri Pirko
Cc: Aya Levin, David Ahern, netdev, Jiri Pirko, Moshe Shemesh,
Eran Ben Elisha
In-Reply-To: <20190222085832.GA2254@nanopsycho>
On Fri, 22 Feb 2019 09:58:32 +0100
Jiri Pirko <jiri@resnulli.us> wrote:
> Thu, Feb 21, 2019 at 07:02:11PM CET, stephen@networkplumber.org wrote:
> >On Thu, 21 Feb 2019 15:42:40 +0200
> >Aya Levin <ayal@mellanox.com> wrote:
> >
> >> Add a new helper functions which outputs only values (without name
> >> label) for different types: boolean, uint, uint64, string and binary.
> >> In addition add a helper function which prints only the name label.
> >>
> >> Signed-off-by: Aya Levin <ayal@mellanox.com>
> >> Reviewed-by: Moshe Shemesh <moshe@mellanox.com>
> >> ---
> >> devlink/devlink.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> >> 1 file changed, 65 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/devlink/devlink.c b/devlink/devlink.c
> >> index b073ae020d52..5d69c4f24f29 100644
> >> --- a/devlink/devlink.c
> >> +++ b/devlink/devlink.c
> >> @@ -1588,7 +1588,6 @@ static void pr_out_port_handle_end(struct dl *dl)
> >> pr_out("\n");
> >> }
> >>
> >> -
> >> static void pr_out_str(struct dl *dl, const char *name, const char *val)
> >> {
> >> if (dl->json_output) {
> >> @@ -1636,6 +1635,71 @@ static void pr_out_u64(struct dl *dl, const char *name, uint64_t val)
> >> }
> >> }
> >>
> >> +static void pr_out_bool_value(struct dl *dl, bool value)
> >> +{
> >> + if (dl->json_output)
> >> + jsonw_bool(dl->jw, value);
> >> + else
> >> + pr_out(" %s", value ? "true" : "false");
> >> +}
> >> +
> >> +static void pr_out_uint_value(struct dl *dl, unsigned int value)
> >> +{
> >> + if (dl->json_output)
> >> + jsonw_uint(dl->jw, value);
> >> + else
> >> + pr_out(" %u", value);
> >> +}
> >> +
> >> +static void pr_out_uint64_value(struct dl *dl, uint64_t value)
> >> +{
> >> + if (dl->json_output)
> >> + jsonw_u64(dl->jw, value);
> >> + else
> >> + pr_out(" %lu", value);
> >> +}
> >> +
> >> +static void pr_out_binary_value(struct dl *dl, uint8_t *data, uint32_t len)
> >> +{
> >> + int i = 1;
> >> +
> >> + if (dl->json_output)
> >> + jsonw_start_array(dl->jw);
> >> + else
> >> + pr_out("\n");
> >> +
> >> + while (i < len) {
> >> + if (dl->json_output) {
> >> + jsonw_printf(dl->jw, "%d", data[i]);
> >> + } else {
> >> + pr_out(" %02x", data[i]);
> >> + if (!(i % 16))
> >> + pr_out("\n");
> >> + }
> >> + i++;
> >> + }
> >> + if (dl->json_output)
> >> + jsonw_end_array(dl->jw);
> >> + else if ((i - 1) % 16)
> >> + pr_out("\n");
> >> +}
> >> +
> >> +static void pr_out_str_value(struct dl *dl, const char *value)
> >> +{
> >> + if (dl->json_output)
> >> + jsonw_string(dl->jw, value);
> >> + else
> >> + pr_out(" %s", value);
> >> +}
> >> +
> >> +static void pr_out_name(struct dl *dl, const char *name)
> >> +{
> >> + if (dl->json_output)
> >> + jsonw_name(dl->jw, name);
> >> + else
> >> + pr_out(" %s:", name);
> >> +}
> >> +
> >> static void pr_out_region_chunk_start(struct dl *dl, uint64_t addr)
> >> {
> >> if (dl->json_output) {
> >
> >NAK
> >
> >Please convert devlink to use existing iproute2 json_print infrasructure
> >rather than reinventing it.
>
> That requires whole devlink/devlink.c code change. Could this be done as
> a follow-up?
There is a tradeoff, if more code creeps in it is harder to do the code change.
Can you commit to a timeframe to convert the JSON support code?
^ permalink raw reply
* Re: [PATCH net] net: socket: set sock->sk to NULL after calling proto_ops::release()
From: Cong Wang @ 2019-02-22 18:05 UTC (permalink / raw)
To: Eric Biggers
Cc: Linux Kernel Network Developers, David S . Miller, LKML,
Mao Wenan, Lorenzo Colitti, Tetsuo Handa, Al Viro
In-Reply-To: <20190221221356.173485-1-ebiggers@kernel.org>
On Thu, Feb 21, 2019 at 2:14 PM Eric Biggers <ebiggers@kernel.org> wrote:
>
> From: Eric Biggers <ebiggers@google.com>
>
> Commit 9060cb719e61 ("net: crypto set sk to NULL when af_alg_release.")
> fixed a use-after-free in sockfs_setattr() when an AF_ALG socket is
> closed concurrently with fchownat(). However, it ignored that many
> other proto_ops::release() methods don't set sock->sk to NULL and
> therefore allow the same use-after-free:
>
> - base_sock_release
> - bnep_sock_release
> - cmtp_sock_release
> - data_sock_release
> - dn_release
> - hci_sock_release
> - hidp_sock_release
> - iucv_sock_release
> - l2cap_sock_release
> - llcp_sock_release
> - llc_ui_release
> - rawsock_release
> - rfcomm_sock_release
> - sco_sock_release
> - svc_release
> - vcc_release
> - x25_release
>
> Rather than fixing all these and relying on every socket type to get
> this right forever, just make __sock_release() set sock->sk to NULL
> itself after calling proto_ops::release().
>
> Reproducer that produces the KASAN splat when any of these socket types
> are configured into the kernel:
>
> #include <pthread.h>
> #include <stdlib.h>
> #include <sys/socket.h>
> #include <unistd.h>
>
> pthread_t t;
> volatile int fd;
>
> void *close_thread(void *arg)
> {
> for (;;) {
> usleep(rand() % 100);
> close(fd);
> }
> }
>
> int main()
> {
> pthread_create(&t, NULL, close_thread, NULL);
> for (;;) {
> fd = socket(rand() % 50, rand() % 11, 0);
> fchownat(fd, "", 1000, 1000, 0x1000);
> close(fd);
> }
> }
>
> Fixes: 86741ec25462 ("net: core: Add a UID field to struct sock.")
> Cc: <stable@vger.kernel.org> # v4.10+
> Signed-off-by: Eric Biggers <ebiggers@google.com>
Acked-by: Cong Wang <xiyou.wangcong@gmail.com>
^ permalink raw reply
* Re: [PATCH v2 iproute2-next 04/11] devlink: Add helper functions for name and value separately
From: Jiri Pirko @ 2019-02-22 17:59 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Aya Levin, David Ahern, netdev, Jiri Pirko, Moshe Shemesh,
Eran Ben Elisha
In-Reply-To: <20190222100125.20d8dd54@shemminger-XPS-13-9360>
Fri, Feb 22, 2019 at 07:01:25PM CET, stephen@networkplumber.org wrote:
>On Fri, 22 Feb 2019 09:58:32 +0100
>Jiri Pirko <jiri@resnulli.us> wrote:
>
>> Thu, Feb 21, 2019 at 07:02:11PM CET, stephen@networkplumber.org wrote:
>> >On Thu, 21 Feb 2019 15:42:40 +0200
>> >Aya Levin <ayal@mellanox.com> wrote:
>> >
>> >> Add a new helper functions which outputs only values (without name
>> >> label) for different types: boolean, uint, uint64, string and binary.
>> >> In addition add a helper function which prints only the name label.
>> >>
>> >> Signed-off-by: Aya Levin <ayal@mellanox.com>
>> >> Reviewed-by: Moshe Shemesh <moshe@mellanox.com>
>> >> ---
>> >> devlink/devlink.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>> >> 1 file changed, 65 insertions(+), 1 deletion(-)
>> >>
>> >> diff --git a/devlink/devlink.c b/devlink/devlink.c
>> >> index b073ae020d52..5d69c4f24f29 100644
>> >> --- a/devlink/devlink.c
>> >> +++ b/devlink/devlink.c
>> >> @@ -1588,7 +1588,6 @@ static void pr_out_port_handle_end(struct dl *dl)
>> >> pr_out("\n");
>> >> }
>> >>
>> >> -
>> >> static void pr_out_str(struct dl *dl, const char *name, const char *val)
>> >> {
>> >> if (dl->json_output) {
>> >> @@ -1636,6 +1635,71 @@ static void pr_out_u64(struct dl *dl, const char *name, uint64_t val)
>> >> }
>> >> }
>> >>
>> >> +static void pr_out_bool_value(struct dl *dl, bool value)
>> >> +{
>> >> + if (dl->json_output)
>> >> + jsonw_bool(dl->jw, value);
>> >> + else
>> >> + pr_out(" %s", value ? "true" : "false");
>> >> +}
>> >> +
>> >> +static void pr_out_uint_value(struct dl *dl, unsigned int value)
>> >> +{
>> >> + if (dl->json_output)
>> >> + jsonw_uint(dl->jw, value);
>> >> + else
>> >> + pr_out(" %u", value);
>> >> +}
>> >> +
>> >> +static void pr_out_uint64_value(struct dl *dl, uint64_t value)
>> >> +{
>> >> + if (dl->json_output)
>> >> + jsonw_u64(dl->jw, value);
>> >> + else
>> >> + pr_out(" %lu", value);
>> >> +}
>> >> +
>> >> +static void pr_out_binary_value(struct dl *dl, uint8_t *data, uint32_t len)
>> >> +{
>> >> + int i = 1;
>> >> +
>> >> + if (dl->json_output)
>> >> + jsonw_start_array(dl->jw);
>> >> + else
>> >> + pr_out("\n");
>> >> +
>> >> + while (i < len) {
>> >> + if (dl->json_output) {
>> >> + jsonw_printf(dl->jw, "%d", data[i]);
>> >> + } else {
>> >> + pr_out(" %02x", data[i]);
>> >> + if (!(i % 16))
>> >> + pr_out("\n");
>> >> + }
>> >> + i++;
>> >> + }
>> >> + if (dl->json_output)
>> >> + jsonw_end_array(dl->jw);
>> >> + else if ((i - 1) % 16)
>> >> + pr_out("\n");
>> >> +}
>> >> +
>> >> +static void pr_out_str_value(struct dl *dl, const char *value)
>> >> +{
>> >> + if (dl->json_output)
>> >> + jsonw_string(dl->jw, value);
>> >> + else
>> >> + pr_out(" %s", value);
>> >> +}
>> >> +
>> >> +static void pr_out_name(struct dl *dl, const char *name)
>> >> +{
>> >> + if (dl->json_output)
>> >> + jsonw_name(dl->jw, name);
>> >> + else
>> >> + pr_out(" %s:", name);
>> >> +}
>> >> +
>> >> static void pr_out_region_chunk_start(struct dl *dl, uint64_t addr)
>> >> {
>> >> if (dl->json_output) {
>> >
>> >NAK
>> >
>> >Please convert devlink to use existing iproute2 json_print infrasructure
>> >rather than reinventing it.
>>
>> That requires whole devlink/devlink.c code change. Could this be done as
>> a follow-up?
>
>There is a tradeoff, if more code creeps in it is harder to do the code change.
>Can you commit to a timeframe to convert the JSON support code?
Okay, I'll take care of it in March. Promise.
^ permalink raw reply
* Re: [RFC] rtnetlink: handle multiple vlan tags in set_vf_vlan
From: Stephen Hemminger @ 2019-02-22 18:09 UTC (permalink / raw)
To: Moshe Shemesh; +Cc: netdev@vger.kernel.org, Stephen Hemminger
In-Reply-To: <24d8a96d-4bac-fe8d-1308-c2caee2a9d2c@mellanox.com>
On Fri, 22 Feb 2019 04:58:06 +0000
Moshe Shemesh <moshe@mellanox.com> wrote:
> On 2/21/2019 7:54 PM, Stephen Hemminger wrote:
> > The netlink API for IFLA_VF_VLAN_LIST allows multiple VLAN tags to
> > be passed (and the message was validated) but only the first VLAN
> > tag was being passed to the device. Change to iterate over each tag received.
> >
> > Fixes: 79aab093a0b5 ("net: Update API for VF vlan protocol 802.1ad support")
> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > ---
> > net/core/rtnetlink.c | 19 ++++++++++---------
> > 1 file changed, 10 insertions(+), 9 deletions(-)
> >
> > diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> > index a51cab95ba64..3a9ec988ae21 100644
> > --- a/net/core/rtnetlink.c
> > +++ b/net/core/rtnetlink.c
> > @@ -2207,11 +2207,10 @@ static int do_setvfinfo(struct net_device *dev, struct nlattr **tb)
> > if (tb[IFLA_VF_VLAN_LIST]) {
> > struct ifla_vf_vlan_info *ivvl[MAX_VLAN_LIST_LEN];
> > struct nlattr *attr;
> > - int rem, len = 0;
> > + int i, rem, len = 0;
> >
> > - err = -EOPNOTSUPP;
> > if (!ops->ndo_set_vf_vlan)
> > - return err;
> > + return -EOPNOTSUPP;
> >
> > nla_for_each_nested(attr, tb[IFLA_VF_VLAN_LIST], rem) {
> > if (nla_type(attr) != IFLA_VF_VLAN_INFO ||
> > @@ -2224,13 +2223,15 @@ static int do_setvfinfo(struct net_device *dev, struct nlattr **tb)
> >
> > len++;
> > }
> > - if (len == 0)
> > - return -EINVAL;
> >
> > - err = ops->ndo_set_vf_vlan(dev, ivvl[0]->vf, ivvl[0]->vlan,
> > - ivvl[0]->qos, ivvl[0]->vlan_proto);
> > - if (err < 0)
> > - return err;
> > + err = -EINVAL; /* empty list error */
> > + for (i = 0; i < len; i++) {
> > + err = ops->ndo_set_vf_vlan(dev, ivvl[i]->vf,
> > + ivvl[i]->vlan, ivvl[i]->qos,
> > + ivvl[i]->vlan_proto);
>
> Doing that each vlan will just overwrite the vf vlan configuration set
> by its preceding one.
> Note #define MAX_VLAN_LIST_LEN 1
> The point here was that I had to add the rtnl interface to set vf vlan
> with option to set vf vlan protocol. While doing that I was asked to add
> option to get a list of vlans from user to support QinQ, so once it will
> be needed there won't be a need to add another rtnl interface.
>
> The driver which will support setting double vlan or more per vf will
> change MAX_VLAN_LIST_LEN and change the ndo function.
>
OK, then the original code kind of makes sense now. Maybe the whole loop
could be eliminated.
I expected that the vlan list was for allowing multiple vlan's to go to
a single VF but it is really a future (unlikely to ever be implemented)
support for QinQ.
^ permalink raw reply
* [PATCH net-next 0/3] net: protodown support for macvlan and vxlan
From: Andy Roulin @ 2019-02-22 18:06 UTC (permalink / raw)
To: davem; +Cc: netdev, roopa, aroulin
This patch series adds dev_change_proto_down_generic, a generic
implementation of ndo_change_proto_down, which sets the netdev carrier
state according to the new proto_down value.
This handler adds the ability to set protodown on macvlan and vxlan
interfaces in a generic way for use by control protocols like VRRPD.
Patch (1) introduces the handler in net/code/dev.c. Patch (2) and (3) add
support for change_proto_down in macvlan and vxlan drivers, respectively,
using the new function.
Andy Roulin (3):
net: dev: add generic protodown handler
macvlan: add ndo_change_proto_down support
vxlan: add ndo_change_proto_down support
drivers/net/macvlan.c | 1 +
drivers/net/vxlan.c | 1 +
include/linux/netdevice.h | 1 +
net/core/dev.c | 19 +++++++++++++++++++
4 files changed, 22 insertions(+)
--
2.11.0
^ permalink raw reply
* [PATCH net-next 3/3] vxlan: add ndo_change_proto_down support
From: Andy Roulin @ 2019-02-22 18:06 UTC (permalink / raw)
To: davem; +Cc: netdev, roopa, aroulin
In-Reply-To: <20190222180638.10904-1-aroulin@cumulusnetworks.com>
Add ndo_change_proto_down support through dev_change_proto_down_generic
for use by control protocols like VRRPD.
Signed-off-by: Andy Roulin <aroulin@cumulusnetworks.com>
Acked-by: Roopa Prabhu <roopa@cumulusnetworks.com>
---
drivers/net/vxlan.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 33edc78e818d..577201cd880c 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -2923,6 +2923,7 @@ static const struct net_device_ops vxlan_netdev_ether_ops = {
.ndo_fdb_dump = vxlan_fdb_dump,
.ndo_fdb_get = vxlan_fdb_get,
.ndo_fill_metadata_dst = vxlan_fill_metadata_dst,
+ .ndo_change_proto_down = dev_change_proto_down_generic,
};
static const struct net_device_ops vxlan_netdev_raw_ops = {
--
2.11.0
^ permalink raw reply related
* [PATCH net-next 1/3] net: dev: add generic protodown handler
From: Andy Roulin @ 2019-02-22 18:06 UTC (permalink / raw)
To: davem; +Cc: netdev, roopa, aroulin
In-Reply-To: <20190222180638.10904-1-aroulin@cumulusnetworks.com>
Introduce dev_change_proto_down_generic, a generic ndo_change_proto_down
implementation, which sets the netdev carrier state according to proto_down.
This adds the ability to set protodown on vxlan and macvlan devices in a
generic way for use by control protocols like VRRPD.
Signed-off-by: Andy Roulin <aroulin@cumulusnetworks.com>
Acked-by: Roopa Prabhu <roopa@cumulusnetworks.com>
---
include/linux/netdevice.h | 1 +
net/core/dev.c | 19 +++++++++++++++++++
2 files changed, 20 insertions(+)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index aab4d9f6613d..9a093fde030d 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -3663,6 +3663,7 @@ int dev_get_port_parent_id(struct net_device *dev,
struct netdev_phys_item_id *ppid, bool recurse);
bool netdev_port_same_parent_id(struct net_device *a, struct net_device *b);
int dev_change_proto_down(struct net_device *dev, bool proto_down);
+int dev_change_proto_down_generic(struct net_device *dev, bool proto_down);
struct sk_buff *validate_xmit_skb_list(struct sk_buff *skb, struct net_device *dev, bool *again);
struct sk_buff *dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev,
struct netdev_queue *txq, int *ret);
diff --git a/net/core/dev.c b/net/core/dev.c
index a3d13f5e2bfc..e99a870772c1 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -7954,6 +7954,25 @@ int dev_change_proto_down(struct net_device *dev, bool proto_down)
}
EXPORT_SYMBOL(dev_change_proto_down);
+/**
+ * dev_change_proto_down_generic - generic implementation for
+ * ndo_change_proto_down that sets carrier according to
+ * proto_down.
+ *
+ * @dev: device
+ * @proto_down: new value
+ */
+int dev_change_proto_down_generic(struct net_device *dev, bool proto_down)
+{
+ if (proto_down)
+ netif_carrier_off(dev);
+ else
+ netif_carrier_on(dev);
+ dev->proto_down = proto_down;
+ return 0;
+}
+EXPORT_SYMBOL(dev_change_proto_down_generic);
+
u32 __dev_xdp_query(struct net_device *dev, bpf_op_t bpf_op,
enum bpf_netdev_command cmd)
{
--
2.11.0
^ permalink raw reply related
* [PATCH net-next 2/3] macvlan: add ndo_change_proto_down support
From: Andy Roulin @ 2019-02-22 18:06 UTC (permalink / raw)
To: davem; +Cc: netdev, roopa, aroulin
In-Reply-To: <20190222180638.10904-1-aroulin@cumulusnetworks.com>
Add ndo_change_proto_down support through dev_change_proto_down_generic
for use by control protocols like VRRPD.
Signed-off-by: Andy Roulin <aroulin@cumulusnetworks.com>
Acked-by: Roopa Prabhu <roopa@cumulusnetworks.com>
---
drivers/net/macvlan.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index 26e53832b095..0c0f105657d3 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -1122,6 +1122,7 @@ static const struct net_device_ops macvlan_netdev_ops = {
#endif
.ndo_get_iflink = macvlan_dev_get_iflink,
.ndo_features_check = passthru_features_check,
+ .ndo_change_proto_down = dev_change_proto_down_generic,
};
void macvlan_common_setup(struct net_device *dev)
--
2.11.0
^ permalink raw reply related
* Re: [PATCH net] net: socket: set sock->sk to NULL after calling proto_ops::release()
From: Eric Dumazet @ 2019-02-22 18:25 UTC (permalink / raw)
To: Eric Biggers, Eric Dumazet
Cc: netdev, David S . Miller, linux-kernel, Mao Wenan, Cong Wang,
Lorenzo Colitti, Tetsuo Handa, Al Viro
In-Reply-To: <20190222175743.GA163909@gmail.com>
On 02/22/2019 09:57 AM, Eric Biggers wrote:
> ->setattr() is called under inode_lock(), which __sock_release() also takes. So
> the uses of sock->sk are serialized. See commit 6d8c50dcb029 ("socket: close
> race condition between sock_close() and sockfs_setattr()").
Oh right, we added another inode_lock()/inode_unlock() for sock_close()
>
> The issue now is that if ->setattr() happens *after* __sock_release() (which is
> possible if fchownat() gets the reference to the file's 'struct path', then the
> file is close()d by another thread, then fchownat() continues), it will see
> stale sock->sk because for many socket types it wasn't set to NULL earlier.
>
> - Eric
>
^ permalink raw reply
* Re: general protection fault in rose_send_frame
From: syzbot @ 2019-02-22 18:26 UTC (permalink / raw)
To: davem, linux-hams, linux-kernel, netdev, ralf, syzkaller-bugs
In-Reply-To: <00000000000089904d057f1e0ae0@google.com>
syzbot has found a reproducer for the following crash on:
HEAD commit: 7a25c6c0aac8 rocker: Add missing break for PRE_BRIDGE_FLAGS
git tree: net-next
console output: https://syzkaller.appspot.com/x/log.txt?x=104002d8c00000
kernel config: https://syzkaller.appspot.com/x/.config?x=58cb9d752ba5f3e0
dashboard link: https://syzkaller.appspot.com/bug?extid=7078ae989d857fe17988
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1224c304c00000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=151824f8c00000
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+7078ae989d857fe17988@syzkaller.appspotmail.com
IPv6: ADDRCONF(NETDEV_CHANGE): hsr_slave_1: link becomes ready
8021q: adding VLAN 0 to HW filter on device batadv0
IPv6: ADDRCONF(NETDEV_CHANGE): rose0: link becomes ready
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: 0 Comm: swapper/0 Not tainted 5.0.0-rc7+ #76
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
RIP: 0010:rose_send_frame+0x1a8/0x280 net/rose/rose_link.c:104
Code: c1 ea 03 80 3c 02 00 0f 85 8d 00 00 00 48 b8 00 00 00 00 00 fc ff df
4c 8b 63 20 49 8d bc 24 58 03 00 00 48 89 fa 48 c1 ea 03 <80> 3c 02 00 75
7e 49 8b 94 24 58 03 00 00 e9 b8 fe ff ff e8 f0 53
RSP: 0018:ffff8880ae807ae8 EFLAGS: 00010202
RAX: dffffc0000000000 RBX: ffff88809aa1fa00 RCX: ffffffff86378efb
RDX: 000000000000006b RSI: ffffffff8637902c RDI: 0000000000000358
RBP: ffff8880ae807b18 R08: ffffffff8887dec0 R09: ffffed101263146d
R10: ffffed101263146c R11: ffff88809318a363 R12: 0000000000000000
R13: 0000000000000078 R14: 0000000000000005 R15: ffff8880a4f6fbc0
FS: 0000000000000000(0000) GS:ffff8880ae800000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f23b3e4ee78 CR3: 000000009eef3000 CR4: 00000000001406f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
<IRQ>
rose_transmit_clear_request+0x1de/0x2a0 net/rose/rose_link.c:258
rose_rx_call_request+0x4ea/0x1990 net/rose/af_rose.c:1001
rose_loopback_timer+0x26a/0x3f0 net/rose/rose_loopback.c:100
call_timer_fn+0x190/0x720 kernel/time/timer.c:1325
expire_timers kernel/time/timer.c:1362 [inline]
__run_timers kernel/time/timer.c:1681 [inline]
__run_timers kernel/time/timer.c:1649 [inline]
run_timer_softirq+0x652/0x1700 kernel/time/timer.c:1694
__do_softirq+0x266/0x95a kernel/softirq.c:292
invoke_softirq kernel/softirq.c:373 [inline]
irq_exit+0x180/0x1d0 kernel/softirq.c:413
exiting_irq arch/x86/include/asm/apic.h:536 [inline]
smp_apic_timer_interrupt+0x14a/0x570 arch/x86/kernel/apic/apic.c:1062
apic_timer_interrupt+0xf/0x20 arch/x86/entry/entry_64.S:807
</IRQ>
RIP: 0010:native_safe_halt+0x2/0x10 arch/x86/include/asm/irqflags.h:58
Code: ff ff ff 48 89 c7 48 89 45 d8 e8 59 6c a1 fa 48 8b 45 d8 e9 ce fe ff
ff 48 89 df e8 48 6c a1 fa eb 82 90 90 90 90 90 90 fb f4 <c3> 0f 1f 00 66
2e 0f 1f 84 00 00 00 00 00 f4 c3 90 90 90 90 90 90
RSP: 0018:ffffffff88807d08 EFLAGS: 00000286 ORIG_RAX: ffffffffffffff13
RAX: 1ffffffff1125061 RBX: ffffffff8887dec0 RCX: 0000000000000000
RDX: dffffc0000000000 RSI: 0000000000000001 RDI: ffffffff8887e73c
RBP: ffffffff88807d38 R08: ffffffff8887dec0 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
R13: ffffffff889282f8 R14: 0000000000000000 R15: 0000000000000000
arch_cpu_idle+0x10/0x20 arch/x86/kernel/process.c:555
default_idle_call+0x36/0x90 kernel/sched/idle.c:93
cpuidle_idle_call kernel/sched/idle.c:153 [inline]
do_idle+0x386/0x570 kernel/sched/idle.c:262
cpu_startup_entry+0x1b/0x20 kernel/sched/idle.c:353
rest_init+0x245/0x37b init/main.c:442
arch_call_rest_init+0xe/0x1b
start_kernel+0x803/0x83c init/main.c:739
x86_64_start_reservations+0x29/0x2b arch/x86/kernel/head64.c:470
x86_64_start_kernel+0x77/0x7b arch/x86/kernel/head64.c:451
secondary_startup_64+0xa4/0xb0 arch/x86/kernel/head_64.S:243
Modules linked in:
---[ end trace 8f29030702ddb052 ]---
RIP: 0010:rose_send_frame+0x1a8/0x280 net/rose/rose_link.c:104
Code: c1 ea 03 80 3c 02 00 0f 85 8d 00 00 00 48 b8 00 00 00 00 00 fc ff df
4c 8b 63 20 49 8d bc 24 58 03 00 00 48 89 fa 48 c1 ea 03 <80> 3c 02 00 75
7e 49 8b 94 24 58 03 00 00 e9 b8 fe ff ff e8 f0 53
RSP: 0018:ffff8880ae807ae8 EFLAGS: 00010202
RAX: dffffc0000000000 RBX: ffff88809aa1fa00 RCX: ffffffff86378efb
RDX: 000000000000006b RSI: ffffffff8637902c RDI: 0000000000000358
RBP: ffff8880ae807b18 R08: ffffffff8887dec0 R09: ffffed101263146d
R10: ffffed101263146c R11: ffff88809318a363 R12: 0000000000000000
R13: 0000000000000078 R14: 0000000000000005 R15: ffff8880a4f6fbc0
FS: 0000000000000000(0000) GS:ffff8880ae800000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f23b3e4ee78 CR3: 000000009eef3000 CR4: 00000000001406f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
^ permalink raw reply
* [PATCH v2 net-next] net: phy: improve definition of __ETHTOOL_LINK_MODE_MASK_NBITS
From: Heiner Kallweit @ 2019-02-22 18:25 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli, David Miller; +Cc: netdev@vger.kernel.org
The way to define __ETHTOOL_LINK_MODE_MASK_NBITS seems to be overly
complicated, go with a standard approach instead.
Whilst we're at it, move the comment to the right place.
v2:
- rebased
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
include/linux/ethtool.h | 4 ----
include/uapi/linux/ethtool.h | 17 +++++++++--------
2 files changed, 9 insertions(+), 12 deletions(-)
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 19a8de532..e6ebc9761 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -98,10 +98,6 @@ static inline u32 ethtool_rxfh_indir_default(u32 index, u32 n_rx_rings)
return index % n_rx_rings;
}
-/* number of link mode bits/ulongs handled internally by kernel */
-#define __ETHTOOL_LINK_MODE_MASK_NBITS \
- (__ETHTOOL_LINK_MODE_LAST + 1)
-
/* declare a link mode bitmap */
#define __ETHTOOL_DECLARE_LINK_MODE_MASK(name) \
DECLARE_BITMAP(name, __ETHTOOL_LINK_MODE_MASK_NBITS)
diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
index 378c52308..3652b239d 100644
--- a/include/uapi/linux/ethtool.h
+++ b/include/uapi/linux/ethtool.h
@@ -1432,6 +1432,13 @@ enum ethtool_link_mode_bit_indices {
ETHTOOL_LINK_MODE_56000baseSR4_Full_BIT = 29,
ETHTOOL_LINK_MODE_56000baseLR4_Full_BIT = 30,
ETHTOOL_LINK_MODE_25000baseCR_Full_BIT = 31,
+
+ /* Last allowed bit for __ETHTOOL_LINK_MODE_LEGACY_MASK is bit
+ * 31. Please do NOT define any SUPPORTED_* or ADVERTISED_*
+ * macro for bits > 31. The only way to use indices > 31 is to
+ * use the new ETHTOOL_GLINKSETTINGS/ETHTOOL_SLINKSETTINGS API.
+ */
+
ETHTOOL_LINK_MODE_25000baseKR_Full_BIT = 32,
ETHTOOL_LINK_MODE_25000baseSR_Full_BIT = 33,
ETHTOOL_LINK_MODE_50000baseCR2_Full_BIT = 34,
@@ -1469,14 +1476,8 @@ enum ethtool_link_mode_bit_indices {
ETHTOOL_LINK_MODE_200000baseDR4_Full_BIT = 65,
ETHTOOL_LINK_MODE_200000baseCR4_Full_BIT = 66,
- /* Last allowed bit for __ETHTOOL_LINK_MODE_LEGACY_MASK is bit
- * 31. Please do NOT define any SUPPORTED_* or ADVERTISED_*
- * macro for bits > 31. The only way to use indices > 31 is to
- * use the new ETHTOOL_GLINKSETTINGS/ETHTOOL_SLINKSETTINGS API.
- */
-
- __ETHTOOL_LINK_MODE_LAST
- = ETHTOOL_LINK_MODE_200000baseCR4_Full_BIT,
+ /* must be last entry */
+ __ETHTOOL_LINK_MODE_MASK_NBITS
};
#define __ETHTOOL_LINK_MODE_LEGACY_MASK(base_name) \
--
2.20.1
^ permalink raw reply related
* [PATCH hyperv-fixes] hv_netvsc: Fix IP header checksum for coalesced packets
From: Haiyang Zhang @ 2019-02-22 18:25 UTC (permalink / raw)
To: sashal, linux-hyperv
Cc: haiyangz, kys, sthemmin, olaf, vkuznets, davem, netdev,
linux-kernel
From: Haiyang Zhang <haiyangz@microsoft.com>
Incoming packets may have IP header checksum verified by the host.
They may not have IP header checksum computed after coalescing.
This patch re-compute the checksum when necessary, otherwise the
packets may be dropped, because Linux network stack always checks it.
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
drivers/net/hyperv/netvsc_drv.c | 22 +++++++++++++++++++---
1 file changed, 19 insertions(+), 3 deletions(-)
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index 256adbd044f5..cf4897043e83 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -744,6 +744,14 @@ void netvsc_linkstatus_callback(struct net_device *net,
schedule_delayed_work(&ndev_ctx->dwork, 0);
}
+static void netvsc_comp_ipcsum(struct sk_buff *skb)
+{
+ struct iphdr *iph = (struct iphdr *)skb->data;
+
+ iph->check = 0;
+ iph->check = ip_fast_csum(iph, iph->ihl);
+}
+
static struct sk_buff *netvsc_alloc_recv_skb(struct net_device *net,
struct netvsc_channel *nvchan)
{
@@ -770,9 +778,17 @@ static struct sk_buff *netvsc_alloc_recv_skb(struct net_device *net,
/* skb is already created with CHECKSUM_NONE */
skb_checksum_none_assert(skb);
- /*
- * In Linux, the IP checksum is always checked.
- * Do L4 checksum offload if enabled and present.
+ /* Incoming packets may have IP header checksum verified by the host.
+ * They may not have IP header checksum computed after coalescing.
+ * We compute it here if the flags are set, because on Linux, the IP
+ * checksum is always checked.
+ */
+ if (csum_info && csum_info->receive.ip_checksum_value_invalid &&
+ csum_info->receive.ip_checksum_succeeded &&
+ skb->protocol == htons(ETH_P_IP))
+ netvsc_comp_ipcsum(skb);
+
+ /* Do L4 checksum offload if enabled and present.
*/
if (csum_info && (net->features & NETIF_F_RXCSUM)) {
if (csum_info->receive.tcp_checksum_succeeded ||
--
2.19.1
^ permalink raw reply related
* Re: [PATCH net-next 1/7] net: phy: marvell10g: Use get_features to get the PHY abilities
From: Heiner Kallweit @ 2019-02-22 18:42 UTC (permalink / raw)
To: Maxime Chevallier, davem
Cc: netdev, linux-kernel, Andrew Lunn, Florian Fainelli, Russell King,
linux-arm-kernel, Antoine Tenart, thomas.petazzoni,
gregory.clement, miquel.raynal, nadavh, stefanc, mw
In-Reply-To: <20190221095128.28188-2-maxime.chevallier@bootlin.com>
On 21.02.2019 10:51, Maxime Chevallier wrote:
> The Alaska family of 10G PHYs has more abilities than the ones listed in
> PHY_10GBIT_FULL_FEATURES, the exact list depending on the model.
>
> Make use of the newly introduced .get_features call to build this list,
> using genphy_c45_pma_read_abilities to build the list of supported
> linkmodes, and adding autoneg ability based on what's reported by the AN
> MMD.
>
> .config_init is still used to validate the interface_mode.
>
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
> ---
> drivers/net/phy/marvell10g.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/phy/marvell10g.c b/drivers/net/phy/marvell10g.c
> index 9ea27acf05ad..65ef469adf58 100644
> --- a/drivers/net/phy/marvell10g.c
> +++ b/drivers/net/phy/marvell10g.c
> @@ -233,8 +233,6 @@ static int mv3310_resume(struct phy_device *phydev)
>
> static int mv3310_config_init(struct phy_device *phydev)
> {
> - int ret, val;
> -
> /* Check that the PHY interface type is compatible */
> if (phydev->interface != PHY_INTERFACE_MODE_SGMII &&
> phydev->interface != PHY_INTERFACE_MODE_XAUI &&
> @@ -242,6 +240,12 @@ static int mv3310_config_init(struct phy_device *phydev)
> phydev->interface != PHY_INTERFACE_MODE_10GKR)
> return -ENODEV;
>
> + return 0;
> +}
> +
> +static int mv3310_get_features(struct phy_device *phydev)
> +{
After my just submitted patch to include the aneg capability checking in
genphy_c45_pma_read_abilities() function mv3310_get_features() isn't
needed any longer and can be replaced with the generic one.
But we can make this change afterwards, then you don't have to
rework your series.
Also I'm not sure whether there will be a 5.0-rc8 or whether beginning
of next week we'll see 5.0. In the latter case we're a little bit in a
hurry because the merge window will start very soon.
> + int ret, val;
> if (phydev->c45_ids.devices_in_package & MDIO_DEVS_AN) {
> val = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_STAT1);
> if (val < 0)
> @@ -429,7 +433,7 @@ static struct phy_driver mv3310_drivers[] = {
> .phy_id = 0x002b09aa,
> .phy_id_mask = MARVELL_PHY_ID_MASK,
> .name = "mv88x3310",
> - .features = PHY_10GBIT_FEATURES,
> + .get_features = mv3310_get_features,
> .soft_reset = gen10g_no_soft_reset,
> .config_init = mv3310_config_init,
> .probe = mv3310_probe,
>
^ permalink raw reply
* Re: [PATCH net-next 4/5] nfp: add .ndo_get_devlink
From: Jakub Kicinski @ 2019-02-22 18:53 UTC (permalink / raw)
To: Michal Kubecek; +Cc: davem, jiri, andrew, f.fainelli, netdev, oss-drivers
In-Reply-To: <20190222172715.GT23151@unicorn.suse.cz>
On Fri, 22 Feb 2019 18:27:15 +0100, Michal Kubecek wrote:
> > > Maybe it would be safer not to call ndo_get_devlink directly and have
> > > an inline wrapper like
> > >
> > > #if IS_ENABLED(CONFIG_NET_DEVLINK)
> > > static inline struct devlink *dev_get_devlink(struct net_device *dev)
> > > {
> > > if (dev->netdev_ops->ndo_get_devlink)
> > > return dev->netdev_ops->ndo_get_devlink();
> > > else
> > > retrurn NULL;
> > > }
> > > #else
> > > static inline struct devlink *dev_get_devlink(struct net_device *dev)
> > > {
> > > return NULL;
> > > }
> > > #endif
> > >
> > > so that one can simply call the wrapper and check return value for NULL.
> >
> > Only devlink code can call this ndo, and it doesn't exist with
> > DEVLINK=n. I don't dislike wrappers for NDOs, but I'll defer to Jiri
> > to decide if we want a wrapper here (without the #if/#else, just the
> > first part for code clarity) :)
>
> If the NDO is only supposed to be called from devlink code (or, more
> precisely, code built only with CONFIG_DEVLINK=y), it should be IMHO
> mentioned in its description. Another option would be enforcing it by
> adding #ifdef around the ndo_get_devlink entry in struct net_device_ops
> but that would require using ifdefs also in each driver providing the
> NDO which seems inconvenient.
Yes, let's just go with your first proposal of a static inline helper.
I think it's the cleanest solution.
^ permalink raw reply
* [PATCH] net: dsa: Remove documentation for port_fdb_prepare
From: Hauke Mehrtens @ 2019-02-22 19:07 UTC (permalink / raw)
To: davem; +Cc: arkadis, netdev, linux-doc, Hauke Mehrtens
This callback was removed some time ago, also remove the documentation.
Fixes: 1b6dd556c304 ("net: dsa: Remove prepare phase for FDB")
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
---
Documentation/networking/dsa/dsa.txt | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/Documentation/networking/dsa/dsa.txt b/Documentation/networking/dsa/dsa.txt
index 25170ad7d25b..101f2b2c69ad 100644
--- a/Documentation/networking/dsa/dsa.txt
+++ b/Documentation/networking/dsa/dsa.txt
@@ -533,16 +533,12 @@ Bridge VLAN filtering
function that the driver has to call for each VLAN the given port is a member
of. A switchdev object is used to carry the VID and bridge flags.
-- port_fdb_prepare: bridge layer function invoked when the bridge prepares the
- installation of a Forwarding Database entry. If the operation is not
- supported, this function should return -EOPNOTSUPP to inform the bridge code
- to fallback to a software implementation. No hardware setup must be done in
- this function. See port_fdb_add for this and details.
-
- port_fdb_add: bridge layer function invoked when the bridge wants to install a
Forwarding Database entry, the switch hardware should be programmed with the
specified address in the specified VLAN Id in the forwarding database
- associated with this VLAN ID
+ associated with this VLAN ID. If the operation is not supported, this
+ function should return -EOPNOTSUPP to inform the bridge code to fallback to
+ a software implementation.
Note: VLAN ID 0 corresponds to the port private database, which, in the context
of DSA, would be the its port-based VLAN, used by the associated bridge device.
--
2.20.1
^ permalink raw reply related
* Re: [PATCH net] net: socket: set sock->sk to NULL after calling proto_ops::release()
From: Al Viro @ 2019-02-22 19:08 UTC (permalink / raw)
To: Eric Dumazet
Cc: Eric Biggers, netdev, David S . Miller, linux-kernel, Mao Wenan,
Cong Wang, Lorenzo Colitti, Tetsuo Handa
In-Reply-To: <0de89a50-6d5c-1d7b-19f5-9a13465bcebd@gmail.com>
On Fri, Feb 22, 2019 at 10:25:09AM -0800, Eric Dumazet wrote:
>
>
> On 02/22/2019 09:57 AM, Eric Biggers wrote:
>
> > ->setattr() is called under inode_lock(), which __sock_release() also takes. So
> > the uses of sock->sk are serialized. See commit 6d8c50dcb029 ("socket: close
> > race condition between sock_close() and sockfs_setattr()").
>
> Oh right, we added another inode_lock()/inode_unlock() for sock_close()
An interesting question is whether anything else will be confused by
sock->sk && sock->sk->sk_socket != sock
I'd still like to figure out if we could simply make sock_orphan()
do something like
if (likely(sk->sk_socket))
sk->sk_socket->sk = NULL;
just before sk_set_socket(sk, NULL);
That would make for much easier rules; the question is whether anything
relies upon the windows when linkage between socket and sock is not
symmetrical...
^ permalink raw reply
* [PATCH] net: dsa: lantiq: Add GPHY firmware files
From: Hauke Mehrtens @ 2019-02-22 19:11 UTC (permalink / raw)
To: davem; +Cc: netdev, Hauke Mehrtens
This adds the file names of the FW files which this driver handles into
the module description.
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
---
drivers/net/dsa/lantiq_gswip.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/net/dsa/lantiq_gswip.c b/drivers/net/dsa/lantiq_gswip.c
index 693a67f45bef..ddc1f9ca8ebc 100644
--- a/drivers/net/dsa/lantiq_gswip.c
+++ b/drivers/net/dsa/lantiq_gswip.c
@@ -1162,6 +1162,12 @@ static struct platform_driver gswip_driver = {
module_platform_driver(gswip_driver);
+MODULE_FIRMWARE("lantiq/xrx300_phy11g_a21.bin");
+MODULE_FIRMWARE("lantiq/xrx300_phy22f_a21.bin");
+MODULE_FIRMWARE("lantiq/xrx200_phy11g_a14.bin");
+MODULE_FIRMWARE("lantiq/xrx200_phy11g_a22.bin");
+MODULE_FIRMWARE("lantiq/xrx200_phy22f_a14.bin");
+MODULE_FIRMWARE("lantiq/xrx200_phy22f_a22.bin");
MODULE_AUTHOR("Hauke Mehrtens <hauke@hauke-m.de>");
MODULE_DESCRIPTION("Lantiq / Intel GSWIP driver");
MODULE_LICENSE("GPL v2");
--
2.20.1
^ permalink raw reply related
* [PATCH] net: lantiq: Do not use eth_change_mtu()
From: Hauke Mehrtens @ 2019-02-22 19:12 UTC (permalink / raw)
To: davem; +Cc: netdev, Hauke Mehrtens
eth_change_mtu() is not needed any more, the networking subsystem will
call it automatically when this callback is not implemented.
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
---
drivers/net/ethernet/lantiq_xrx200.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/net/ethernet/lantiq_xrx200.c b/drivers/net/ethernet/lantiq_xrx200.c
index 2d4d10a017e5..d29104de0d53 100644
--- a/drivers/net/ethernet/lantiq_xrx200.c
+++ b/drivers/net/ethernet/lantiq_xrx200.c
@@ -335,7 +335,6 @@ static const struct net_device_ops xrx200_netdev_ops = {
.ndo_start_xmit = xrx200_start_xmit,
.ndo_set_mac_address = eth_mac_addr,
.ndo_validate_addr = eth_validate_addr,
- .ndo_change_mtu = eth_change_mtu,
};
static irqreturn_t xrx200_dma_irq(int irq, void *ptr)
--
2.20.1
^ permalink raw reply related
* Re: [PATCH 3/8] dt-bindings: net: bluetooth: Add rtl8723bs-bluetooth
From: David Summers @ 2019-02-22 19:14 UTC (permalink / raw)
To: Vasily Khoruzhick, Stefan Wahren
Cc: Rob Herring, Mark Rutland, devicetree, Johan Hedberg,
Maxime Ripard, netdev, Marcel Holtmann, linux-bluetooth,
Chen-Yu Tsai, David S. Miller, arm-linux
In-Reply-To: <CA+E=qVdMNmVkytCzXqYgJtV196-G=c_bWx+aLvs=Y0iuRpjdRg@mail.gmail.com>
Hi Vasily,
Just catching up with this series - good that you are doing it. We need
it for users on ArchLinux ARM!
On question though, what is "firmware-postfix" used for? I see in the
documentation, that it points to the board name. But can't see where
else it is used.
Is there a need to have the board name?
Anyway good work.
David.
On 18/02/2019 22:28, Vasily Khoruzhick wrote:
> On Mon, Feb 18, 2019 at 2:08 PM Stefan Wahren <stefan.wahren@i2se.com> wrote:
>> Hi Vasily,
> Hi Stefan,
>
>>> Vasily Khoruzhick <anarsoul@gmail.com> hat am 18. Februar 2019 um 22:24 geschrieben:
>>>
>>>
>>> On Mon, Feb 18, 2019 at 1:10 PM Rob Herring <robh@kernel.org> wrote:
>>>> On Fri, Jan 18, 2019 at 09:02:27AM -0800, Vasily Khoruzhick wrote:
>>>>> Add binding document for bluetooth part of RTL8723BS/RTL8723CS
>>>>>
>>>>> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
>>>>> ---
>>>>> .../bindings/net/rtl8723bs-bluetooth.txt | 35 +++++++++++++++++++
>>>>> 1 file changed, 35 insertions(+)
>>>>> create mode 100644 Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt b/Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt
>>>>> new file mode 100644
>>>>> index 000000000000..8357f242ae4c
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt
>>>>> @@ -0,0 +1,35 @@
>>>>> +RTL8723BS/RTL8723CS Bluetooth
>>>>> +---------------------
>>>>> +
>>>>> +RTL8723CS/RTL8723CS is WiFi + BT chip. WiFi part is connected over SDIO, while
>>>>> +BT is connected over serial. It speaks H5 protocol with few extra commands
>>>>> +to upload firmware and change module speed.
>>>>> +
>>>>> +Required properties:
>>>>> +
>>>>> + - compatible: should be one of the following:
>>>>> + * "realtek,rtl8723bs-bt"
>>>>> + * "realtek,rtl8723cs-bt"
>>>>> +Optional properties:
>>>>> +
>>>>> + - device-wake-gpios: GPIO specifier, used to wakeup the BT module (active high)
>>>>> + - enable-gpios: GPIO specifier, used to enable the BT module (active high)
>>>>> + - host-wake-gpios: GPIO specifier, used to wakeup the host processor (active high)
>>>>> + - firmware-postfix: firmware postfix to be used for firmware config
>> sorry, i didn't noticed your great series before. David and i working at the same stuff but for the Asus Tinker Board.
>>
>> I created a similiar yet untested patch version for hci_h5 [1]. Maybe it's useful.
> Looks good to me, but you may need to add firmware-postfix.
>
>> Just a comment about the binding. It's really necessary to add the reset-gpio? Can't we use the enable-gpio with inverse polarity for this?
> Yes, we can use enable-gpio instead of reset-gpio on pine64 and pinebook.
>
>> Stefan
^ permalink raw reply
* Re: [PATCH 3/8] dt-bindings: net: bluetooth: Add rtl8723bs-bluetooth
From: Vasily Khoruzhick @ 2019-02-22 19:21 UTC (permalink / raw)
To: David Summers
Cc: Stefan Wahren, Rob Herring, Mark Rutland, devicetree,
Johan Hedberg, Maxime Ripard, netdev, Marcel Holtmann,
open list:BLUETOOTH DRIVERS, Chen-Yu Tsai, David S. Miller,
arm-linux
In-Reply-To: <8b97f349-4416-058c-0d01-3be592b89a7d@davidjohnsummers.uk>
On Fri, Feb 22, 2019 at 11:14 AM David Summers
<beagleboard@davidjohnsummers.uk> wrote:
>
> Hi Vasily,
>
> Just catching up with this series - good that you are doing it. We need
> it for users on ArchLinux ARM!
>
> On question though, what is "firmware-postfix" used for? I see in the
> documentation, that it points to the board name. But can't see where
> else it is used.
See h5_serdev_probe() in drivers/bluetooth/hci_h5.c. Basically it
specifies what firmware config to use.
I agree with Rob that we should probably use firmware-name here instead.
> Is there a need to have the board name?
As far as I understand firmware config depends on board, so I think
it's a good idea to use board name here.
> Anyway good work.
Thanks!
>
> David.
>
> On 18/02/2019 22:28, Vasily Khoruzhick wrote:
> > On Mon, Feb 18, 2019 at 2:08 PM Stefan Wahren <stefan.wahren@i2se.com> wrote:
> >> Hi Vasily,
> > Hi Stefan,
> >
> >>> Vasily Khoruzhick <anarsoul@gmail.com> hat am 18. Februar 2019 um 22:24 geschrieben:
> >>>
> >>>
> >>> On Mon, Feb 18, 2019 at 1:10 PM Rob Herring <robh@kernel.org> wrote:
> >>>> On Fri, Jan 18, 2019 at 09:02:27AM -0800, Vasily Khoruzhick wrote:
> >>>>> Add binding document for bluetooth part of RTL8723BS/RTL8723CS
> >>>>>
> >>>>> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
> >>>>> ---
> >>>>> .../bindings/net/rtl8723bs-bluetooth.txt | 35 +++++++++++++++++++
> >>>>> 1 file changed, 35 insertions(+)
> >>>>> create mode 100644 Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt
> >>>>>
> >>>>> diff --git a/Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt b/Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt
> >>>>> new file mode 100644
> >>>>> index 000000000000..8357f242ae4c
> >>>>> --- /dev/null
> >>>>> +++ b/Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt
> >>>>> @@ -0,0 +1,35 @@
> >>>>> +RTL8723BS/RTL8723CS Bluetooth
> >>>>> +---------------------
> >>>>> +
> >>>>> +RTL8723CS/RTL8723CS is WiFi + BT chip. WiFi part is connected over SDIO, while
> >>>>> +BT is connected over serial. It speaks H5 protocol with few extra commands
> >>>>> +to upload firmware and change module speed.
> >>>>> +
> >>>>> +Required properties:
> >>>>> +
> >>>>> + - compatible: should be one of the following:
> >>>>> + * "realtek,rtl8723bs-bt"
> >>>>> + * "realtek,rtl8723cs-bt"
> >>>>> +Optional properties:
> >>>>> +
> >>>>> + - device-wake-gpios: GPIO specifier, used to wakeup the BT module (active high)
> >>>>> + - enable-gpios: GPIO specifier, used to enable the BT module (active high)
> >>>>> + - host-wake-gpios: GPIO specifier, used to wakeup the host processor (active high)
> >>>>> + - firmware-postfix: firmware postfix to be used for firmware config
> >> sorry, i didn't noticed your great series before. David and i working at the same stuff but for the Asus Tinker Board.
> >>
> >> I created a similiar yet untested patch version for hci_h5 [1]. Maybe it's useful.
> > Looks good to me, but you may need to add firmware-postfix.
> >
> >> Just a comment about the binding. It's really necessary to add the reset-gpio? Can't we use the enable-gpio with inverse polarity for this?
> > Yes, we can use enable-gpio instead of reset-gpio on pine64 and pinebook.
> >
> >> Stefan
>
>
^ permalink raw reply
* Re: [PATCH] sctp: don't compare hb_timer expire date before starting it
From: David Miller @ 2019-02-22 19:22 UTC (permalink / raw)
To: kwiecienmaciek; +Cc: linux-sctp, netdev, alexander.sverdlin, maciej.kwiecien
In-Reply-To: <20190222084526.8214-1-maciej.kwiecien@nokia.com>
From: kwiecienmaciek@gmail.com
Date: Fri, 22 Feb 2019 09:45:26 +0100
> From: Maciej Kwiecien <maciej.kwiecien@nokia.com>
>
> hb_timer might not start at all for a particular transport because its
> start is conditional. In a result a node is not sending heartbeats.
>
> Function sctp_transport_reset_hb_timer has two roles:
> - initial start of hb_timer for a given transport,
> - update expire date of hb_timer for a given transport.
> The function is optimized to update timer's expire only if it is before
> a new calculated one but this comparison is invalid for a timer which
> has not yet started. Such a timer has expire == 0 and if a new expire
> value is bigger than (MAX_JIFFIES / 2 + 2) then "time_before" macro will
> fail and timer will not start resulting in no heartbeat packets send by
> the node.
>
> This was found when association was initialized within first 5 mins
> after system boot due to jiffies init value which is near to MAX_JIFFIES.
>
> Test kernel version: 4.9.154 (ARCH=arm)
> hb_timer.expire = 0; //initialized, not started timer
> new_expire = MAX_JIFFIES / 2 + 2; //or more
> time_before(hb_timer.expire, new_expire) == false
>
> Fixes: ba6f5e33bdbb ("sctp: avoid refreshing heartbeat timer too often")
> Reported-by: Marcin Stojek <marcin.stojek@nokia.com>
> Tested-by: Marcin Stojek <marcin.stojek@nokia.com>
> Signed-off-by: Maciej Kwiecien <maciej.kwiecien@nokia.com>
Applied and queued up for -stable, thank you.
^ permalink raw reply
* Re: [PATCH v2] kcm: remove any offset before parsing messages
From: Tom Herbert @ 2019-02-22 19:24 UTC (permalink / raw)
To: Dominique Martinet
Cc: Tom Herbert, David Miller, Doron Roberts-Kedes, Dave Watson,
Linux Kernel Network Developers, LKML
In-Reply-To: <20190221082209.GA32719@nautica>
On Thu, Feb 21, 2019 at 12:22 AM Dominique Martinet
<asmadeus@codewreck.org> wrote:
>
> Tom Herbert wrote on Wed, Feb 20, 2019:
> > > When the client closes the socket, some messages are obviously still "in
> > > flight", and the server will recv a POLLERR notification on the csock at
> > > some point with many messages left.
> > > The documentation says to unattach the csock when you get POLLER. If I
> > > do that, the kcm socket will no longer give me any message, so all the
> > > messages still in flight at the time are lost.
> >
> > So basically it sounds like you're interested in supporting TCP
> > connections that are half closed. I believe that the error in half
> > closed is EPIPE, so if the TCP socket returns that it can be ignored
> > and the socket can continue being attached and used to send data.
>
> Did you mean 'can continue being attached and used to receive data'?
>
No, I meant shutdown on receive side when FIN is receved. TX is still
allowed to drain an queued bytes. To support shutdown on the TX side
would require additional logic since we need to effectively detach the
transmit path but retain the receive path. I'm not sure this is a
compelling use case to support.
> I can confirm getsockopt with SO_ERROR gets me EPIPE, but I don't see
> how to efficiently ignore EPIPE until POLLIN gets unset -- polling on
> both the csock and kcm socket will do many needless wakeups on only the
> csock from what I can see, so I'd need some holdoff timer or something.
> I guess it's possible though.
We might need to clear the error somehow. May a read of zero bytes?
>
> > Another possibility is to add some linger semantics to an attached
> > socket. For instance, a large message might be sent so that part of
> > the messge is queued in TCP and part is queued in the KCM socket.
> > Unattach would probably break that message. We probably want to linger
> > option similar to SO_LINGER (or maybe just use the option on the TCP
> > socket) that means don't complete the detach until any message being
> > transmitted on the lower socket has been queued.
>
> That would certainly work, even if non-obvious from a user standpoint.
>
>
> > > > I'd like to see some retry on ENOMEM before this is merged though, so
> > > > while I'm there I'll resend this with a second patch doing that
> > > > retry,.. I think just not setting strp->interrupted and not reporting
> > > > the error up might be enough? Will have to try either way.
> > >
> > > I also tried playing with that without much success.
> > > I had assumed just not calling strp_parser_err() (which calls the
> > > abort_parser cb) would be enough, eventually calling strp_start_timer()
> > > like the !len case, but no can do.
> >
> > I think you need to ignore the ENOMEM and have a timer or other
> > callback to retry the operation in the future.
>
> Yes, that's what I had intended to try; basically just break out and
> schedule timer as said above.
You might want to look at some other systems, I don't recall if
there's a hook that can be used for when memory pressure is relieved.
>
> After a bit more debugging, this part works (__strp_recv() is called
> again); but the next packet that is treated properly is rejected because
> by the time __strp_recv() was called again a new skb was read and the
> length isn't large enough to go all the way into the new packet, so this
> test fails:
> } else if (len <= (ssize_t)head->len -
> skb->len - stm->strp.offset) {
> /* Length must be into new skb (and also
> * greater than zero)
> */
> STRP_STATS_INCR(strp->stats.bad_hdr_len);
> strp_parser_err(strp, -EPROTO, desc);
>
> So I need to figure a way to say "call this function again without
> reading more data" somehow, or make this check more lax e.g. accept any
> len > 0 after a retry maybe...
> Removing that branch altogether seems to work at least but I'm not sure
> we'd want to?
I like the check since it's conservative and covers the normal case.
Maybe just need some more logic?
> (grmbl at this slow VM and strparser not being possible to enable as a
> module, it takes ages to test)
>
>
> > > With that said, returning 0 from the parse function also raises POLLERR
> > > on the csock and hangs netparser, so things aren't that simple...
> >
> > Can you point to where this is happening. If the parse_msg callback
> > returns 0 that is suppose to indicate that more bytes are needed.
>
> I just blindly returned 0 "from time to time" in the kcm parser
> function, but looking at above it was failing on the same check.
> This somewhat makes sense for this one to fail here if a new packet was
> read, no sane parser function should give a length smaller than what
> they require to determine the length.
>
>
> Thanks,
> --
> Dominique
^ permalink raw reply
* Re: [PATCH 1/2 v2] kprobe: Do not use uaccess functions to access kernel memory that can fault
From: Alexei Starovoitov @ 2019-02-22 19:27 UTC (permalink / raw)
To: Linus Torvalds
Cc: Masami Hiramatsu, Steven Rostedt, Andy Lutomirski,
Linux List Kernel Mailing, Ingo Molnar, Andrew Morton, stable,
Changbin Du, Jann Horn, Kees Cook, Andy Lutomirski, daniel,
netdev, bpf
In-Reply-To: <CAHk-=whNf_n1WXWW+ugAVeL5ZK0GcEP3cTYocju1nS85VtMjjQ@mail.gmail.com>
On Fri, Feb 22, 2019 at 09:43:14AM -0800, Linus Torvalds wrote:
>
> Then we should still probably fix up "__probe_kernel_read()" to not
> allow user accesses. The easiest way to do that is actually likely to
> use the "unsafe_get_user()" functions *without* doing a
> uaccess_begin(), which will mean that modern CPU's will simply fault
> on a kernel access to user space.
On bpf side the bpf_probe_read() helper just calls probe_kernel_read()
and users pass both user and kernel addresses into it and expect
that the helper will actually try to read from that address.
If __probe_kernel_read will suddenly start failing on all user addresses
it will break the expectations.
How do we solve it in bpf_probe_read?
Call probe_kernel_read and if that fails call unsafe_get_user byte-by-byte
in the loop?
That's doable, but people already complain that bpf_probe_read() is slow
and shows up in their perf report.
^ 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