* Re: [PATCH bpf-next v4 0/2] libbpf: adding AF_XDP support
From: Magnus Karlsson @ 2019-02-13 11:32 UTC (permalink / raw)
To: Jonathan Lemon
Cc: Magnus Karlsson, Björn Töpel, ast, Daniel Borkmann,
Network Development, Jakub Kicinski, Björn Töpel,
Zhang, Qi Z, Jesper Dangaard Brouer, xiaolong.ye
In-Reply-To: <36557463-D23A-432E-AA18-7731F43CEBA6@gmail.com>
On Mon, Feb 11, 2019 at 9:44 PM Jonathan Lemon <jonathan.lemon@gmail.com> wrote:
>
> On 8 Feb 2019, at 5:05, Magnus Karlsson wrote:
>
> > This patch proposes to add AF_XDP support to libbpf. The main reason
> > for this is to facilitate writing applications that use AF_XDP by
> > offering higher-level APIs that hide many of the details of the AF_XDP
> > uapi. This is in the same vein as libbpf facilitates XDP adoption by
> > offering easy-to-use higher level interfaces of XDP
> > functionality. Hopefully this will facilitate adoption of AF_XDP, make
> > applications using it simpler and smaller, and finally also make it
> > possible for applications to benefit from optimizations in the AF_XDP
> > user space access code. Previously, people just copied and pasted the
> > code from the sample application into their application, which is not
> > desirable.
>
> I like the idea of encapsulating the boilerplate logic in a library.
>
> I do think there is an important missing piece though - there should be
> some code which queries the netdev for how many queues are attached, and
> create the appropriate number of umem/AF_XDP sockets.
>
> I ran into this issue when testing the current AF_XDP code - on my test
> boxes, the mlx5 card has 55 channels (aka queues), so when the test program
> binds only to channel 0, nothing works as expected, since not all traffic
> is being intercepted. While obvious in hindsight, this took a while to
> track down.
Yes, agreed. You are not the first one to stumble upon this problem
:-). Let me think a little bit on how to solve this in a good way. We
need this to be simple and intuitive, as you say.
/Magnus
> --
> Jonathan
^ permalink raw reply
* Re: [PATCH RFC v2 3/3] udp: Support UDP fraglist GRO/GSO.
From: Steffen Klassert @ 2019-02-13 11:48 UTC (permalink / raw)
To: Willem de Bruijn
Cc: Network Development, Willem de Bruijn, Paolo Abeni,
Jason A. Donenfeld
In-Reply-To: <CAF=yD-+HfQ8Q=KCTq3UKqK2UOHaKFt7ztnbnyKiUutt_-Cr0EQ@mail.gmail.com>
On Mon, Jan 28, 2019 at 02:49:32PM -0600, Willem de Bruijn wrote:
> On Mon, Jan 28, 2019 at 2:51 AM Steffen Klassert
> <steffen.klassert@secunet.com> wrote:
> >
> > This patch extends UDP GRO to support fraglist GRO/GSO
> > by using the previously introduced infrastructure.
> > All UDP packets that are not targeted to a GRO capable
> > UDP sockets are going to fraglist GRO now (local input
> > and forward).
> > ---
> > net/ipv4/udp_offload.c | 45 ++++++++++++++++++++++++++++++++++++++----
> > net/ipv6/udp_offload.c | 9 +++++++++
> > 2 files changed, 50 insertions(+), 4 deletions(-)
> >
> > diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
> > index 584635db9231..c0be33216750 100644
> > --- a/net/ipv4/udp_offload.c
> > +++ b/net/ipv4/udp_offload.c
> > @@ -188,6 +188,22 @@ struct sk_buff *skb_udp_tunnel_segment(struct sk_buff *skb,
> > }
> > EXPORT_SYMBOL(skb_udp_tunnel_segment);
> >
> > +static struct sk_buff *__udp_gso_segment_list(struct sk_buff *skb,
> > + netdev_features_t features)
> > +{
> > + unsigned int mss = skb_shinfo(skb)->gso_size;
> > +
> > + skb = skb_segment_list(skb, features, skb_mac_header_len(skb));
> > + if (IS_ERR(skb))
> > + return skb;
> > +
> > + udp_hdr(skb)->len = htons(sizeof(struct udphdr) + mss);
> > + skb->ip_summed = CHECKSUM_NONE;
> > + skb->csum_valid = 1;
>
> csum_valid is only used on ingress.
>
> Hardcoding CHECKSUM_NONE is probably fine as long as this function is
> only used for forwarding, assuming we don't care about verifiying
> checksums in the forwarding case.
>
> But this is fragile if we ever add local list segment output. Should
> convert the checksum field in skb_forward_csum, instead of at the GSO
> layer, just as for forwarding of non list skbs? Though that would
> require traversing the list yet another time. Other option is to
> already do this conversion when building the list in GRO.
>
> The comment also applies to the same logic in skb_segment_list. As a
> matter or fact, even if this belongs in GSO instead of core forwarding
> or GRO, then probably both the list head and frag_list skbs should be
> set in the same function, so skb_segment_list.
I had a deeper look into this now, it seems that the head skb has
already the correct conversion (either for local input or forwarding).
So we probaply just need to adjust the frag_list skbs to the
head skb checksum conversion.
> > /* pull encapsulating udp header */
> > skb_gro_pull(skb, sizeof(struct udphdr));
> > - skb_gro_postpull_rcsum(skb, uh, sizeof(struct udphdr));
> >
> > list_for_each_entry(p, head, list) {
> > if (!NAPI_GRO_CB(p)->same_flow)
> > @@ -379,8 +397,17 @@ static struct sk_buff *udp_gro_receive_segment(struct list_head *head,
> > * Under small packet flood GRO count could elsewhere grow a lot
> > * leading to execessive truesize values
> > */
> > - if (!skb_gro_receive(p, skb) &&
> > - NAPI_GRO_CB(p)->count >= UDP_GRO_CNT_MAX)
> > + if (NAPI_GRO_CB(skb)->is_flist) {
> > + if (!pskb_may_pull(skb, skb_gro_offset(skb)))
> > + return NULL;
> > + ret = skb_gro_receive_list(p, skb);
> > + } else {
> > + skb_gro_postpull_rcsum(skb, uh, sizeof(struct udphdr));
> > +
> > + ret = skb_gro_receive(p, skb);
> > + }
> > +
> > + if (!ret && NAPI_GRO_CB(p)->count > UDP_GRO_CNT_MAX)
> > pp = p;
> > else if (uh->len != uh2->len)
> > pp = p;
> > @@ -402,6 +429,7 @@ struct sk_buff *udp_gro_receive(struct list_head *head, struct sk_buff *skb,
> > int flush = 1;
> >
> > if (!sk || !udp_sk(sk)->gro_receive) {
> > + NAPI_GRO_CB(skb)->is_flist = sk ? !udp_sk(sk)->gro_enabled: 1;
>
> This updates the choice of whether to use a list on each received skb.
> Which is problematic as a socket can call the setsockopt in between
> packets.
>
> Actually, there no longer is a need for a route lookup for each skb at
> all. We always apply GRO, which was the previous reason for the
> lookup. And if a matching flow is found in the GRO table, we already
> the choice to use a list is already stored.
Yes, that's true. I'll change that.
^ permalink raw reply
* Re: [PATCH RFC v2 2/3] net: Support GRO/GSO fraglist chaining.
From: Steffen Klassert @ 2019-02-13 11:49 UTC (permalink / raw)
To: Willem de Bruijn
Cc: Network Development, Willem de Bruijn, Paolo Abeni,
Jason A. Donenfeld
In-Reply-To: <CAF=yD-+Tvyz8x+F+VnZXYDToW-kC2MuPG5Lcna2W+CQwTOMybQ@mail.gmail.com>
On Mon, Jan 28, 2019 at 02:50:34PM -0600, Willem de Bruijn wrote:
> On Mon, Jan 28, 2019 at 2:53 AM Steffen Klassert
> <steffen.klassert@secunet.com> wrote:
> > +
> > + if (skb_needs_linearize(skb, features) &&
> > + __skb_linearize(skb)) {
> > + skb->next = NULL;
> > + kfree_skb_list(skb->next);
>
> inverse order
Oh yes, apparently.
>
> also, I would probably deduplicate with the same branch above in a new
> err_linearize: block
Will do that.
Thanks for the review!
^ permalink raw reply
* Re: [PATCH bpf-next v4 0/2] libbpf: adding AF_XDP support
From: Jesper Dangaard Brouer @ 2019-02-13 11:55 UTC (permalink / raw)
To: Magnus Karlsson
Cc: Jonathan Lemon, Magnus Karlsson, Björn Töpel, ast,
Daniel Borkmann, Network Development, Jakub Kicinski,
Björn Töpel, Zhang, Qi Z, xiaolong.ye, brouer,
xdp-newbies@vger.kernel.org
In-Reply-To: <CAJ8uoz19UjmEHTc28Qd_9KdY9D-ojXSBRTbmffRhUTX49mnWvg@mail.gmail.com>
On Wed, 13 Feb 2019 12:32:47 +0100
Magnus Karlsson <magnus.karlsson@gmail.com> wrote:
> On Mon, Feb 11, 2019 at 9:44 PM Jonathan Lemon <jonathan.lemon@gmail.com> wrote:
> >
> > On 8 Feb 2019, at 5:05, Magnus Karlsson wrote:
> >
> > > This patch proposes to add AF_XDP support to libbpf. The main reason
> > > for this is to facilitate writing applications that use AF_XDP by
> > > offering higher-level APIs that hide many of the details of the AF_XDP
> > > uapi. This is in the same vein as libbpf facilitates XDP adoption by
> > > offering easy-to-use higher level interfaces of XDP
> > > functionality. Hopefully this will facilitate adoption of AF_XDP, make
> > > applications using it simpler and smaller, and finally also make it
> > > possible for applications to benefit from optimizations in the AF_XDP
> > > user space access code. Previously, people just copied and pasted the
> > > code from the sample application into their application, which is not
> > > desirable.
> >
> > I like the idea of encapsulating the boilerplate logic in a library.
> >
> > I do think there is an important missing piece though - there should be
> > some code which queries the netdev for how many queues are attached, and
> > create the appropriate number of umem/AF_XDP sockets.
> >
> > I ran into this issue when testing the current AF_XDP code - on my test
> > boxes, the mlx5 card has 55 channels (aka queues), so when the test program
> > binds only to channel 0, nothing works as expected, since not all traffic
> > is being intercepted. While obvious in hindsight, this took a while to
> > track down.
>
> Yes, agreed. You are not the first one to stumble upon this problem
> :-). Let me think a little bit on how to solve this in a good way. We
> need this to be simple and intuitive, as you say.
I see people hitting this with AF_XDP all the time... I had some
backup-slides[2] in our FOSDEM presentation[1] that describe the issue,
give the performance reason why and propose a workaround.
[1] https://github.com/xdp-project/xdp-project/tree/master/conference/FOSDEM2019
[2] https://github.com/xdp-project/xdp-project/blob/master/conference/FOSDEM2019/xdp_building_block.org#backup-slides
Alternative work-around
* Create as many AF_XDP sockets as RXQs
* Have userspace poll()/select on all sockets
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
* Backup Slides :export:
** Slide: Where does AF_XDP performance come from? :export:
/Lock-free [[https://lwn.net/Articles/169961/][channel]] directly from driver RX-queue into AF_XDP socket/
- Single-Producer/Single-Consumer (SPSC) descriptor ring queues
- *Single*-/Producer/ (SP) via bind to specific RX-*/queue id/*
* NAPI-softirq assures only 1-CPU process 1-RX-queue id (per sched)
- *Single*-/Consumer/ (SC) via 1-Application
- *Bounded* buffer pool (UMEM) allocated by userspace (register with kernel)
* Descriptor(s) in ring(s) point into UMEM
* /No memory allocation/, but return frames to UMEM in timely manner
- [[http://www.lemis.com/grog/Documentation/vj/lca06vj.pdf][Transport signature]] Van Jacobson talked about
* Replaced by XDP/eBPF program choosing to XDP_REDIRECT
** Slide: Details: Actually *four* SPSC ring queues :export:
AF_XDP /socket/: Has /two rings/: *RX* and *TX*
- Descriptor(s) in ring points into UMEM
/UMEM/ consists of a number of equally sized chunks
- Has /two rings/: *FILL* ring and *COMPLETION* ring
- FILL ring: application gives kernel area to RX fill
- COMPLETION ring: kernel tells app TX is done for area (can be reused)
** Slide: Gotcha by RX-queue id binding :export:
AF_XDP bound to */single RX-queue id/* (for SPSC performance reasons)
- NIC by default spreads flows with RSS-hashing over RX-queues
* Traffic likely not hitting queue you expect
- You *MUST* configure NIC *HW filters* to /steer to RX-queue id/
* Out of scope for XDP setup
* Use ethtool or TC HW offloading for filter setup
- *Alternative* work-around
* /Create as many AF_XDP sockets as RXQs/
* Have userspace poll()/select on all sockets
^ permalink raw reply
* Re: [ISSUE][4.20.6] mlx5 and checksum failures
From: Ian Kumlien @ 2019-02-13 12:04 UTC (permalink / raw)
To: Saeed Mahameed
Cc: Cong Wang, David Miller, Saeed Mahameed,
Linux Kernel Network Developers
In-Reply-To: <CAA85sZvw9DouvMLT8+TgS5Y-0kG-XSVbNmBweJVBjOSNQPovRw@mail.gmail.com>
One last update on this, 4.20.8 compiled with the same compiler works
- I still suspect that it was fixed by:
net/mlx5e: Force CHECKSUM_UNNECESSARY for short ethernet frames
Anyway, we can forget about it now ;)
On Sat, Feb 9, 2019 at 4:54 PM Ian Kumlien <ian.kumlien@gmail.com> wrote:
>
> On Fri, Feb 8, 2019 at 5:29 PM Ian Kumlien <ian.kumlien@gmail.com> wrote
> > On Thu, Feb 7, 2019 at 11:01 PM Ian Kumlien <ian.kumlien@gmail.com> wrote:
> > > On Thu, Feb 7, 2019 at 7:43 PM Saeed Mahameed <saeedm@dev.mellanox.co.il> wrote:
> > > > On Thu, Feb 7, 2019 at 2:17 AM Ian Kumlien <ian.kumlien@gmail.com> wrote:
> > > > > On Thu, Feb 7, 2019 at 2:01 AM Saeed Mahameed <saeedm@dev.mellanox.co.il> wrote:
> > > > > > On Wed, Feb 6, 2019 at 3:00 PM Ian Kumlien <ian.kumlien@gmail.com> wrote:
> > > > > > > It changes directly after the first hw checksum failure, I don't know why =/
> > > > > >
> > > > > > weird, Maybe a real check-summing issue/corruption on the PCI ?!
> > > > >
> > > > > Actually, it seems to have been introduced in 4.20.6 - 4.20.5 works just fine
> >
> > > > Great, the difference is only 120 patches.
> > > > that is bisect-able, it will only take 5 iterations to find the
> > > > offending commit.
> > >
> > > I just wish it wasn't a server that takes, what feels like 5 minutes to boot...
> > >
> > > All of these seas of sensors 2d and 3d... =P
> > >
> > > But, yep, that's the plan
> >
> > Huh, spent most of the day with two bisects and none of them yielded
> > any results....
> >
> > Looks like I'll have to start investigating the elrepo kernel-ml build =(
>
> Just realized that it's not an entirely fair comparison - since
> retpolines wasn't enabled, damned old compilers...
^ permalink raw reply
* Re: [PATCH] inet_diag: fix reporting cgroup classid and fallback to priority
From: Konstantin Khlebnikov @ 2019-02-13 12:28 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-kernel, sashal, linux-sctp
In-Reply-To: <20190212.103700.893095897141469287.davem@davemloft.net>
On 12.02.2019 21:37, David Miller wrote:
> From: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
> Date: Sat, 09 Feb 2019 13:35:52 +0300
>
>> Field idiag_ext in struct inet_diag_req_v2 used as bitmap of requested
>> extensions has only 8 bits. Thus extensions starting from DCTCPINFO
>> cannot be requested directly. Some of them included into response
>> unconditionally or hook into some of lower 8 bits.
>>
>> Extension INET_DIAG_CLASS_ID has not way to request from the beginning.
>>
>> This patch bundle it with INET_DIAG_TCLASS (ipv6 tos), fixes space
>> reservation, and documents behavior for other extensions.
>>
>> Also this patch adds fallback to reporting socket priority. This filed
>> is more widely used for traffic classification because ipv4 sockets
>> automatically maps TOS to priority and default qdisc pfifo_fast knows
>> about that. But priority could be changed via setsockopt SO_PRIORITY so
>> INET_DIAG_TOS isn't enough for predicting class.
>>
>> Also cgroup2 obsoletes net_cls classid (it always zero), but we cannot
>> reuse this field for reporting cgroup2 id because it is 64-bit (ino+gen).
>>
>> So, after this patch INET_DIAG_CLASS_ID will report socket priority
>> for most common setup when net_cls isn't set and/or cgroup2 in use.
>>
>> Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
>> Fixes: 0888e372c37f ("net: inet: diag: expose sockets cgroup classid")
>
> Applied, and queued up for -stable.
>
> Please always put the Fixes: tag first in the list of tags. I fixed
> it up for you this time.
Ok. Never heard about that rule, checkpatch.pl doesn't complain about that too.
>
> Thanks.
>
^ permalink raw reply
* [PATCH iproute2] ss: add option --tos for requesting ipv4 tos and ipv6 tclass
From: Konstantin Khlebnikov @ 2019-02-13 12:39 UTC (permalink / raw)
To: Stephen Hemminger, netdev; +Cc: Eric Dumazet
Also show socket class_id/priority used by classful qdisc.
Kernel report this together with tclass since commit
("inet_diag: fix reporting cgroup classid and fallback to priority")
Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
---
man/man8/ss.8 | 17 +++++++++++++++++
misc/ss.c | 27 +++++++++++++++++++++++++++
2 files changed, 44 insertions(+)
diff --git a/man/man8/ss.8 b/man/man8/ss.8
index 553a6cf46f0e..9f21202de424 100644
--- a/man/man8/ss.8
+++ b/man/man8/ss.8
@@ -244,6 +244,23 @@ the pacing rate and max pacing rate
a helper variable for TCP internal auto tuning socket receive buffer
.RE
.TP
+.B \-\-tos
+Show ToS and priority information. Below fields may appear:
+.RS
+.P
+.TP
+.B tos
+IPv4 Type-of-Service byte
+.P
+.TP
+.B tclass
+IPv6 Traffic Class byte
+.P
+.TP
+.B class_id
+Class id set by net_cls cgroup. If class is zero this shows priority set by SO_PRIORITY.
+.RE
+.TP
.B \-K, \-\-kill
Attempts to forcibly close sockets. This option displays sockets that are
successfully closed and silently skips sockets that the kernel does not support
diff --git a/misc/ss.c b/misc/ss.c
index 3589ebedc5a0..9e821faf0d31 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -110,6 +110,7 @@ static int show_header = 1;
static int follow_events;
static int sctp_ino;
static int show_tipcinfo;
+static int show_tos;
enum col_id {
COL_NETID,
@@ -3008,6 +3009,15 @@ static int inet_show_sock(struct nlmsghdr *nlh,
}
}
+ if (show_tos) {
+ if (tb[INET_DIAG_TOS])
+ out(" tos:%#x", rta_getattr_u8(tb[INET_DIAG_TOS]));
+ if (tb[INET_DIAG_TCLASS])
+ out(" tclass:%#x", rta_getattr_u8(tb[INET_DIAG_TCLASS]));
+ if (tb[INET_DIAG_CLASS_ID])
+ out(" class_id:%#x", rta_getattr_u32(tb[INET_DIAG_CLASS_ID]));
+ }
+
if (show_mem || (show_tcpinfo && s->type != IPPROTO_UDP)) {
out("\n\t");
if (s->type == IPPROTO_SCTP)
@@ -3058,6 +3068,11 @@ static int tcpdiag_send(int fd, int protocol, struct filter *f)
req.r.idiag_ext |= (1<<(INET_DIAG_CONG-1));
}
+ if (show_tos) {
+ req.r.idiag_ext |= (1<<(INET_DIAG_TOS-1));
+ req.r.idiag_ext |= (1<<(INET_DIAG_TCLASS-1));
+ }
+
iov[0] = (struct iovec){
.iov_base = &req,
.iov_len = sizeof(req)
@@ -3118,6 +3133,11 @@ static int sockdiag_send(int family, int fd, int protocol, struct filter *f)
req.r.idiag_ext |= (1<<(INET_DIAG_CONG-1));
}
+ if (show_tos) {
+ req.r.idiag_ext |= (1<<(INET_DIAG_TOS-1));
+ req.r.idiag_ext |= (1<<(INET_DIAG_TCLASS-1));
+ }
+
iov[0] = (struct iovec){
.iov_base = &req,
.iov_len = sizeof(req)
@@ -4661,6 +4681,7 @@ static void _usage(FILE *dest)
" -i, --info show internal TCP information\n"
" --tipcinfo show internal tipc socket information\n"
" -s, --summary show socket usage summary\n"
+" --tos show tos and priority information\n"
" -b, --bpf show bpf filter socket information\n"
" -E, --events continually display sockets as they are destroyed\n"
" -Z, --context display process SELinux security contexts\n"
@@ -4765,6 +4786,8 @@ static int scan_state(const char *state)
#define OPT_TIPCSOCK 257
#define OPT_TIPCINFO 258
+#define OPT_TOS 259
+
static const struct option long_opts[] = {
{ "numeric", 0, 0, 'n' },
{ "resolve", 0, 0, 'r' },
@@ -4800,6 +4823,7 @@ static const struct option long_opts[] = {
{ "contexts", 0, 0, 'z' },
{ "net", 1, 0, 'N' },
{ "tipcinfo", 0, 0, OPT_TIPCINFO},
+ { "tos", 0, 0, OPT_TOS },
{ "kill", 0, 0, 'K' },
{ "no-header", 0, 0, 'H' },
{ 0 }
@@ -4977,6 +5001,9 @@ int main(int argc, char *argv[])
case OPT_TIPCINFO:
show_tipcinfo = 1;
break;
+ case OPT_TOS:
+ show_tos = 1;
+ break;
case 'K':
current_filter.kill = 1;
break;
^ permalink raw reply related
* Re: [PATCH 2/2] mac80211: Free mpath object when rhashtable insertion fails (fwd)
From: Julia Lawall @ 2019-02-13 12:50 UTC (permalink / raw)
To: Herbert Xu
Cc: David Miller, johannes, linux-wireless, netdev, j, tgraf,
johannes.berg, kbuild-all
[-- Attachment #1: Type: text/plain, Size: 6385 bytes --]
Perhaps a return is needed after line 436.
julia
---------- Forwarded message ----------
Date: Wed, 13 Feb 2019 20:29:38 +0800
From: kbuild test robot <lkp@intel.com>
To: kbuild@01.org
Cc: Julia Lawall <julia.lawall@lip6.fr>
Subject: Re: [PATCH 2/2] mac80211: Free mpath object when rhashtable insertion
fails
CC: kbuild-all@01.org
In-Reply-To: <E1gtmu6-0001Vl-O7@gondobar>
References: <E1gtmu6-0001Vl-O7@gondobar>
TO: Herbert Xu <herbert@gondor.apana.org.au>
CC: David Miller <davem@davemloft.net>, johannes@sipsolutions.net, linux-wireless@vger.kernel.org, netdev@vger.kernel.org, j@w1.fi, tgraf@suug.ch, johannes.berg@intel.com
CC:
Hi Herbert,
I love your patch! Perhaps something to improve:
[auto build test WARNING on mac80211-next/master]
[also build test WARNING on v5.0-rc4 next-20190212]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Herbert-Xu/mac80211-Fix-incorrect-usage-of-rhashtable-walk-API/20190213-181325
base: https://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next.git master
:::::: branch date: 2 hours ago
:::::: commit date: 2 hours ago
>> net/mac80211/mesh_pathtbl.c:448:8-17: ERROR: reference preceded by free on line 436
# https://github.com/0day-ci/linux/commit/a0886e834aacf883ceaf0c34c842c4cdb4d318fd
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout a0886e834aacf883ceaf0c34c842c4cdb4d318fd
vim +448 net/mac80211/mesh_pathtbl.c
b15dc38b9 Bob Copeland 2016-02-28 390
eb2b9311f Luis Carlos Cobo 2008-02-23 391 /**
eb2b9311f Luis Carlos Cobo 2008-02-23 392 * mesh_path_add - allocate and add a new path to the mesh path table
bf7cd94dc Johannes Berg 2013-02-15 393 * @dst: destination address of the path (ETH_ALEN length)
f698d856f Jasper Bryant-Greene 2008-08-03 394 * @sdata: local subif
eb2b9311f Luis Carlos Cobo 2008-02-23 395 *
af901ca18 André Goddard Rosa 2009-11-14 396 * Returns: 0 on success
eb2b9311f Luis Carlos Cobo 2008-02-23 397 *
eb2b9311f Luis Carlos Cobo 2008-02-23 398 * State: the initial state of the new path is set to 0
eb2b9311f Luis Carlos Cobo 2008-02-23 399 */
ae76eef02 Bob Copeland 2013-03-29 400 struct mesh_path *mesh_path_add(struct ieee80211_sub_if_data *sdata,
ae76eef02 Bob Copeland 2013-03-29 401 const u8 *dst)
eb2b9311f Luis Carlos Cobo 2008-02-23 402 {
349eb8cf4 Johannes Berg 2011-05-14 403 struct mesh_table *tbl;
eb2b9311f Luis Carlos Cobo 2008-02-23 404 struct mesh_path *mpath, *new_mpath;
60854fd94 Bob Copeland 2016-03-02 405 int ret;
eb2b9311f Luis Carlos Cobo 2008-02-23 406
b203ca391 Joe Perches 2012-05-08 407 if (ether_addr_equal(dst, sdata->vif.addr))
eb2b9311f Luis Carlos Cobo 2008-02-23 408 /* never add ourselves as neighbours */
ae76eef02 Bob Copeland 2013-03-29 409 return ERR_PTR(-ENOTSUPP);
eb2b9311f Luis Carlos Cobo 2008-02-23 410
eb2b9311f Luis Carlos Cobo 2008-02-23 411 if (is_multicast_ether_addr(dst))
ae76eef02 Bob Copeland 2013-03-29 412 return ERR_PTR(-ENOTSUPP);
eb2b9311f Luis Carlos Cobo 2008-02-23 413
472dbc45d Johannes Berg 2008-09-11 414 if (atomic_add_unless(&sdata->u.mesh.mpaths, 1, MESH_MAX_MPATHS) == 0)
ae76eef02 Bob Copeland 2013-03-29 415 return ERR_PTR(-ENOSPC);
ae76eef02 Bob Copeland 2013-03-29 416
b15dc38b9 Bob Copeland 2016-02-28 417 new_mpath = mesh_path_new(sdata, dst, GFP_ATOMIC);
402d7752e Pavel Emelyanov 2008-05-06 418 if (!new_mpath)
60854fd94 Bob Copeland 2016-03-02 419 return ERR_PTR(-ENOMEM);
402d7752e Pavel Emelyanov 2008-05-06 420
60854fd94 Bob Copeland 2016-03-02 421 tbl = sdata->u.mesh.mesh_paths;
60854fd94 Bob Copeland 2016-03-02 422 do {
60854fd94 Bob Copeland 2016-03-02 423 ret = rhashtable_lookup_insert_fast(&tbl->rhead,
60854fd94 Bob Copeland 2016-03-02 424 &new_mpath->rhash,
60854fd94 Bob Copeland 2016-03-02 425 mesh_rht_params);
f84e71a94 Pavel Emelyanov 2008-05-06 426
60854fd94 Bob Copeland 2016-03-02 427 if (ret == -EEXIST)
60854fd94 Bob Copeland 2016-03-02 428 mpath = rhashtable_lookup_fast(&tbl->rhead,
60854fd94 Bob Copeland 2016-03-02 429 dst,
60854fd94 Bob Copeland 2016-03-02 430 mesh_rht_params);
05b0152f1 Herbert Xu 2019-02-13 431 else if (!ret)
05b0152f1 Herbert Xu 2019-02-13 432 hlist_add_head(&new_mpath->walk_list, &tbl->walk_head);
60854fd94 Bob Copeland 2016-03-02 433 } while (unlikely(ret == -EEXIST && !mpath));
f5ea9120b Johannes Berg 2009-08-07 434
a0886e834 Herbert Xu 2019-02-13 435 if (ret)
a0886e834 Herbert Xu 2019-02-13 @436 kfree(new_mpath);
a0886e834 Herbert Xu 2019-02-13 437
a0886e834 Herbert Xu 2019-02-13 438 if (ret != -EEXIST)
60854fd94 Bob Copeland 2016-03-02 439 return ERR_PTR(ret);
ae76eef02 Bob Copeland 2013-03-29 440
60854fd94 Bob Copeland 2016-03-02 441 /* At this point either new_mpath was added, or we found a
60854fd94 Bob Copeland 2016-03-02 442 * matching entry already in the table; in the latter case
60854fd94 Bob Copeland 2016-03-02 443 * free the unnecessary new entry.
60854fd94 Bob Copeland 2016-03-02 444 */
a0886e834 Herbert Xu 2019-02-13 445 if (ret == -EEXIST)
60854fd94 Bob Copeland 2016-03-02 446 new_mpath = mpath;
60854fd94 Bob Copeland 2016-03-02 447 sdata->u.mesh.mesh_paths_generation++;
60854fd94 Bob Copeland 2016-03-02 @448 return new_mpath;
18889231e Javier Cardona 2009-08-10 449 }
eb2b9311f Luis Carlos Cobo 2008-02-23 450
:::::: The code at line 448 was first introduced by commit
:::::: 60854fd94573f0d3b80b55b40cf0140a0430f3ab mac80211: mesh: convert path table to rhashtable
:::::: TO: Bob Copeland <me@bobcopeland.com>
:::::: CC: Johannes Berg <johannes.berg@intel.com>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
^ permalink raw reply
* Re: [PATCH] net: phy: at803x: disable delay only for RGMII mode
From: Niklas Cassel @ 2019-02-13 13:12 UTC (permalink / raw)
To: Vinod Koul
Cc: David S Miller, linux-arm-msm, Bjorn Andersson, netdev,
Andrew Lunn, Florian Fainelli, Nori, Sekhar, Peter Ujfalusi,
Marc Gonzalez
In-Reply-To: <20190212141922.12849-1-vkoul@kernel.org>
On Tue, Feb 12, 2019 at 07:49:22PM +0530, Vinod Koul wrote:
> Per "Documentation/devicetree/bindings/net/ethernet.txt" RGMII mode
> should not have delay in PHY whereas RGMII_ID and RGMII_RXID/RGMII_TXID
> can have delay in phy.
>
> So disable the delay only for RGMII mode and disable for other modes
>
> Fixes: cd28d1d6e52e: ("net: phy: at803x: Disable phy delay for RGMII mode")
> Reported-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> Signed-off-by: Vinod Koul <vkoul@kernel.org>
> ---
> drivers/net/phy/at803x.c | 33 ++++++++++++++++++++++++++-------
> 1 file changed, 26 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
> index 8ff12938ab47..7b54b54e3316 100644
> --- a/drivers/net/phy/at803x.c
> +++ b/drivers/net/phy/at803x.c
> @@ -110,6 +110,18 @@ static int at803x_debug_reg_mask(struct phy_device *phydev, u16 reg,
> return phy_write(phydev, AT803X_DEBUG_DATA, val);
> }
>
> +static inline int at803x_enable_rx_delay(struct phy_device *phydev)
> +{
> + return at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_0, 0,
> + AT803X_DEBUG_RX_CLK_DLY_EN);
> +}
> +
> +static inline int at803x_enable_tx_delay(struct phy_device *phydev)
> +{
> + return at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_5, 0,
> + AT803X_DEBUG_TX_CLK_DLY_EN);
> +}
> +
> static inline int at803x_disable_rx_delay(struct phy_device *phydev)
> {
> return at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_0,
> @@ -255,18 +267,25 @@ static int at803x_config_init(struct phy_device *phydev)
> if (ret < 0)
> return ret;
>
> - if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID ||
> - phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
> - phydev->interface == PHY_INTERFACE_MODE_RGMII) {
> + if (phydev->interface == PHY_INTERFACE_MODE_RGMII) {
> ret = at803x_disable_rx_delay(phydev);
> if (ret < 0)
> return ret;
> + ret = at803x_disable_tx_delay(phydev);
> + if (ret < 0)
> + return ret;
> + };
> +
> + if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
> + phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID) {
> + ret = at803x_enable_rx_delay(phydev);
> + if (ret < 0)
> + return ret;
> }
>
> - if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID ||
> - phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
> - phydev->interface == PHY_INTERFACE_MODE_RGMII) {
> - ret = at803x_disable_tx_delay(phydev);
> + if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
> + phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID) {
> + ret = at803x_enable_tx_delay(phydev);
> if (ret < 0)
> return ret;
> }
So we have these modes:
PHY_INTERFACE_MODE_RGMII: TX and RX delays disabled
PHY_INTERFACE_MODE_RGMII_ID: TX and RX delays enabled
PHY_INTERFACE_MODE_RGMII_RXID: RX delay enabled, TX delay disabled
PHY_INTERFACE_MODE_RGMII_TXID: TX delay enabled, RX delay disabled
What I don't like with this patch, is that if we specify phy-mode
PHY_INTERFACE_MODE_RGMII_TXID, this patch will enable TX delay,
but RX delay will not be explicitly set.
Thus it will use the default value of the PHY, and for this PHY
both TX and RX delays are enabled by default.
This means that someone specifying PHY_INTERFACE_MODE_RGMII_TXID
will actually be getting PHY_INTERFACE_MODE_RGMII_ID.
Marc's patch:
https://www.spinics.net/lists/netdev/msg445053.html
does explicitly either enable or disable each delay
(so it does not depend on default values).
However, Marc's patch was never merged, because someone reported
a regression on am335x-evm.
On Vinod's V1 submission Andrew replied that if this change
breaks some existing DT (because that DT specifies a phy-mode
that does not match with reality), then we should fix so that
DT specifies the correct phy-mode:
https://www.spinics.net/lists/netdev/msg542638.html
IMHO, this makes most sense, since if a DT specifies an
incorrect phy-mode, then that is what the user should get.
Kind regards,
Niklas
^ permalink raw reply
* Re: [PATCH net] net: dsa: bcm_sf2: potential array overflow in bcm_sf2_sw_suspend()
From: Vivien Didelot @ 2019-02-13 13:13 UTC (permalink / raw)
To: Dan Carpenter
Cc: Andrew Lunn, Florian Fainelli, David S. Miller, netdev,
kernel-janitors
In-Reply-To: <20190213082304.GA14113@kadam>
On Wed, 13 Feb 2019 11:23:04 +0300, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> The value of ->num_ports comes from bcm_sf2_sw_probe() and it is less
> than or equal to DSA_MAX_PORTS. The ds->ports[] array is used inside
> the dsa_is_user_port() and dsa_is_cpu_port() functions. The ds->ports[]
> array is allocated in dsa_switch_alloc() and it has ds->num_ports
> elements so this leads to a static checker warning about a potential out
> of bounds read.
>
> Fixes: 8cfa94984c9c ("net: dsa: bcm_sf2: add suspend/resume callbacks")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Reviewed-by: Vivien Didelot <vivien.didelot@gmail.com>
^ permalink raw reply
* Re: [PATCH 1/3 net-next] lib: objagg: Fix an error code in objagg_hints_get()
From: Jiri Pirko @ 2019-02-13 13:15 UTC (permalink / raw)
To: Dan Carpenter; +Cc: Jiri Pirko, netdev, Ido Schimmel, kernel-janitors
In-Reply-To: <20190213085650.GB14113@kadam>
Wed, Feb 13, 2019 at 09:56:50AM CET, dan.carpenter@oracle.com wrote:
>We need to set the error code on this path otherwise we return
>ERR_PTR(0) which would result in a NULL dereference in the caller.
>
>Fixes: 9069a3817d82 ("lib: objagg: implement optimization hints assembly and use hints for object creation")
>Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
I have the same patch in my queue.
Acked-by: Jiri Pirko <jiri@mellanox.com>
^ permalink raw reply
* Re: [PATCH 2/3 net-next] test_objagg: Test the correct variable
From: Jiri Pirko @ 2019-02-13 13:16 UTC (permalink / raw)
To: Dan Carpenter; +Cc: Jiri Pirko, netdev, kernel-janitors
In-Reply-To: <20190213085820.GC14113@kadam>
Wed, Feb 13, 2019 at 09:58:20AM CET, dan.carpenter@oracle.com wrote:
>There is a typo here. We intended to check "objagg2" but we instead
>test "objagg" which is not an error pointer.
>
>Fixes: 9069a3817d82 ("lib: objagg: implement optimization hints assembly and use hints for object creation")
>Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
Thanks.
^ permalink raw reply
* Re: [PATCH 3/3 net-next] test_objagg: Uninitialized variable in error handling
From: Jiri Pirko @ 2019-02-13 13:16 UTC (permalink / raw)
To: Dan Carpenter; +Cc: Jiri Pirko, netdev, kernel-janitors
In-Reply-To: <20190213085931.GD14113@kadam>
Wed, Feb 13, 2019 at 09:59:31AM CET, dan.carpenter@oracle.com wrote:
>We need to set the error message on this path otherwise some of the
>callers, such as test_hints_case(), print from an uninitialized pointer.
>
>We had a similar bug earlier and set "errmsg" to NULL in the caller,
>test_delta_action_item(). That code is no longer required so I have
>removed it.
>
>Fixes: 9069a3817d82 ("lib: objagg: implement optimization hints assembly and use hints for object creation")
>Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
^ permalink raw reply
* Re: [PATCH] net: phy: at803x: disable delay only for RGMII mode
From: Andrew Lunn @ 2019-02-13 13:29 UTC (permalink / raw)
To: Niklas Cassel
Cc: Vinod Koul, David S Miller, linux-arm-msm, Bjorn Andersson,
netdev, Florian Fainelli, Nori, Sekhar, Peter Ujfalusi,
Marc Gonzalez
In-Reply-To: <20190213131206.GA460@centauri.lan>
> So we have these modes:
>
> PHY_INTERFACE_MODE_RGMII: TX and RX delays disabled
> PHY_INTERFACE_MODE_RGMII_ID: TX and RX delays enabled
> PHY_INTERFACE_MODE_RGMII_RXID: RX delay enabled, TX delay disabled
> PHY_INTERFACE_MODE_RGMII_TXID: TX delay enabled, RX delay disabled
>
> What I don't like with this patch, is that if we specify phy-mode
> PHY_INTERFACE_MODE_RGMII_TXID, this patch will enable TX delay,
> but RX delay will not be explicitly set.
That is not the behaviour we want. It is best to assume the device is
in a random state, and correctly enable/disable all delays as
requested. Only leave the hardware alone if PHY_INTERFACE_MODE_NA is
used.
Andrew
^ permalink raw reply
* Re: [PATCH] net: phy: at803x: disable delay only for RGMII mode
From: Marc Gonzalez @ 2019-02-13 13:40 UTC (permalink / raw)
To: Andrew Lunn, Niklas Cassel, Florian Fainelli
Cc: Vinod Koul, David S Miller, linux-arm-msm, Bjorn Andersson,
netdev, Florian Fainelli, Nori, Sekhar, Peter Ujfalusi
In-Reply-To: <20190213132900.GA24589@lunn.ch>
On 13/02/2019 14:29, Andrew Lunn wrote:
>> So we have these modes:
>>
>> PHY_INTERFACE_MODE_RGMII: TX and RX delays disabled
>> PHY_INTERFACE_MODE_RGMII_ID: TX and RX delays enabled
>> PHY_INTERFACE_MODE_RGMII_RXID: RX delay enabled, TX delay disabled
>> PHY_INTERFACE_MODE_RGMII_TXID: TX delay enabled, RX delay disabled
>>
>> What I don't like with this patch, is that if we specify phy-mode
>> PHY_INTERFACE_MODE_RGMII_TXID, this patch will enable TX delay,
>> but RX delay will not be explicitly set.
>
> That is not the behaviour we want. It is best to assume the device is
> in a random state, and correctly enable/disable all delays as
> requested. Only leave the hardware alone if PHY_INTERFACE_MODE_NA is
> used.
That's what my patch did:
https://www.spinics.net/lists/netdev/msg445053.html
But see Florian's remarks:
https://www.spinics.net/lists/netdev/msg445133.html
Regards.
^ permalink raw reply
* Re: [PATCH] rhashtable: Remove obsolete rhashtable_walk_init function
From: Herbert Xu @ 2019-02-13 13:41 UTC (permalink / raw)
To: kbuild test robot
Cc: kbuild-all, David Miller, johannes, linux-wireless, netdev, j,
tgraf, johannes.berg
In-Reply-To: <201902131934.29Pw8ywP%fengguang.wu@intel.com>
On Wed, Feb 13, 2019 at 07:20:51PM +0800, kbuild test robot wrote:
> Hi Herbert,
>
> I love your patch! Yet something to improve:
>
> [auto build test ERROR on linus/master]
> [also build test ERROR on v5.0-rc4 next-20190212]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
>
> url: https://github.com/0day-ci/linux/commits/Herbert-Xu/rhashtable-Remove-obsolete-rhashtable_walk_init-function/20190213-182336
> config: x86_64-lkp (attached as .config)
> compiler: gcc-8 (Debian 8.2.0-20) 8.2.0
> reproduce:
> # save the attached .config to linux build tree
> make ARCH=x86_64
>
> All errors (new ones prefixed by >>):
>
> net/mac80211/mesh_pathtbl.c: In function '__mesh_path_lookup_by_idx':
> >> net/mac80211/mesh_pathtbl.c:256:8: error: implicit declaration of function 'rhashtable_walk_init'; did you mean 'rhashtable_walk_exit'? [-Werror=implicit-function-declaration]
> ret = rhashtable_walk_init(&tbl->rhead, &iter, GFP_ATOMIC);
> ^~~~~~~~~~~~~~~~~~~~
> rhashtable_walk_exit
> cc1: some warnings being treated as errors
This patch depends on a prior patch to mac80211.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH 2/2] mac80211: Free mpath object when rhashtable insertion fails (fwd)
From: Herbert Xu @ 2019-02-13 13:43 UTC (permalink / raw)
To: Julia Lawall
Cc: David Miller, johannes, linux-wireless, netdev, j, tgraf,
johannes.berg, kbuild-all
In-Reply-To: <alpine.DEB.2.20.1902131349440.3561@hadrien>
On Wed, Feb 13, 2019 at 01:50:20PM +0100, Julia Lawall wrote:
> Perhaps a return is needed after line 436.
I don't think so. The kfree only occurs in the case where the
hash table insertion fails. While we should only return an error
if both the hash table insertion failed and the error is not due
to an existing entry (EEXIST).
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH 1/2] mac80211: Use linked list instead of rhashtable walk for mesh tables
From: Herbert Xu @ 2019-02-13 13:47 UTC (permalink / raw)
To: David Miller, johannes, linux-wireless, netdev, j, tgraf,
johannes.berg
In-Reply-To: <E1gtmu5-0001Vb-LI@gondobar>
On Wed, Feb 13, 2019 at 01:16:13PM +0800, Herbert Xu wrote:
> The mesh table code walks over hash tables for two purposes. First of
> all it's used as part of a netlink dump process, but it is also used
> for looking up entries to delete using criteria other than the hash
> key.
>
> The second purpose is directly contrary to the design specification
> of rhashtable walks. It is only meant for use by netlink dumps.
>
> This is because rhashtable is resizable and you cannot obtain a
> stable walk over it during a resize process.
>
> In fact mesh's use of rhashtable for dumping is bogus too. Rather
> than using rhashtable walk's iterator to keep track of the current
> position, it always converts the current position to an integer
> which defeats the purpose of the iterator.
>
> Therefore this patch converts all uses of rhashtable walk into a
> simple linked list.
>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
OK this patch is broken because there is no locking on the linked
list. I'll repost the series.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: KASAN: use-after-free Read in sctp_outq_tail
From: Marcelo Ricardo Leitner @ 2019-02-13 13:51 UTC (permalink / raw)
To: Xin Long
Cc: syzbot, davem, LKML, linux-sctp, network dev, Neil Horman,
syzkaller-bugs, Vlad Yasevich
In-Reply-To: <CADvbK_dndFMxyUZk=k8qEy9=PNbLc81F++2L0fbdtUdbupq4Xg@mail.gmail.com>
On Wed, Feb 13, 2019 at 12:35:56PM +0800, Xin Long wrote:
> On Wed, Feb 13, 2019 at 4:00 AM syzbot
> <syzbot+7823fa3f3e2d69341ea8@syzkaller.appspotmail.com> wrote:
> >
> > Hello,
> >
> > syzbot found the following crash on:
> >
> > HEAD commit: d4104460aec1 Add linux-next specific files for 20190211
> > git tree: linux-next
> > console output: https://syzkaller.appspot.com/x/log.txt?x=14140124c00000
> > kernel config: https://syzkaller.appspot.com/x/.config?x=c8a112d3b0d6719b
> > dashboard link: https://syzkaller.appspot.com/bug?extid=7823fa3f3e2d69341ea8
> > compiler: gcc (GCC) 9.0.0 20181231 (experimental)
> >
> > Unfortunately, I don't have any reproducer for this crash yet.
> >
> > IMPORTANT: if you fix the bug, please add the following tag to the commit:
> > Reported-by: syzbot+7823fa3f3e2d69341ea8@syzkaller.appspotmail.com
> >
> > ==================================================================
> > BUG: KASAN: use-after-free in list_add_tail include/linux/list.h:93 [inline]
> > BUG: KASAN: use-after-free in sctp_outq_tail_data net/sctp/outqueue.c:105
> > [inline]
> > BUG: KASAN: use-after-free in sctp_outq_tail+0x816/0x930
> > net/sctp/outqueue.c:313
> > Read of size 8 at addr ffff88807b19a7b8 by task syz-executor.0/30745
> I think https://patchwork.ozlabs.org/patch/1040500/ will fix this.
I don't think so. Seems it will switch from use-after-free to NULL deref
instead with that patch.
> > CPU: 1 PID: 30745 Comm: syz-executor.0 Not tainted 5.0.0-rc5-next-20190211
> > #32
> > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> > Google 01/01/2011
> > Call Trace:
> > __dump_stack lib/dump_stack.c:77 [inline]
> > dump_stack+0x172/0x1f0 lib/dump_stack.c:113
> > print_address_description.cold+0x7c/0x20d mm/kasan/report.c:187
> > kasan_report.cold+0x1b/0x40 mm/kasan/report.c:317
> > __asan_report_load8_noabort+0x14/0x20 mm/kasan/generic_report.c:132
> > list_add_tail include/linux/list.h:93 [inline]
> > sctp_outq_tail_data net/sctp/outqueue.c:105 [inline]
> > sctp_outq_tail+0x816/0x930 net/sctp/outqueue.c:313
> > sctp_cmd_send_msg net/sctp/sm_sideeffect.c:1109 [inline]
> > sctp_cmd_interpreter net/sctp/sm_sideeffect.c:1784 [inline]
> > sctp_side_effects net/sctp/sm_sideeffect.c:1220 [inline]
> > sctp_do_sm+0x68e/0x5380 net/sctp/sm_sideeffect.c:1191
> > sctp_primitive_SEND+0xa0/0xd0 net/sctp/primitive.c:178
> > sctp_sendmsg_to_asoc+0xa63/0x17b0 net/sctp/socket.c:1955
sctp_sendmsg_to_asoc()
...
if (sinfo->sinfo_stream >= asoc->stream.outcnt) {
err = -EINVAL;
goto err;
}
if (unlikely(!SCTP_SO(&asoc->stream, sinfo->sinfo_stream)->ext)) {
...
It should have aborted even if an old ->ext was still there because
outcnt is correctly updated. So somehow outcnt was wrong here.
sctp_stream_init()
...
/* Filter out chunks queued on streams that won't exist anymore */
sched->unsched_all(stream);
sctp_stream_outq_migrate(stream, NULL, outcnt); <--- [A]
sched->sched_all(stream);
ret = sctp_stream_alloc_out(stream, outcnt, gfp); <--- [B]
if (ret)
goto out;
stream->outcnt = outcnt; <--- [C]
...
We have a problem here because [A] is freeing ->ext, but [B] can fail (ENOMEM),
which would lead it to not update outcnt in [C] even after the
changes already performed in [A].
It should handle the freeing of ->ext in sctp_stream_alloc_out()
instead, or allocate the flexarray earlier (so it can bail out before
freeing stuff).
> > sctp_sendmsg+0x10a9/0x17e0 net/sctp/socket.c:2113
> > inet_sendmsg+0x147/0x5d0 net/ipv4/af_inet.c:798
> > sock_sendmsg_nosec net/socket.c:621 [inline]
> > sock_sendmsg+0xdd/0x130 net/socket.c:631
> > ___sys_sendmsg+0x806/0x930 net/socket.c:2136
> > __sys_sendmsg+0x105/0x1d0 net/socket.c:2174
> > __do_sys_sendmsg net/socket.c:2183 [inline]
> > __se_sys_sendmsg net/socket.c:2181 [inline]
> > __x64_sys_sendmsg+0x78/0xb0 net/socket.c:2181
> > do_syscall_64+0x103/0x610 arch/x86/entry/common.c:290
> > entry_SYSCALL_64_after_hwframe+0x49/0xbe
> > RIP: 0033:0x457e39
> > Code: ad b8 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7
> > 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff
> > ff 0f 83 7b b8 fb ff c3 66 2e 0f 1f 84 00 00 00 00
> > RSP: 002b:00007fa9b8630c78 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
> > RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 0000000000457e39
> > RDX: 0000000000000000 RSI: 0000000020000140 RDI: 0000000000000003
> > RBP: 000000000073bf00 R08: 0000000000000000 R09: 0000000000000000
> > R10: 0000000000000000 R11: 0000000000000246 R12: 00007fa9b86316d4
> > R13: 00000000004c4e2b R14: 00000000004d8ab8 R15: 00000000ffffffff
> >
> > Allocated by task 30745:
> > save_stack+0x45/0xd0 mm/kasan/common.c:75
> > set_track mm/kasan/common.c:87 [inline]
> > __kasan_kmalloc mm/kasan/common.c:498 [inline]
> > __kasan_kmalloc.constprop.0+0xcf/0xe0 mm/kasan/common.c:471
> > kasan_kmalloc+0x9/0x10 mm/kasan/common.c:506
> > kmem_cache_alloc_trace+0x151/0x760 mm/slab.c:3615
> > kmalloc include/linux/slab.h:548 [inline]
> > kzalloc include/linux/slab.h:743 [inline]
> > sctp_stream_init_ext+0x51/0x110 net/sctp/stream.c:172
> > sctp_sendmsg_to_asoc+0x1273/0x17b0 net/sctp/socket.c:1896
> > sctp_sendmsg+0x10a9/0x17e0 net/sctp/socket.c:2113
> > inet_sendmsg+0x147/0x5d0 net/ipv4/af_inet.c:798
> > sock_sendmsg_nosec net/socket.c:621 [inline]
> > sock_sendmsg+0xdd/0x130 net/socket.c:631
> > ___sys_sendmsg+0x806/0x930 net/socket.c:2136
> > __sys_sendmsg+0x105/0x1d0 net/socket.c:2174
> > __do_sys_sendmsg net/socket.c:2183 [inline]
> > __se_sys_sendmsg net/socket.c:2181 [inline]
> > __x64_sys_sendmsg+0x78/0xb0 net/socket.c:2181
> > do_syscall_64+0x103/0x610 arch/x86/entry/common.c:290
> > entry_SYSCALL_64_after_hwframe+0x49/0xbe
> >
> > Freed by task 30745:
> > save_stack+0x45/0xd0 mm/kasan/common.c:75
> > set_track mm/kasan/common.c:87 [inline]
> > __kasan_slab_free+0x102/0x150 mm/kasan/common.c:460
> > kasan_slab_free+0xe/0x10 mm/kasan/common.c:468
> > __cache_free mm/slab.c:3491 [inline]
> > kfree+0xcf/0x230 mm/slab.c:3816
> > sctp_stream_outq_migrate+0x3e6/0x540 net/sctp/stream.c:88
> > sctp_stream_init+0xbc/0x410 net/sctp/stream.c:139
> > sctp_process_init+0x21c3/0x2b20 net/sctp/sm_make_chunk.c:2466
> > sctp_cmd_process_init net/sctp/sm_sideeffect.c:682 [inline]
> > sctp_cmd_interpreter net/sctp/sm_sideeffect.c:1410 [inline]
> > sctp_side_effects net/sctp/sm_sideeffect.c:1220 [inline]
> > sctp_do_sm+0x3145/0x5380 net/sctp/sm_sideeffect.c:1191
> > sctp_assoc_bh_rcv+0x343/0x660 net/sctp/associola.c:1074
> > sctp_inq_push+0x1ea/0x290 net/sctp/inqueue.c:95
> > sctp_backlog_rcv+0x196/0xbe0 net/sctp/input.c:354
> > sk_backlog_rcv include/net/sock.h:937 [inline]
> > __release_sock+0x12e/0x3a0 net/core/sock.c:2379
> > release_sock+0x59/0x1c0 net/core/sock.c:2895
> > sctp_wait_for_connect+0x316/0x540 net/sctp/socket.c:8998
> > sctp_sendmsg_to_asoc+0x13e3/0x17b0 net/sctp/socket.c:1967
> > sctp_sendmsg+0x10a9/0x17e0 net/sctp/socket.c:2113
> > inet_sendmsg+0x147/0x5d0 net/ipv4/af_inet.c:798
> > sock_sendmsg_nosec net/socket.c:621 [inline]
> > sock_sendmsg+0xdd/0x130 net/socket.c:631
> > ___sys_sendmsg+0x806/0x930 net/socket.c:2136
> > __sys_sendmsg+0x105/0x1d0 net/socket.c:2174
> > __do_sys_sendmsg net/socket.c:2183 [inline]
> > __se_sys_sendmsg net/socket.c:2181 [inline]
> > __x64_sys_sendmsg+0x78/0xb0 net/socket.c:2181
> > do_syscall_64+0x103/0x610 arch/x86/entry/common.c:290
> > entry_SYSCALL_64_after_hwframe+0x49/0xbe
> >
> > The buggy address belongs to the object at ffff88807b19a780
> > which belongs to the cache kmalloc-96 of size 96
> > The buggy address is located 56 bytes inside of
> > 96-byte region [ffff88807b19a780, ffff88807b19a7e0)
> > The buggy address belongs to the page:
> > page:ffffea0001ec6680 count:1 mapcount:0 mapping:ffff88812c3f04c0
> > index:0xffff88807b19a800
> > flags: 0x1fffc0000000200(slab)
> > raw: 01fffc0000000200 ffffea000262acc8 ffffea0001448348 ffff88812c3f04c0
> > raw: ffff88807b19a800 ffff88807b19a000 000000010000001d 0000000000000000
> > page dumped because: kasan: bad access detected
> >
> > Memory state around the buggy address:
> > ffff88807b19a680: fb fb fb fb fb fb fb fb fb fb fb fb fc fc fc fc
> > ffff88807b19a700: fb fb fb fb fb fb fb fb fb fb fb fb fc fc fc fc
> > > ffff88807b19a780: fb fb fb fb fb fb fb fb fb fb fb fb fc fc fc fc
> > ^
> > ffff88807b19a800: fb fb fb fb fb fb fb fb fb fb fb fb fc fc fc fc
> > ffff88807b19a880: fb fb fb fb fb fb fb fb fb fb fb fb fc fc fc fc
> > ==================================================================
> >
> >
> > ---
> > This bug is generated by a bot. It may contain errors.
> > See https://goo.gl/tpsmEJ for more information about syzbot.
> > syzbot engineers can be reached at syzkaller@googlegroups.com.
> >
> > syzbot will keep track of this bug report. See:
> > https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with
> > syzbot.
>
^ permalink raw reply
* Re: [PATCH net-next 09/12] mlxsw: core: Extend hwmon interface with fan fault attribute
From: Andrew Lunn @ 2019-02-13 13:53 UTC (permalink / raw)
To: Ido Schimmel
Cc: netdev@vger.kernel.org, davem@davemloft.net, Jiri Pirko, mlxsw,
Vadim Pasternak, Guenter Roeck
In-Reply-To: <20190213112814.32334-10-idosch@mellanox.com>
On Wed, Feb 13, 2019 at 11:28:53AM +0000, Ido Schimmel wrote:
> From: Vadim Pasternak <vadimp@mellanox.com>
>
> Add new fan hwmon attribute for exposing fan faults (fault indication is
> read from Fan Out of Range Event Register).
>
> Signed-off-by: Vadim Pasternak <vadimp@mellanox.com>
> Reviewed-by: Jiri Pirko <jiri@mellanox.com>
> Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Hi Ido
You should include the HWMON maintainer in the Cc: list.
I would not be too surprised if he says to use
hwmon_device_register_with_info().
Andrew
^ permalink raw reply
* Re: KASAN: use-after-free Read in sctp_outq_tail
From: Dmitry Vyukov @ 2019-02-13 14:00 UTC (permalink / raw)
To: Marcelo Ricardo Leitner
Cc: Xin Long, syzbot, davem, LKML, linux-sctp, network dev,
Neil Horman, syzkaller-bugs, Vlad Yasevich
In-Reply-To: <20190213135157.GJ10665@localhost.localdomain>
On Wed, Feb 13, 2019 at 2:52 PM Marcelo Ricardo Leitner
<marcelo.leitner@gmail.com> wrote:
>
> On Wed, Feb 13, 2019 at 12:35:56PM +0800, Xin Long wrote:
> > On Wed, Feb 13, 2019 at 4:00 AM syzbot
> > <syzbot+7823fa3f3e2d69341ea8@syzkaller.appspotmail.com> wrote:
> > >
> > > Hello,
> > >
> > > syzbot found the following crash on:
> > >
> > > HEAD commit: d4104460aec1 Add linux-next specific files for 20190211
> > > git tree: linux-next
> > > console output: https://syzkaller.appspot.com/x/log.txt?x=14140124c00000
> > > kernel config: https://syzkaller.appspot.com/x/.config?x=c8a112d3b0d6719b
> > > dashboard link: https://syzkaller.appspot.com/bug?extid=7823fa3f3e2d69341ea8
> > > compiler: gcc (GCC) 9.0.0 20181231 (experimental)
> > >
> > > Unfortunately, I don't have any reproducer for this crash yet.
> > >
> > > IMPORTANT: if you fix the bug, please add the following tag to the commit:
> > > Reported-by: syzbot+7823fa3f3e2d69341ea8@syzkaller.appspotmail.com
> > >
> > > ==================================================================
> > > BUG: KASAN: use-after-free in list_add_tail include/linux/list.h:93 [inline]
> > > BUG: KASAN: use-after-free in sctp_outq_tail_data net/sctp/outqueue.c:105
> > > [inline]
> > > BUG: KASAN: use-after-free in sctp_outq_tail+0x816/0x930
> > > net/sctp/outqueue.c:313
> > > Read of size 8 at addr ffff88807b19a7b8 by task syz-executor.0/30745
> > I think https://patchwork.ozlabs.org/patch/1040500/ will fix this.
>
> I don't think so. Seems it will switch from use-after-free to NULL deref
> instead with that patch.
Let's unmark it as fixed for now then so that syzbot does not close it
prematurely
#syz fix: see discussion on the report email thread
> > > CPU: 1 PID: 30745 Comm: syz-executor.0 Not tainted 5.0.0-rc5-next-20190211
> > > #32
> > > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> > > Google 01/01/2011
> > > Call Trace:
> > > __dump_stack lib/dump_stack.c:77 [inline]
> > > dump_stack+0x172/0x1f0 lib/dump_stack.c:113
> > > print_address_description.cold+0x7c/0x20d mm/kasan/report.c:187
> > > kasan_report.cold+0x1b/0x40 mm/kasan/report.c:317
> > > __asan_report_load8_noabort+0x14/0x20 mm/kasan/generic_report.c:132
> > > list_add_tail include/linux/list.h:93 [inline]
> > > sctp_outq_tail_data net/sctp/outqueue.c:105 [inline]
> > > sctp_outq_tail+0x816/0x930 net/sctp/outqueue.c:313
> > > sctp_cmd_send_msg net/sctp/sm_sideeffect.c:1109 [inline]
> > > sctp_cmd_interpreter net/sctp/sm_sideeffect.c:1784 [inline]
> > > sctp_side_effects net/sctp/sm_sideeffect.c:1220 [inline]
> > > sctp_do_sm+0x68e/0x5380 net/sctp/sm_sideeffect.c:1191
> > > sctp_primitive_SEND+0xa0/0xd0 net/sctp/primitive.c:178
> > > sctp_sendmsg_to_asoc+0xa63/0x17b0 net/sctp/socket.c:1955
>
> sctp_sendmsg_to_asoc()
> ...
> if (sinfo->sinfo_stream >= asoc->stream.outcnt) {
> err = -EINVAL;
> goto err;
> }
>
> if (unlikely(!SCTP_SO(&asoc->stream, sinfo->sinfo_stream)->ext)) {
> ...
>
> It should have aborted even if an old ->ext was still there because
> outcnt is correctly updated. So somehow outcnt was wrong here.
>
> sctp_stream_init()
> ...
> /* Filter out chunks queued on streams that won't exist anymore */
> sched->unsched_all(stream);
> sctp_stream_outq_migrate(stream, NULL, outcnt); <--- [A]
> sched->sched_all(stream);
>
> ret = sctp_stream_alloc_out(stream, outcnt, gfp); <--- [B]
> if (ret)
> goto out;
>
> stream->outcnt = outcnt; <--- [C]
> ...
>
> We have a problem here because [A] is freeing ->ext, but [B] can fail (ENOMEM),
> which would lead it to not update outcnt in [C] even after the
> changes already performed in [A].
>
> It should handle the freeing of ->ext in sctp_stream_alloc_out()
> instead, or allocate the flexarray earlier (so it can bail out before
> freeing stuff).
>
> > > sctp_sendmsg+0x10a9/0x17e0 net/sctp/socket.c:2113
> > > inet_sendmsg+0x147/0x5d0 net/ipv4/af_inet.c:798
> > > sock_sendmsg_nosec net/socket.c:621 [inline]
> > > sock_sendmsg+0xdd/0x130 net/socket.c:631
> > > ___sys_sendmsg+0x806/0x930 net/socket.c:2136
> > > __sys_sendmsg+0x105/0x1d0 net/socket.c:2174
> > > __do_sys_sendmsg net/socket.c:2183 [inline]
> > > __se_sys_sendmsg net/socket.c:2181 [inline]
> > > __x64_sys_sendmsg+0x78/0xb0 net/socket.c:2181
> > > do_syscall_64+0x103/0x610 arch/x86/entry/common.c:290
> > > entry_SYSCALL_64_after_hwframe+0x49/0xbe
> > > RIP: 0033:0x457e39
> > > Code: ad b8 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7
> > > 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff
> > > ff 0f 83 7b b8 fb ff c3 66 2e 0f 1f 84 00 00 00 00
> > > RSP: 002b:00007fa9b8630c78 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
> > > RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 0000000000457e39
> > > RDX: 0000000000000000 RSI: 0000000020000140 RDI: 0000000000000003
> > > RBP: 000000000073bf00 R08: 0000000000000000 R09: 0000000000000000
> > > R10: 0000000000000000 R11: 0000000000000246 R12: 00007fa9b86316d4
> > > R13: 00000000004c4e2b R14: 00000000004d8ab8 R15: 00000000ffffffff
> > >
> > > Allocated by task 30745:
> > > save_stack+0x45/0xd0 mm/kasan/common.c:75
> > > set_track mm/kasan/common.c:87 [inline]
> > > __kasan_kmalloc mm/kasan/common.c:498 [inline]
> > > __kasan_kmalloc.constprop.0+0xcf/0xe0 mm/kasan/common.c:471
> > > kasan_kmalloc+0x9/0x10 mm/kasan/common.c:506
> > > kmem_cache_alloc_trace+0x151/0x760 mm/slab.c:3615
> > > kmalloc include/linux/slab.h:548 [inline]
> > > kzalloc include/linux/slab.h:743 [inline]
> > > sctp_stream_init_ext+0x51/0x110 net/sctp/stream.c:172
> > > sctp_sendmsg_to_asoc+0x1273/0x17b0 net/sctp/socket.c:1896
> > > sctp_sendmsg+0x10a9/0x17e0 net/sctp/socket.c:2113
> > > inet_sendmsg+0x147/0x5d0 net/ipv4/af_inet.c:798
> > > sock_sendmsg_nosec net/socket.c:621 [inline]
> > > sock_sendmsg+0xdd/0x130 net/socket.c:631
> > > ___sys_sendmsg+0x806/0x930 net/socket.c:2136
> > > __sys_sendmsg+0x105/0x1d0 net/socket.c:2174
> > > __do_sys_sendmsg net/socket.c:2183 [inline]
> > > __se_sys_sendmsg net/socket.c:2181 [inline]
> > > __x64_sys_sendmsg+0x78/0xb0 net/socket.c:2181
> > > do_syscall_64+0x103/0x610 arch/x86/entry/common.c:290
> > > entry_SYSCALL_64_after_hwframe+0x49/0xbe
> > >
> > > Freed by task 30745:
> > > save_stack+0x45/0xd0 mm/kasan/common.c:75
> > > set_track mm/kasan/common.c:87 [inline]
> > > __kasan_slab_free+0x102/0x150 mm/kasan/common.c:460
> > > kasan_slab_free+0xe/0x10 mm/kasan/common.c:468
> > > __cache_free mm/slab.c:3491 [inline]
> > > kfree+0xcf/0x230 mm/slab.c:3816
> > > sctp_stream_outq_migrate+0x3e6/0x540 net/sctp/stream.c:88
> > > sctp_stream_init+0xbc/0x410 net/sctp/stream.c:139
> > > sctp_process_init+0x21c3/0x2b20 net/sctp/sm_make_chunk.c:2466
> > > sctp_cmd_process_init net/sctp/sm_sideeffect.c:682 [inline]
> > > sctp_cmd_interpreter net/sctp/sm_sideeffect.c:1410 [inline]
> > > sctp_side_effects net/sctp/sm_sideeffect.c:1220 [inline]
> > > sctp_do_sm+0x3145/0x5380 net/sctp/sm_sideeffect.c:1191
> > > sctp_assoc_bh_rcv+0x343/0x660 net/sctp/associola.c:1074
> > > sctp_inq_push+0x1ea/0x290 net/sctp/inqueue.c:95
> > > sctp_backlog_rcv+0x196/0xbe0 net/sctp/input.c:354
> > > sk_backlog_rcv include/net/sock.h:937 [inline]
> > > __release_sock+0x12e/0x3a0 net/core/sock.c:2379
> > > release_sock+0x59/0x1c0 net/core/sock.c:2895
> > > sctp_wait_for_connect+0x316/0x540 net/sctp/socket.c:8998
> > > sctp_sendmsg_to_asoc+0x13e3/0x17b0 net/sctp/socket.c:1967
> > > sctp_sendmsg+0x10a9/0x17e0 net/sctp/socket.c:2113
> > > inet_sendmsg+0x147/0x5d0 net/ipv4/af_inet.c:798
> > > sock_sendmsg_nosec net/socket.c:621 [inline]
> > > sock_sendmsg+0xdd/0x130 net/socket.c:631
> > > ___sys_sendmsg+0x806/0x930 net/socket.c:2136
> > > __sys_sendmsg+0x105/0x1d0 net/socket.c:2174
> > > __do_sys_sendmsg net/socket.c:2183 [inline]
> > > __se_sys_sendmsg net/socket.c:2181 [inline]
> > > __x64_sys_sendmsg+0x78/0xb0 net/socket.c:2181
> > > do_syscall_64+0x103/0x610 arch/x86/entry/common.c:290
> > > entry_SYSCALL_64_after_hwframe+0x49/0xbe
> > >
> > > The buggy address belongs to the object at ffff88807b19a780
> > > which belongs to the cache kmalloc-96 of size 96
> > > The buggy address is located 56 bytes inside of
> > > 96-byte region [ffff88807b19a780, ffff88807b19a7e0)
> > > The buggy address belongs to the page:
> > > page:ffffea0001ec6680 count:1 mapcount:0 mapping:ffff88812c3f04c0
> > > index:0xffff88807b19a800
> > > flags: 0x1fffc0000000200(slab)
> > > raw: 01fffc0000000200 ffffea000262acc8 ffffea0001448348 ffff88812c3f04c0
> > > raw: ffff88807b19a800 ffff88807b19a000 000000010000001d 0000000000000000
> > > page dumped because: kasan: bad access detected
> > >
> > > Memory state around the buggy address:
> > > ffff88807b19a680: fb fb fb fb fb fb fb fb fb fb fb fb fc fc fc fc
> > > ffff88807b19a700: fb fb fb fb fb fb fb fb fb fb fb fb fc fc fc fc
> > > > ffff88807b19a780: fb fb fb fb fb fb fb fb fb fb fb fb fc fc fc fc
> > > ^
> > > ffff88807b19a800: fb fb fb fb fb fb fb fb fb fb fb fb fc fc fc fc
> > > ffff88807b19a880: fb fb fb fb fb fb fb fb fb fb fb fb fc fc fc fc
> > > ==================================================================
> > >
> > >
> > > ---
> > > This bug is generated by a bot. It may contain errors.
> > > See https://goo.gl/tpsmEJ for more information about syzbot.
> > > syzbot engineers can be reached at syzkaller@googlegroups.com.
> > >
> > > syzbot will keep track of this bug report. See:
> > > https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with
> > > syzbot.
> >
>
> --
> You received this message because you are subscribed to the Google Groups "syzkaller-bugs" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to syzkaller-bugs+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/syzkaller-bugs/20190213135157.GJ10665%40localhost.localdomain.
> For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply
* [PATCH net] mwifiex: Fix NL80211_TX_POWER_LIMITED
From: Adrian Bunk @ 2019-02-13 13:59 UTC (permalink / raw)
To: netdev
Cc: Amitkumar Karwar, Nishant Sarmukadam, Ganapathi Bhat, Xinming Hu,
Kalle Valo, linux-wireless
NL80211_TX_POWER_LIMITED was treated as NL80211_TX_POWER_AUTOMATIC,
which is the opposite of what should happen and can cause nasty
regulatory problems.
if/else converted to a switch without default to make gcc warn
on unhandled enum values.
Signed-off-by: Adrian Bunk <bunk@kernel.org>
---
I tried to follow the conventions used inside this driver for
variable types, dbm_min is u16 like dbm.
---
drivers/net/wireless/marvell/mwifiex/cfg80211.c | 13 +++++++++++--
drivers/net/wireless/marvell/mwifiex/ioctl.h | 1 +
drivers/net/wireless/marvell/mwifiex/sta_ioctl.c | 11 +++++++----
3 files changed, 19 insertions(+), 6 deletions(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.c b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
index 1467af22e394..9ef0fc108641 100644
--- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
+++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
@@ -376,11 +376,20 @@ mwifiex_cfg80211_set_tx_power(struct wiphy *wiphy,
struct mwifiex_power_cfg power_cfg;
int dbm = MBM_TO_DBM(mbm);
- if (type == NL80211_TX_POWER_FIXED) {
+ switch (type) {
+ case NL80211_TX_POWER_FIXED:
power_cfg.is_power_auto = 0;
+ power_cfg.is_power_fixed = 1;
power_cfg.power_level = dbm;
- } else {
+ break;
+ case NL80211_TX_POWER_LIMITED:
+ power_cfg.is_power_auto = 0;
+ power_cfg.is_power_fixed = 0;
+ power_cfg.power_level = dbm;
+ break;
+ case NL80211_TX_POWER_AUTOMATIC:
power_cfg.is_power_auto = 1;
+ break;
}
priv = mwifiex_get_priv(adapter, MWIFIEX_BSS_ROLE_ANY);
diff --git a/drivers/net/wireless/marvell/mwifiex/ioctl.h b/drivers/net/wireless/marvell/mwifiex/ioctl.h
index 48e154e1865d..0dd592ea6e83 100644
--- a/drivers/net/wireless/marvell/mwifiex/ioctl.h
+++ b/drivers/net/wireless/marvell/mwifiex/ioctl.h
@@ -267,6 +267,7 @@ struct mwifiex_ds_encrypt_key {
struct mwifiex_power_cfg {
u32 is_power_auto;
+ u32 is_power_fixed;
u32 power_level;
};
diff --git a/drivers/net/wireless/marvell/mwifiex/sta_ioctl.c b/drivers/net/wireless/marvell/mwifiex/sta_ioctl.c
index b454b5f85503..ebc0e41e5d3b 100644
--- a/drivers/net/wireless/marvell/mwifiex/sta_ioctl.c
+++ b/drivers/net/wireless/marvell/mwifiex/sta_ioctl.c
@@ -688,6 +688,9 @@ int mwifiex_set_tx_power(struct mwifiex_private *priv,
txp_cfg = (struct host_cmd_ds_txpwr_cfg *) buf;
txp_cfg->action = cpu_to_le16(HostCmd_ACT_GEN_SET);
if (!power_cfg->is_power_auto) {
+ u16 dbm_min = power_cfg->is_power_fixed ?
+ dbm : priv->min_tx_power_level;
+
txp_cfg->mode = cpu_to_le32(1);
pg_tlv = (struct mwifiex_types_power_group *)
(buf + sizeof(struct host_cmd_ds_txpwr_cfg));
@@ -702,7 +705,7 @@ int mwifiex_set_tx_power(struct mwifiex_private *priv,
pg->last_rate_code = 0x03;
pg->modulation_class = MOD_CLASS_HR_DSSS;
pg->power_step = 0;
- pg->power_min = (s8) dbm;
+ pg->power_min = (s8) dbm_min;
pg->power_max = (s8) dbm;
pg++;
/* Power group for modulation class OFDM */
@@ -710,7 +713,7 @@ int mwifiex_set_tx_power(struct mwifiex_private *priv,
pg->last_rate_code = 0x07;
pg->modulation_class = MOD_CLASS_OFDM;
pg->power_step = 0;
- pg->power_min = (s8) dbm;
+ pg->power_min = (s8) dbm_min;
pg->power_max = (s8) dbm;
pg++;
/* Power group for modulation class HTBW20 */
@@ -718,7 +721,7 @@ int mwifiex_set_tx_power(struct mwifiex_private *priv,
pg->last_rate_code = 0x20;
pg->modulation_class = MOD_CLASS_HT;
pg->power_step = 0;
- pg->power_min = (s8) dbm;
+ pg->power_min = (s8) dbm_min;
pg->power_max = (s8) dbm;
pg->ht_bandwidth = HT_BW_20;
pg++;
@@ -727,7 +730,7 @@ int mwifiex_set_tx_power(struct mwifiex_private *priv,
pg->last_rate_code = 0x20;
pg->modulation_class = MOD_CLASS_HT;
pg->power_step = 0;
- pg->power_min = (s8) dbm;
+ pg->power_min = (s8) dbm_min;
pg->power_max = (s8) dbm;
pg->ht_bandwidth = HT_BW_40;
}
--
2.11.0
^ permalink raw reply related
* [v2 PATCH 0/4] mac80211: Fix incorrect usage of rhashtable walk API
From: Herbert Xu @ 2019-02-13 14:38 UTC (permalink / raw)
To: David Miller, johannes, linux-wireless, netdev, j, tgraf,
johannes.berg
In-Reply-To: <20190213050551.x3jffq3ipghw6g2m@gondor.apana.org.au>
Hi:
The first two patches in this series are bug fixes and should be
backported to stable.
They fixes a number of issues with the use of the rhashtable API
in mac80211. First of all it converts the use of rashtable walks over
to a simple linked list. This is because an rhashtable walk is
inherently unstable and not meant for uses that require stability,
e.g., when you're trying to lookup an object to delete.
It also fixes a potential memory leak when the rhashtable insertion
fails (which can occur due to OOM).
The third patch is a code-cleanup to mac80211 while the last patch
removes an obsolete rhashtable API.
Thanks,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* [PATCH 2/4] mac80211: Free mpath object when rhashtable insertion fails
From: Herbert Xu @ 2019-02-13 14:39 UTC (permalink / raw)
To: David Miller, johannes, linux-wireless, netdev, j, tgraf,
johannes.berg
In-Reply-To: <20190213143853.labj6zdcsoupkris@gondor.apana.org.au>
When rhashtable insertion fails the mesh table code doesn't free
the now-orphan mesh path object. This patch fixes that.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---
net/mac80211/mesh_pathtbl.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/net/mac80211/mesh_pathtbl.c b/net/mac80211/mesh_pathtbl.c
index 884a0d212e8b..db5a1aef22db 100644
--- a/net/mac80211/mesh_pathtbl.c
+++ b/net/mac80211/mesh_pathtbl.c
@@ -436,17 +436,18 @@ struct mesh_path *mesh_path_add(struct ieee80211_sub_if_data *sdata,
} while (unlikely(ret == -EEXIST && !mpath));
spin_unlock_bh(&tbl->walk_lock);
- if (ret && ret != -EEXIST)
+ if (ret)
+ kfree(new_mpath);
+
+ if (ret != -EEXIST)
return ERR_PTR(ret);
/* At this point either new_mpath was added, or we found a
* matching entry already in the table; in the latter case
* free the unnecessary new entry.
*/
- if (ret == -EEXIST) {
- kfree(new_mpath);
+ if (ret == -EEXIST)
new_mpath = mpath;
- }
sdata->u.mesh.mesh_paths_generation++;
return new_mpath;
}
@@ -481,6 +482,9 @@ int mpp_path_add(struct ieee80211_sub_if_data *sdata,
hlist_add_head_rcu(&new_mpath->walk_list, &tbl->walk_head);
spin_unlock_bh(&tbl->walk_lock);
+ if (ret)
+ kfree(new_mpath);
+
sdata->u.mesh.mpp_paths_generation++;
return ret;
}
^ permalink raw reply related
* [PATCH 1/4] mac80211: Use linked list instead of rhashtable walk for mesh tables
From: Herbert Xu @ 2019-02-13 14:39 UTC (permalink / raw)
To: David Miller, johannes, linux-wireless, netdev, j, tgraf,
johannes.berg
In-Reply-To: <20190213143853.labj6zdcsoupkris@gondor.apana.org.au>
The mesh table code walks over hash tables for two purposes. First of
all it's used as part of a netlink dump process, but it is also used
for looking up entries to delete using criteria other than the hash
key.
The second purpose is directly contrary to the design specification
of rhashtable walks. It is only meant for use by netlink dumps.
This is because rhashtable is resizable and you cannot obtain a
stable walk over it during a resize process.
In fact mesh's use of rhashtable for dumping is bogus too. Rather
than using rhashtable walk's iterator to keep track of the current
position, it always converts the current position to an integer
which defeats the purpose of the iterator.
Therefore this patch converts all uses of rhashtable walk into a
simple linked list.
This patch also adds a new spin lock to protect the hash table
insertion/removal as well as the walk list modifications. In fact
the previous code was buggy as the removals can race with each
other, potentially resulting in a double-free.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---
net/mac80211/mesh.h | 6 +
net/mac80211/mesh_pathtbl.c | 138 +++++++++++---------------------------------
2 files changed, 43 insertions(+), 101 deletions(-)
diff --git a/net/mac80211/mesh.h b/net/mac80211/mesh.h
index cad6592c52a1..2ec7011a4d07 100644
--- a/net/mac80211/mesh.h
+++ b/net/mac80211/mesh.h
@@ -70,6 +70,7 @@ enum mesh_deferred_task_flags {
* @dst: mesh path destination mac address
* @mpp: mesh proxy mac address
* @rhash: rhashtable list pointer
+ * @walk_list: linked list containing all mesh_path objects.
* @gate_list: list pointer for known gates list
* @sdata: mesh subif
* @next_hop: mesh neighbor to which frames for this destination will be
@@ -105,6 +106,7 @@ struct mesh_path {
u8 dst[ETH_ALEN];
u8 mpp[ETH_ALEN]; /* used for MPP or MAP */
struct rhash_head rhash;
+ struct hlist_node walk_list;
struct hlist_node gate_list;
struct ieee80211_sub_if_data *sdata;
struct sta_info __rcu *next_hop;
@@ -133,12 +135,16 @@ struct mesh_path {
* gate's mpath may or may not be resolved and active.
* @gates_lock: protects updates to known_gates
* @rhead: the rhashtable containing struct mesh_paths, keyed by dest addr
+ * @walk_head: linked list containging all mesh_path objects
+ * @walk_lock: lock protecting walk_head
* @entries: number of entries in the table
*/
struct mesh_table {
struct hlist_head known_gates;
spinlock_t gates_lock;
struct rhashtable rhead;
+ struct hlist_head walk_head;
+ spinlock_t walk_lock;
atomic_t entries; /* Up to MAX_MESH_NEIGHBOURS */
};
diff --git a/net/mac80211/mesh_pathtbl.c b/net/mac80211/mesh_pathtbl.c
index a5125624a76d..884a0d212e8b 100644
--- a/net/mac80211/mesh_pathtbl.c
+++ b/net/mac80211/mesh_pathtbl.c
@@ -59,8 +59,10 @@ static struct mesh_table *mesh_table_alloc(void)
return NULL;
INIT_HLIST_HEAD(&newtbl->known_gates);
+ INIT_HLIST_HEAD(&newtbl->walk_head);
atomic_set(&newtbl->entries, 0);
spin_lock_init(&newtbl->gates_lock);
+ spin_lock_init(&newtbl->walk_lock);
return newtbl;
}
@@ -249,28 +251,15 @@ mpp_path_lookup(struct ieee80211_sub_if_data *sdata, const u8 *dst)
static struct mesh_path *
__mesh_path_lookup_by_idx(struct mesh_table *tbl, int idx)
{
- int i = 0, ret;
- struct mesh_path *mpath = NULL;
- struct rhashtable_iter iter;
-
- ret = rhashtable_walk_init(&tbl->rhead, &iter, GFP_ATOMIC);
- if (ret)
- return NULL;
-
- rhashtable_walk_start(&iter);
+ int i = 0;
+ struct mesh_path *mpath;
- while ((mpath = rhashtable_walk_next(&iter))) {
- if (IS_ERR(mpath) && PTR_ERR(mpath) == -EAGAIN)
- continue;
- if (IS_ERR(mpath))
- break;
+ hlist_for_each_entry_rcu(mpath, &tbl->walk_head, walk_list) {
if (i++ == idx)
break;
}
- rhashtable_walk_stop(&iter);
- rhashtable_walk_exit(&iter);
- if (IS_ERR(mpath) || !mpath)
+ if (!mpath)
return NULL;
if (mpath_expired(mpath)) {
@@ -432,6 +421,7 @@ struct mesh_path *mesh_path_add(struct ieee80211_sub_if_data *sdata,
return ERR_PTR(-ENOMEM);
tbl = sdata->u.mesh.mesh_paths;
+ spin_lock_bh(&tbl->walk_lock);
do {
ret = rhashtable_lookup_insert_fast(&tbl->rhead,
&new_mpath->rhash,
@@ -441,8 +431,10 @@ struct mesh_path *mesh_path_add(struct ieee80211_sub_if_data *sdata,
mpath = rhashtable_lookup_fast(&tbl->rhead,
dst,
mesh_rht_params);
-
+ else if (!ret)
+ hlist_add_head(&new_mpath->walk_list, &tbl->walk_head);
} while (unlikely(ret == -EEXIST && !mpath));
+ spin_unlock_bh(&tbl->walk_lock);
if (ret && ret != -EEXIST)
return ERR_PTR(ret);
@@ -480,9 +472,14 @@ int mpp_path_add(struct ieee80211_sub_if_data *sdata,
memcpy(new_mpath->mpp, mpp, ETH_ALEN);
tbl = sdata->u.mesh.mpp_paths;
+
+ spin_lock_bh(&tbl->walk_lock);
ret = rhashtable_lookup_insert_fast(&tbl->rhead,
&new_mpath->rhash,
mesh_rht_params);
+ if (!ret)
+ hlist_add_head_rcu(&new_mpath->walk_list, &tbl->walk_head);
+ spin_unlock_bh(&tbl->walk_lock);
sdata->u.mesh.mpp_paths_generation++;
return ret;
@@ -503,20 +500,9 @@ void mesh_plink_broken(struct sta_info *sta)
struct mesh_table *tbl = sdata->u.mesh.mesh_paths;
static const u8 bcast[ETH_ALEN] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
struct mesh_path *mpath;
- struct rhashtable_iter iter;
- int ret;
-
- ret = rhashtable_walk_init(&tbl->rhead, &iter, GFP_ATOMIC);
- if (ret)
- return;
-
- rhashtable_walk_start(&iter);
- while ((mpath = rhashtable_walk_next(&iter))) {
- if (IS_ERR(mpath) && PTR_ERR(mpath) == -EAGAIN)
- continue;
- if (IS_ERR(mpath))
- break;
+ rcu_read_lock();
+ hlist_for_each_entry_rcu(mpath, &tbl->walk_head, walk_list) {
if (rcu_access_pointer(mpath->next_hop) == sta &&
mpath->flags & MESH_PATH_ACTIVE &&
!(mpath->flags & MESH_PATH_FIXED)) {
@@ -530,8 +516,7 @@ void mesh_plink_broken(struct sta_info *sta)
WLAN_REASON_MESH_PATH_DEST_UNREACHABLE, bcast);
}
}
- rhashtable_walk_stop(&iter);
- rhashtable_walk_exit(&iter);
+ rcu_read_unlock();
}
static void mesh_path_free_rcu(struct mesh_table *tbl,
@@ -551,6 +536,7 @@ static void mesh_path_free_rcu(struct mesh_table *tbl,
static void __mesh_path_del(struct mesh_table *tbl, struct mesh_path *mpath)
{
+ hlist_del_rcu(&mpath->walk_list);
rhashtable_remove_fast(&tbl->rhead, &mpath->rhash, mesh_rht_params);
mesh_path_free_rcu(tbl, mpath);
}
@@ -571,27 +557,14 @@ void mesh_path_flush_by_nexthop(struct sta_info *sta)
struct ieee80211_sub_if_data *sdata = sta->sdata;
struct mesh_table *tbl = sdata->u.mesh.mesh_paths;
struct mesh_path *mpath;
- struct rhashtable_iter iter;
- int ret;
-
- ret = rhashtable_walk_init(&tbl->rhead, &iter, GFP_ATOMIC);
- if (ret)
- return;
-
- rhashtable_walk_start(&iter);
-
- while ((mpath = rhashtable_walk_next(&iter))) {
- if (IS_ERR(mpath) && PTR_ERR(mpath) == -EAGAIN)
- continue;
- if (IS_ERR(mpath))
- break;
+ struct hlist_node *n;
+ spin_lock_bh(&tbl->walk_lock);
+ hlist_for_each_entry_safe(mpath, n, &tbl->walk_head, walk_list) {
if (rcu_access_pointer(mpath->next_hop) == sta)
__mesh_path_del(tbl, mpath);
}
-
- rhashtable_walk_stop(&iter);
- rhashtable_walk_exit(&iter);
+ spin_unlock_bh(&tbl->walk_lock);
}
static void mpp_flush_by_proxy(struct ieee80211_sub_if_data *sdata,
@@ -599,51 +572,26 @@ static void mpp_flush_by_proxy(struct ieee80211_sub_if_data *sdata,
{
struct mesh_table *tbl = sdata->u.mesh.mpp_paths;
struct mesh_path *mpath;
- struct rhashtable_iter iter;
- int ret;
-
- ret = rhashtable_walk_init(&tbl->rhead, &iter, GFP_ATOMIC);
- if (ret)
- return;
-
- rhashtable_walk_start(&iter);
-
- while ((mpath = rhashtable_walk_next(&iter))) {
- if (IS_ERR(mpath) && PTR_ERR(mpath) == -EAGAIN)
- continue;
- if (IS_ERR(mpath))
- break;
+ struct hlist_node *n;
+ spin_lock_bh(&tbl->walk_lock);
+ hlist_for_each_entry_safe(mpath, n, &tbl->walk_head, walk_list) {
if (ether_addr_equal(mpath->mpp, proxy))
__mesh_path_del(tbl, mpath);
}
-
- rhashtable_walk_stop(&iter);
- rhashtable_walk_exit(&iter);
+ spin_unlock_bh(&tbl->walk_lock);
}
static void table_flush_by_iface(struct mesh_table *tbl)
{
struct mesh_path *mpath;
- struct rhashtable_iter iter;
- int ret;
-
- ret = rhashtable_walk_init(&tbl->rhead, &iter, GFP_ATOMIC);
- if (ret)
- return;
-
- rhashtable_walk_start(&iter);
+ struct hlist_node *n;
- while ((mpath = rhashtable_walk_next(&iter))) {
- if (IS_ERR(mpath) && PTR_ERR(mpath) == -EAGAIN)
- continue;
- if (IS_ERR(mpath))
- break;
+ spin_lock_bh(&tbl->walk_lock);
+ hlist_for_each_entry_safe(mpath, n, &tbl->walk_head, walk_list) {
__mesh_path_del(tbl, mpath);
}
-
- rhashtable_walk_stop(&iter);
- rhashtable_walk_exit(&iter);
+ spin_unlock_bh(&tbl->walk_lock);
}
/**
@@ -675,7 +623,7 @@ static int table_path_del(struct mesh_table *tbl,
{
struct mesh_path *mpath;
- rcu_read_lock();
+ spin_lock_bh(&tbl->walk_lock);
mpath = rhashtable_lookup_fast(&tbl->rhead, addr, mesh_rht_params);
if (!mpath) {
rcu_read_unlock();
@@ -683,7 +631,7 @@ static int table_path_del(struct mesh_table *tbl,
}
__mesh_path_del(tbl, mpath);
- rcu_read_unlock();
+ spin_unlock_bh(&tbl->walk_lock);
return 0;
}
@@ -854,28 +802,16 @@ void mesh_path_tbl_expire(struct ieee80211_sub_if_data *sdata,
struct mesh_table *tbl)
{
struct mesh_path *mpath;
- struct rhashtable_iter iter;
- int ret;
+ struct hlist_node *n;
- ret = rhashtable_walk_init(&tbl->rhead, &iter, GFP_KERNEL);
- if (ret)
- return;
-
- rhashtable_walk_start(&iter);
-
- while ((mpath = rhashtable_walk_next(&iter))) {
- if (IS_ERR(mpath) && PTR_ERR(mpath) == -EAGAIN)
- continue;
- if (IS_ERR(mpath))
- break;
+ spin_lock_bh(&tbl->walk_lock);
+ hlist_for_each_entry_safe(mpath, n, &tbl->walk_head, walk_list) {
if ((!(mpath->flags & MESH_PATH_RESOLVING)) &&
(!(mpath->flags & MESH_PATH_FIXED)) &&
time_after(jiffies, mpath->exp_time + MESH_PATH_EXPIRE))
__mesh_path_del(tbl, mpath);
}
-
- rhashtable_walk_stop(&iter);
- rhashtable_walk_exit(&iter);
+ spin_unlock_bh(&tbl->walk_lock);
}
void mesh_path_expire(struct ieee80211_sub_if_data *sdata)
^ permalink raw reply related
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