* [PATCH bpf-next v3 1/3] bpf: allow wide (u64) aligned stores for some fields of bpf_sock_addr
From: Stanislav Fomichev @ 2019-07-01 17:38 UTC (permalink / raw)
To: netdev, bpf
Cc: davem, ast, daniel, Stanislav Fomichev, Andrii Nakryiko,
Yonghong Song, kernel test robot
In-Reply-To: <20190701173841.32249-1-sdf@google.com>
Since commit cd17d7770578 ("bpf/tools: sync bpf.h") clang decided
that it can do a single u64 store into user_ip6[2] instead of two
separate u32 ones:
# 17: (18) r2 = 0x100000000000000
# ; ctx->user_ip6[2] = bpf_htonl(DST_REWRITE_IP6_2);
# 19: (7b) *(u64 *)(r1 +16) = r2
# invalid bpf_context access off=16 size=8
From the compiler point of view it does look like a correct thing
to do, so let's support it on the kernel side.
Credit to Andrii Nakryiko for a proper implementation of
bpf_ctx_wide_store_ok.
Cc: Andrii Nakryiko <andriin@fb.com>
Cc: Yonghong Song <yhs@fb.com>
Fixes: cd17d7770578 ("bpf/tools: sync bpf.h")
Reported-by: kernel test robot <rong.a.chen@intel.com>
Acked-by: Yonghong Song <yhs@fb.com>
Acked-by: Andrii Nakryiko <andriin@fb.com>
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
include/linux/filter.h | 6 ++++++
include/uapi/linux/bpf.h | 6 +++---
net/core/filter.c | 22 ++++++++++++++--------
3 files changed, 23 insertions(+), 11 deletions(-)
diff --git a/include/linux/filter.h b/include/linux/filter.h
index 1fe53e78c7e3..6d944369ca87 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -747,6 +747,12 @@ bpf_ctx_narrow_access_ok(u32 off, u32 size, u32 size_default)
return size <= size_default && (size & (size - 1)) == 0;
}
+#define bpf_ctx_wide_store_ok(off, size, type, field) \
+ (size == sizeof(__u64) && \
+ off >= offsetof(type, field) && \
+ off + sizeof(__u64) <= offsetofend(type, field) && \
+ off % sizeof(__u64) == 0)
+
#define bpf_classic_proglen(fprog) (fprog->len * sizeof(fprog->filter[0]))
static inline void bpf_prog_lock_ro(struct bpf_prog *fp)
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index cffea1826a1f..dcdc606e57d6 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -3240,7 +3240,7 @@ struct bpf_sock_addr {
__u32 user_ip4; /* Allows 1,2,4-byte read and 4-byte write.
* Stored in network byte order.
*/
- __u32 user_ip6[4]; /* Allows 1,2,4-byte read an 4-byte write.
+ __u32 user_ip6[4]; /* Allows 1,2,4-byte read and 4,8-byte write.
* Stored in network byte order.
*/
__u32 user_port; /* Allows 4-byte read and write.
@@ -3249,10 +3249,10 @@ struct bpf_sock_addr {
__u32 family; /* Allows 4-byte read, but no write */
__u32 type; /* Allows 4-byte read, but no write */
__u32 protocol; /* Allows 4-byte read, but no write */
- __u32 msg_src_ip4; /* Allows 1,2,4-byte read an 4-byte write.
+ __u32 msg_src_ip4; /* Allows 1,2,4-byte read and 4-byte write.
* Stored in network byte order.
*/
- __u32 msg_src_ip6[4]; /* Allows 1,2,4-byte read an 4-byte write.
+ __u32 msg_src_ip6[4]; /* Allows 1,2,4-byte read and 4,8-byte write.
* Stored in network byte order.
*/
__bpf_md_ptr(struct bpf_sock *, sk);
diff --git a/net/core/filter.c b/net/core/filter.c
index 4836264f82ee..2520dbc539fc 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -6851,6 +6851,16 @@ static bool sock_addr_is_valid_access(int off, int size,
if (!bpf_ctx_narrow_access_ok(off, size, size_default))
return false;
} else {
+ if (bpf_ctx_wide_store_ok(off, size,
+ struct bpf_sock_addr,
+ user_ip6))
+ return true;
+
+ if (bpf_ctx_wide_store_ok(off, size,
+ struct bpf_sock_addr,
+ msg_src_ip6))
+ return true;
+
if (size != size_default)
return false;
}
@@ -7691,9 +7701,6 @@ static u32 xdp_convert_ctx_access(enum bpf_access_type type,
/* SOCK_ADDR_STORE_NESTED_FIELD_OFF() has semantic similar to
* SOCK_ADDR_LOAD_NESTED_FIELD_SIZE_OFF() but for store operation.
*
- * It doesn't support SIZE argument though since narrow stores are not
- * supported for now.
- *
* In addition it uses Temporary Field TF (member of struct S) as the 3rd
* "register" since two registers available in convert_ctx_access are not
* enough: we can't override neither SRC, since it contains value to store, nor
@@ -7701,7 +7708,7 @@ static u32 xdp_convert_ctx_access(enum bpf_access_type type,
* instructions. But we need a temporary place to save pointer to nested
* structure whose field we want to store to.
*/
-#define SOCK_ADDR_STORE_NESTED_FIELD_OFF(S, NS, F, NF, OFF, TF) \
+#define SOCK_ADDR_STORE_NESTED_FIELD_OFF(S, NS, F, NF, SIZE, OFF, TF) \
do { \
int tmp_reg = BPF_REG_9; \
if (si->src_reg == tmp_reg || si->dst_reg == tmp_reg) \
@@ -7712,8 +7719,7 @@ static u32 xdp_convert_ctx_access(enum bpf_access_type type,
offsetof(S, TF)); \
*insn++ = BPF_LDX_MEM(BPF_FIELD_SIZEOF(S, F), tmp_reg, \
si->dst_reg, offsetof(S, F)); \
- *insn++ = BPF_STX_MEM( \
- BPF_FIELD_SIZEOF(NS, NF), tmp_reg, si->src_reg, \
+ *insn++ = BPF_STX_MEM(SIZE, tmp_reg, si->src_reg, \
bpf_target_off(NS, NF, FIELD_SIZEOF(NS, NF), \
target_size) \
+ OFF); \
@@ -7725,8 +7731,8 @@ static u32 xdp_convert_ctx_access(enum bpf_access_type type,
TF) \
do { \
if (type == BPF_WRITE) { \
- SOCK_ADDR_STORE_NESTED_FIELD_OFF(S, NS, F, NF, OFF, \
- TF); \
+ SOCK_ADDR_STORE_NESTED_FIELD_OFF(S, NS, F, NF, SIZE, \
+ OFF, TF); \
} else { \
SOCK_ADDR_LOAD_NESTED_FIELD_SIZE_OFF( \
S, NS, F, NF, SIZE, OFF); \
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCH bpf-next v3 0/3] bpf: allow wide (u64) aligned stores for some fields of bpf_sock_addr
From: Stanislav Fomichev @ 2019-07-01 17:38 UTC (permalink / raw)
To: netdev, bpf; +Cc: davem, ast, daniel, Stanislav Fomichev
Clang can generate 8-byte stores for user_ip6 & msg_src_ip6,
let's support that on the verifier side.
v3:
* fix comments spelling an -> and (Andrii Nakryiko)
v2:
* Add simple cover letter (Yonghong Song)
* Update comments (Yonghong Song)
* Remove [4] selftests (Yonghong Song)
Stanislav Fomichev (3):
bpf: allow wide (u64) aligned stores for some fields of bpf_sock_addr
bpf: sync bpf.h to tools/
selftests/bpf: add verifier tests for wide stores
include/linux/filter.h | 6 ++++
include/uapi/linux/bpf.h | 6 ++--
net/core/filter.c | 22 +++++++-----
tools/include/uapi/linux/bpf.h | 6 ++--
tools/testing/selftests/bpf/test_verifier.c | 17 +++++++--
.../selftests/bpf/verifier/wide_store.c | 36 +++++++++++++++++++
6 files changed, 76 insertions(+), 17 deletions(-)
create mode 100644 tools/testing/selftests/bpf/verifier/wide_store.c
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply
* Re: Use-after-free in br_multicast_rcv
From: Nikolay Aleksandrov @ 2019-07-01 17:37 UTC (permalink / raw)
To: Martin Weinelt, bridge, Roopa Prabhu; +Cc: netdev
In-Reply-To: <3fcf8b05-e1ad-ac97-10bf-bd2b6354424c@linuxlounge.net>
[-- Attachment #1: Type: text/plain, Size: 3413 bytes --]
Hi again,
On 01/07/2019 20:31, Martin Weinelt wrote:
> Hi Nik,
>
> On 7/1/19 7:03 PM, Nikolay Aleksandrov wrote:
>> Hi Martin,
>>
>> On 01/07/2019 19:53, Martin Weinelt wrote:
>>> Hi Nik,
>>>
>>> more info below.
>>>
>>> On 6/29/19 3:11 PM, nikolay@cumulusnetworks.com wrote:
>>>> On 29 June 2019 14:54:44 EEST, Martin Weinelt <martin@linuxlounge.net> wrote:
>>>>> Hello,
>>>>>
>>>>> we've recently been experiencing memory leaks on our Linux-based
>>>>> routers,
>>>>> at least as far back as v4.19.16.
>>>>>
>>>>> After rebuilding with KASAN it found a use-after-free in
>>>>> br_multicast_rcv which I could reproduce on v5.2.0-rc6.
>>>>>
>>>>> Please find the KASAN report below, I'm anot sure what else to provide
>>>>> so
>>>>> feel free to ask.
>>>>>
>>>>> Best,
>>>>> Martin
>>>>>
>>>>>
>>>>
>>>> Hi Martin,
>>>> I'll look into this, are there any specific steps to reproduce it?
>>>>
>>>> Thanks,
>>>> Nik
>>>>>
>>> Each server is a KVM Guest and has 18 bridges with the same master/slave
>>> relationships:
>>>
>>> bridge -> batman-adv -> {l2 tunnel, virtio device}
>>>
>>> Linus Lüssing from the batman-adv asked me to apply this patch to help
>>> debugging.
>>>
>>> v5.2-rc6-170-g728254541ebc with this patch yielded the following KASAN
>>> report, not sure if the additional information at the end is a result of
>>> the added patch though.
>>>
>>> Best,
>>> Martin
>>>
>>
>> I see a couple of issues that can cause out-of-bounds accesses in br_multicast.c
>> more specifically there're pskb_may_pull calls and accesses to stale skb pointers.
>> I've had these on my "to fix" list for some time now, will prepare, test the fixes and
>> send them for review. In a few minutes I'll send a test patch for you.
>> That being said, I thought you said you've been experiencing memory leaks, but below
>> reports are for out-of-bounds accesses, could you please clarify if you were
>> speaking about these or is there another issue as well ?
>> If you're experiencing memory leaks, are you sure they're related to the bridge ?
>> You could try kmemleak for those.
>>
>> Thank you,
>> Nik
>>
>
> we had been experiencing memory leaks on v4.19.37, thats why we started to turn on
> KASAN and kmemleak in the first place. This is when we found this use-after-free.
>
> The memory leak exists, and is a separate issue. Apparently kmemleak does not work,
> I suspect the early log size is too small
>
> root@gw02:~# echo scan > /sys/kernel/debug/kmemleak -bash: echo: write error: Device or resource busy
>
> CONFIG_HAVE_DEBUG_KMEMLEAK=y
> CONFIG_DEBUG_KMEMLEAK=y
> CONFIG_DEBUG_KMEMLEAK_EARLY_LOG_SIZE=400
> # CONFIG_DEBUG_KMEMLEAK_TEST is not set
> # CONFIG_DEBUG_KMEMLEAK_DEFAULT_OFF is not set
> CONFIG_DEBUG_KMEMLEAK_AUTO_SCAN=y
>
> I'll increase the early log size with the next build to try and get more information
> on the memory leak, I'll open a separate thread for that then.
>
> Thanks,
> Martin
>
I see, thanks for clarifying this. So on the KASAN could you please try the attached patch ?
Also could you please run the br_multicast_rcv+xxx addresses through
linux/scripts/faddr2line for your kernel/bridge:
usage: faddr2line [--list] <object file> <func+offset> <func+offset>...
Thanks,
Nik
[-- Attachment #2: 0001-net-bridge-mcast-fix-possible-uses-of-stale-pointers.patch --]
[-- Type: text/x-patch, Size: 3442 bytes --]
From 5358f2ad1228967d5e8a2dc21e0025651726a3b8 Mon Sep 17 00:00:00 2001
From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Date: Mon, 1 Jul 2019 20:31:14 +0300
Subject: [PATCH TEST] net: bridge: mcast: fix possible uses of stale pointers
Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
net/bridge/br_multicast.c | 24 ++++++++++++++----------
1 file changed, 14 insertions(+), 10 deletions(-)
diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index de22c8fbbb15..fbedef3fa930 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -917,6 +917,8 @@ static int br_ip4_multicast_igmp3_report(struct net_bridge *br,
len = skb_transport_offset(skb) + sizeof(*ih);
for (i = 0; i < num; i++) {
+ u16 nsrcs;
+
len += sizeof(*grec);
if (!ip_mc_may_pull(skb, len))
return -EINVAL;
@@ -924,8 +926,9 @@ static int br_ip4_multicast_igmp3_report(struct net_bridge *br,
grec = (void *)(skb->data + len - sizeof(*grec));
group = grec->grec_mca;
type = grec->grec_type;
+ nsrcs = ntohs(grec->grec_nsrcs);
- len += ntohs(grec->grec_nsrcs) * 4;
+ len += nsrcs * 4;
if (!ip_mc_may_pull(skb, len))
return -EINVAL;
@@ -946,7 +949,7 @@ static int br_ip4_multicast_igmp3_report(struct net_bridge *br,
src = eth_hdr(skb)->h_source;
if ((type == IGMPV3_CHANGE_TO_INCLUDE ||
type == IGMPV3_MODE_IS_INCLUDE) &&
- ntohs(grec->grec_nsrcs) == 0) {
+ nsrcs == 0) {
br_ip4_multicast_leave_group(br, port, group, vid, src);
} else {
err = br_ip4_multicast_add_group(br, port, group, vid,
@@ -983,7 +986,8 @@ static int br_ip6_multicast_mld2_report(struct net_bridge *br,
len = skb_transport_offset(skb) + sizeof(*icmp6h);
for (i = 0; i < num; i++) {
- __be16 *nsrcs, _nsrcs;
+ __be16 *_nsrcs, __nsrcs;
+ u16 nsrcs;
nsrcs_offset = len + offsetof(struct mld2_grec, grec_nsrcs);
@@ -991,12 +995,13 @@ static int br_ip6_multicast_mld2_report(struct net_bridge *br,
nsrcs_offset + sizeof(_nsrcs))
return -EINVAL;
- nsrcs = skb_header_pointer(skb, nsrcs_offset,
- sizeof(_nsrcs), &_nsrcs);
- if (!nsrcs)
+ _nsrcs = skb_header_pointer(skb, nsrcs_offset,
+ sizeof(_nsrcs), &__nsrcs);
+ if (!_nsrcs)
return -EINVAL;
- grec_len = struct_size(grec, grec_src, ntohs(*nsrcs));
+ nsrcs = ntohs(*_nsrcs);
+ grec_len = struct_size(grec, grec_src, nsrcs);
if (!ipv6_mc_may_pull(skb, len + grec_len))
return -EINVAL;
@@ -1021,7 +1026,7 @@ static int br_ip6_multicast_mld2_report(struct net_bridge *br,
src = eth_hdr(skb)->h_source;
if ((grec->grec_type == MLD2_CHANGE_TO_INCLUDE ||
grec->grec_type == MLD2_MODE_IS_INCLUDE) &&
- ntohs(*nsrcs) == 0) {
+ nsrcs == 0) {
br_ip6_multicast_leave_group(br, port, &grec->grec_mca,
vid, src);
} else {
@@ -1275,7 +1280,6 @@ static int br_ip6_multicast_query(struct net_bridge *br,
u16 vid)
{
unsigned int transport_len = ipv6_transport_len(skb);
- const struct ipv6hdr *ip6h = ipv6_hdr(skb);
struct mld_msg *mld;
struct net_bridge_mdb_entry *mp;
struct mld2_query *mld2q;
@@ -1319,7 +1323,7 @@ static int br_ip6_multicast_query(struct net_bridge *br,
if (is_general_query) {
saddr.proto = htons(ETH_P_IPV6);
- saddr.u.ip6 = ip6h->saddr;
+ saddr.u.ip6 = ipv6_hdr(skb)->saddr;
br_multicast_query_received(br, port, &br->ip6_other_query,
&saddr, max_delay);
--
2.21.0
^ permalink raw reply related
* Re: [RFC PATCH v2 2/2] Documentation: net: dsa: b53: Describe b53 configuration
From: Andrew Lunn @ 2019-07-01 17:35 UTC (permalink / raw)
To: Benedikt Spranger
Cc: Florian Fainelli, netdev, Sebastian Andrzej Siewior,
Kurt Kanzenbach, Vivien Didelot
In-Reply-To: <20190701154209.27656-3-b.spranger@linutronix.de>
> +Configuration without tagging support
> +-------------------------------------
How does this differ to the text you just added in the previous patch?
Do we need both?
Andrew
^ permalink raw reply
* Re: [PATCH bpf-next v2 1/3] bpf: allow wide (u64) aligned stores for some fields of bpf_sock_addr
From: Stanislav Fomichev @ 2019-07-01 17:34 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Stanislav Fomichev, Networking, bpf, David S. Miller,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Yonghong Song, kernel test robot
In-Reply-To: <CAEf4BzYRHjkuKKk+eR3-zbTFjjxae1Ks3SXr7kkAVgZxmVWU-A@mail.gmail.com>
On 07/01, Andrii Nakryiko wrote:
> On Mon, Jul 1, 2019 at 9:51 AM Stanislav Fomichev <sdf@google.com> wrote:
> >
> > Since commit cd17d7770578 ("bpf/tools: sync bpf.h") clang decided
> > that it can do a single u64 store into user_ip6[2] instead of two
> > separate u32 ones:
> >
> > # 17: (18) r2 = 0x100000000000000
> > # ; ctx->user_ip6[2] = bpf_htonl(DST_REWRITE_IP6_2);
> > # 19: (7b) *(u64 *)(r1 +16) = r2
> > # invalid bpf_context access off=16 size=8
> >
> > From the compiler point of view it does look like a correct thing
> > to do, so let's support it on the kernel side.
> >
> > Credit to Andrii Nakryiko for a proper implementation of
> > bpf_ctx_wide_store_ok.
> >
> > Cc: Andrii Nakryiko <andriin@fb.com>
> > Cc: Yonghong Song <yhs@fb.com>
> > Fixes: cd17d7770578 ("bpf/tools: sync bpf.h")
> > Reported-by: kernel test robot <rong.a.chen@intel.com>
> > Acked-by: Yonghong Song <yhs@fb.com>
> > Acked-by: Andrii Nakryiko <andriin@fb.com>
> > Signed-off-by: Stanislav Fomichev <sdf@google.com>
> > ---
> > include/linux/filter.h | 6 ++++++
> > include/uapi/linux/bpf.h | 4 ++--
> > net/core/filter.c | 22 ++++++++++++++--------
> > 3 files changed, 22 insertions(+), 10 deletions(-)
> >
> > diff --git a/include/linux/filter.h b/include/linux/filter.h
> > index 340f7d648974..3901007e36f1 100644
> > --- a/include/linux/filter.h
> > +++ b/include/linux/filter.h
> > @@ -746,6 +746,12 @@ bpf_ctx_narrow_access_ok(u32 off, u32 size, u32 size_default)
> > return size <= size_default && (size & (size - 1)) == 0;
> > }
> >
> > +#define bpf_ctx_wide_store_ok(off, size, type, field) \
> > + (size == sizeof(__u64) && \
> > + off >= offsetof(type, field) && \
> > + off + sizeof(__u64) <= offsetofend(type, field) && \
> > + off % sizeof(__u64) == 0)
> > +
> > #define bpf_classic_proglen(fprog) (fprog->len * sizeof(fprog->filter[0]))
> >
> > static inline void bpf_prog_lock_ro(struct bpf_prog *fp)
> > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> > index a396b516a2b2..586867fe6102 100644
> > --- a/include/uapi/linux/bpf.h
> > +++ b/include/uapi/linux/bpf.h
> > @@ -3237,7 +3237,7 @@ struct bpf_sock_addr {
> > __u32 user_ip4; /* Allows 1,2,4-byte read and 4-byte write.
> > * Stored in network byte order.
> > */
> > - __u32 user_ip6[4]; /* Allows 1,2,4-byte read an 4-byte write.
> > + __u32 user_ip6[4]; /* Allows 1,2,4-byte read an 4,8-byte write.
>
> typo: an -> and
Oh, I was thinking that it was an article :-/
Will send a v3 with a fix, thanks!
> > * Stored in network byte order.
> > */
> > __u32 user_port; /* Allows 4-byte read and write.
> > @@ -3249,7 +3249,7 @@ struct bpf_sock_addr {
> > __u32 msg_src_ip4; /* Allows 1,2,4-byte read an 4-byte write.
>
> same
>
> > * Stored in network byte order.
> > */
> > - __u32 msg_src_ip6[4]; /* Allows 1,2,4-byte read an 4-byte write.
> > + __u32 msg_src_ip6[4]; /* Allows 1,2,4-byte read an 4,8-byte write.
>
> the power of copy/paste! :)
>
> > * Stored in network byte order.
> > */
> > __bpf_md_ptr(struct bpf_sock *, sk);
> > diff --git a/net/core/filter.c b/net/core/filter.c
> > index dc8534be12fc..5d33f2146dab 100644
> > --- a/net/core/filter.c
> > +++ b/net/core/filter.c
> > @@ -6849,6 +6849,16 @@ static bool sock_addr_is_valid_access(int off, int size,
> > if (!bpf_ctx_narrow_access_ok(off, size, size_default))
> > return false;
> > } else {
> > + if (bpf_ctx_wide_store_ok(off, size,
> > + struct bpf_sock_addr,
> > + user_ip6))
> > + return true;
> > +
> > + if (bpf_ctx_wide_store_ok(off, size,
> > + struct bpf_sock_addr,
> > + msg_src_ip6))
> > + return true;
> > +
> > if (size != size_default)
> > return false;
> > }
> > @@ -7689,9 +7699,6 @@ static u32 xdp_convert_ctx_access(enum bpf_access_type type,
> > /* SOCK_ADDR_STORE_NESTED_FIELD_OFF() has semantic similar to
> > * SOCK_ADDR_LOAD_NESTED_FIELD_SIZE_OFF() but for store operation.
> > *
> > - * It doesn't support SIZE argument though since narrow stores are not
> > - * supported for now.
> > - *
> > * In addition it uses Temporary Field TF (member of struct S) as the 3rd
> > * "register" since two registers available in convert_ctx_access are not
> > * enough: we can't override neither SRC, since it contains value to store, nor
> > @@ -7699,7 +7706,7 @@ static u32 xdp_convert_ctx_access(enum bpf_access_type type,
> > * instructions. But we need a temporary place to save pointer to nested
> > * structure whose field we want to store to.
> > */
> > -#define SOCK_ADDR_STORE_NESTED_FIELD_OFF(S, NS, F, NF, OFF, TF) \
> > +#define SOCK_ADDR_STORE_NESTED_FIELD_OFF(S, NS, F, NF, SIZE, OFF, TF) \
> > do { \
> > int tmp_reg = BPF_REG_9; \
> > if (si->src_reg == tmp_reg || si->dst_reg == tmp_reg) \
> > @@ -7710,8 +7717,7 @@ static u32 xdp_convert_ctx_access(enum bpf_access_type type,
> > offsetof(S, TF)); \
> > *insn++ = BPF_LDX_MEM(BPF_FIELD_SIZEOF(S, F), tmp_reg, \
> > si->dst_reg, offsetof(S, F)); \
> > - *insn++ = BPF_STX_MEM( \
> > - BPF_FIELD_SIZEOF(NS, NF), tmp_reg, si->src_reg, \
> > + *insn++ = BPF_STX_MEM(SIZE, tmp_reg, si->src_reg, \
> > bpf_target_off(NS, NF, FIELD_SIZEOF(NS, NF), \
> > target_size) \
> > + OFF); \
> > @@ -7723,8 +7729,8 @@ static u32 xdp_convert_ctx_access(enum bpf_access_type type,
> > TF) \
> > do { \
> > if (type == BPF_WRITE) { \
> > - SOCK_ADDR_STORE_NESTED_FIELD_OFF(S, NS, F, NF, OFF, \
> > - TF); \
> > + SOCK_ADDR_STORE_NESTED_FIELD_OFF(S, NS, F, NF, SIZE, \
> > + OFF, TF); \
> > } else { \
> > SOCK_ADDR_LOAD_NESTED_FIELD_SIZE_OFF( \
> > S, NS, F, NF, SIZE, OFF); \
> > --
> > 2.22.0.410.gd8fdbe21b5-goog
> >
^ permalink raw reply
* Re: [RFC PATCH v2 1/2] Documentation: net: dsa: Describe DSA switch configuration
From: Andrew Lunn @ 2019-07-01 17:33 UTC (permalink / raw)
To: Benedikt Spranger
Cc: Florian Fainelli, netdev, Sebastian Andrzej Siewior,
Kurt Kanzenbach, Vivien Didelot
In-Reply-To: <20190701154209.27656-2-b.spranger@linutronix.de>
On Mon, Jul 01, 2019 at 05:42:08PM +0200, Benedikt Spranger wrote:
> Document DSA tagged and VLAN based switch configuration by showcases.
>
> Signed-off-by: Benedikt Spranger <b.spranger@linutronix.de>
> ---
> .../networking/dsa/configuration.rst | 292 ++++++++++++++++++
> Documentation/networking/dsa/index.rst | 1 +
> 2 files changed, 293 insertions(+)
> create mode 100644 Documentation/networking/dsa/configuration.rst
>
> diff --git a/Documentation/networking/dsa/configuration.rst b/Documentation/networking/dsa/configuration.rst
> new file mode 100644
> index 000000000000..55d6dce6500d
> --- /dev/null
> +++ b/Documentation/networking/dsa/configuration.rst
> @@ -0,0 +1,292 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +=======================================
> +DSA switch configuration from userspace
> +=======================================
> +
> +The DSA switch configuration is not integrated into the main userspace
> +network configuration suites by now and has to be performed manualy.
> +
> +.. _dsa-config-showcases:
> +
> +Configuration showcases
> +-----------------------
> +
> +To configure a DSA switch a couple of commands need to be executed. In this
> +documentation some common configuration scenarios are handled as showcases:
> +
> +*single port*
> + Every switch port acts as a different configurable Ethernet port
> +
> +*bridge*
> + Every switch port is part of one configurable Ethernet bridge
> +
> +*gateway*
> + Every switch port except one upstream port is part of a configurable
> + Ethernet bridge.
> + The upstream port acts as different configurable Ethernet port.
> +
> +All configurations are performed with tools from iproute2, wich is available at
which
Once that is fixed:
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: Use-after-free in br_multicast_rcv
From: Martin Weinelt @ 2019-07-01 17:31 UTC (permalink / raw)
To: Nikolay Aleksandrov, bridge, Roopa Prabhu; +Cc: netdev
In-Reply-To: <cc232ed3-9e02-ebb4-4901-9d617013abb8@cumulusnetworks.com>
Hi Nik,
On 7/1/19 7:03 PM, Nikolay Aleksandrov wrote:
> Hi Martin,
>
> On 01/07/2019 19:53, Martin Weinelt wrote:
>> Hi Nik,
>>
>> more info below.
>>
>> On 6/29/19 3:11 PM, nikolay@cumulusnetworks.com wrote:
>>> On 29 June 2019 14:54:44 EEST, Martin Weinelt <martin@linuxlounge.net> wrote:
>>>> Hello,
>>>>
>>>> we've recently been experiencing memory leaks on our Linux-based
>>>> routers,
>>>> at least as far back as v4.19.16.
>>>>
>>>> After rebuilding with KASAN it found a use-after-free in
>>>> br_multicast_rcv which I could reproduce on v5.2.0-rc6.
>>>>
>>>> Please find the KASAN report below, I'm anot sure what else to provide
>>>> so
>>>> feel free to ask.
>>>>
>>>> Best,
>>>> Martin
>>>>
>>>>
>>>
>>> Hi Martin,
>>> I'll look into this, are there any specific steps to reproduce it?
>>>
>>> Thanks,
>>> Nik
>>>>
>> Each server is a KVM Guest and has 18 bridges with the same master/slave
>> relationships:
>>
>> bridge -> batman-adv -> {l2 tunnel, virtio device}
>>
>> Linus Lüssing from the batman-adv asked me to apply this patch to help
>> debugging.
>>
>> v5.2-rc6-170-g728254541ebc with this patch yielded the following KASAN
>> report, not sure if the additional information at the end is a result of
>> the added patch though.
>>
>> Best,
>> Martin
>>
>
> I see a couple of issues that can cause out-of-bounds accesses in br_multicast.c
> more specifically there're pskb_may_pull calls and accesses to stale skb pointers.
> I've had these on my "to fix" list for some time now, will prepare, test the fixes and
> send them for review. In a few minutes I'll send a test patch for you.
> That being said, I thought you said you've been experiencing memory leaks, but below
> reports are for out-of-bounds accesses, could you please clarify if you were
> speaking about these or is there another issue as well ?
> If you're experiencing memory leaks, are you sure they're related to the bridge ?
> You could try kmemleak for those.
>
> Thank you,
> Nik
>
we had been experiencing memory leaks on v4.19.37, thats why we started to turn on
KASAN and kmemleak in the first place. This is when we found this use-after-free.
The memory leak exists, and is a separate issue. Apparently kmemleak does not work,
I suspect the early log size is too small
root@gw02:~# echo scan > /sys/kernel/debug/kmemleak -bash: echo: write error: Device or resource busy
CONFIG_HAVE_DEBUG_KMEMLEAK=y
CONFIG_DEBUG_KMEMLEAK=y
CONFIG_DEBUG_KMEMLEAK_EARLY_LOG_SIZE=400
# CONFIG_DEBUG_KMEMLEAK_TEST is not set
# CONFIG_DEBUG_KMEMLEAK_DEFAULT_OFF is not set
CONFIG_DEBUG_KMEMLEAK_AUTO_SCAN=y
I'll increase the early log size with the next build to try and get more information
on the memory leak, I'll open a separate thread for that then.
Thanks,
Martin
^ permalink raw reply
* Re: [net-next 08/15] iavf: Fix up debug print macro
From: Jeff Kirsher @ 2019-07-01 17:27 UTC (permalink / raw)
To: Joe Perches, davem; +Cc: netdev, nhorman, sassmann, Andrew Bowers
In-Reply-To: <9408eb59ecaa3e245fd71ec0211a34c3fb0e324b.camel@perches.com>
[-- Attachment #1: Type: text/plain, Size: 4682 bytes --]
On Sat, 2019-06-29 at 13:42 -0700, Joe Perches wrote:
> On Fri, 2019-06-28 at 15:49 -0700, Jeff Kirsher wrote:
> > This aligns the iavf_debug() macro with the other Intel drivers.
> >
> > Add the bus number, bus_id field to i40e_bus_info so output shows
> > each physical port(i.e func) in following format:
> > [[[[<domain>]:]<bus>]:][<slot>][.[<func>]]
> > domains are numbered from 0 to ffff), bus (0-ff), slot (0-1f) and
> > function (0-7).
> >
> > Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> > Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
> > ---
> > drivers/net/ethernet/intel/iavf/iavf_osdep.h | 10 +++++++---
> > 1 file changed, 7 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/intel/iavf/iavf_osdep.h
> > b/drivers/net/ethernet/intel/iavf/iavf_osdep.h
> > index d39684558597..a452ce90679a 100644
> > --- a/drivers/net/ethernet/intel/iavf/iavf_osdep.h
> > +++ b/drivers/net/ethernet/intel/iavf/iavf_osdep.h
> > @@ -44,8 +44,12 @@ struct iavf_virt_mem {
> > #define iavf_allocate_virt_mem(h, m, s)
> > iavf_allocate_virt_mem_d(h, m, s)
> > #define iavf_free_virt_mem(h, m) iavf_free_virt_mem_d(h, m)
> >
> > -#define iavf_debug(h, m, s, ...) iavf_debug_d(h, m, s,
> > ##__VA_ARGS__)
> > -extern void iavf_debug_d(void *hw, u32 mask, char *fmt_str, ...)
> > - __printf(3, 4);
> > +#define iavf_debug(h, m, s, ...) \
> > +do {
> > \
> > + if (((m) & (h)->debug_mask)) \
> > + pr_info("iavf %02x:%02x.%x " s, \
> > + (h)->bus.bus_id, (h)->bus.device, \
> > + (h)->bus.func, ##__VA_ARGS__); \
> > +} while (0)
>
> Why not change the function to do this?
>
> And if this is really wanted this particular way
> the now unused function should be removed too.
>
> But I suggest emitting at KERN_DEBUG and using
> the more typical %pV vsprintf extension.
I see what you are saying, I was only looking at the macro in the osdep
to align with our other drivers and sync up with our internal driver
code. Let me review the iavf driver debug function to see if there are
additional fix-ups/sync-ups needed.
>
> ---
>
> drivers/net/ethernet/intel/iavf/iavf_main.c | 25 ++++++++++++++--
> ---------
> drivers/net/ethernet/intel/iavf/iavf_osdep.h | 9 ++++++---
> 2 files changed, 20 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c
> b/drivers/net/ethernet/intel/iavf/iavf_main.c
> index 881561b36083..8504fd71d398 100644
> --- a/drivers/net/ethernet/intel/iavf/iavf_main.c
> +++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
> @@ -143,25 +143,28 @@ enum iavf_status iavf_free_virt_mem_d(struct
> iavf_hw *hw,
> }
>
> /**
> - * iavf_debug_d - OS dependent version of debug printing
> + * _iavf_debug - OS dependent version of debug printing
> * @hw: pointer to the HW structure
> * @mask: debug level mask
> - * @fmt_str: printf-type format description
> + * @fmt: printf-type format description
> **/
> -void iavf_debug_d(void *hw, u32 mask, char *fmt_str, ...)
> +void _iavf_debug(const struct iavf_hw *hw, u32 mask, const char
> *fmt, ...)
> {
> - char buf[512];
> - va_list argptr;
> + struct va_format vaf;
> + va_list args;
>
> - if (!(mask & ((struct iavf_hw *)hw)->debug_mask))
> + if (!(hw->debug_mask & mask))
> return;
>
> - va_start(argptr, fmt_str);
> - vsnprintf(buf, sizeof(buf), fmt_str, argptr);
> - va_end(argptr);
> + va_start(args, fmt);
>
> - /* the debug string is already formatted with a newline */
> - pr_info("%s", buf);
> + vaf.fmt = fmt;
> + vaf.va = &args;
> +
> + pr_debug("iavf %02x:%02x.%x %pV",
> + hw->bus.bus_id, hw->bus.device, hw->bus.func, &vaf);
> +
> + va_end(args);
> }
>
> /**
> diff --git a/drivers/net/ethernet/intel/iavf/iavf_osdep.h
> b/drivers/net/ethernet/intel/iavf/iavf_osdep.h
> index d39684558597..0e6ac7d262c8 100644
> --- a/drivers/net/ethernet/intel/iavf/iavf_osdep.h
> +++ b/drivers/net/ethernet/intel/iavf/iavf_osdep.h
> @@ -44,8 +44,11 @@ struct iavf_virt_mem {
> #define iavf_allocate_virt_mem(h, m, s) iavf_allocate_virt_mem_d(h,
> m, s)
> #define iavf_free_virt_mem(h, m) iavf_free_virt_mem_d(h, m)
>
> -#define iavf_debug(h, m, s, ...) iavf_debug_d(h, m, s,
> ##__VA_ARGS__)
> -extern void iavf_debug_d(void *hw, u32 mask, char *fmt_str, ...)
> - __printf(3, 4);
> +struct iavf_hw;
> +
> +__printf(3, 4)
> +void _iavf_debug(const struct iavf_hw *hw, u32 mask, const char
> *fmt, ...);
> +#define iavf_debug(hw, mask, fmt, ...)
> \
> + _iavf_debug(hw, mask, fmt, ##__VA_ARGS__)
>
> #endif /* _IAVF_OSDEP_H_ */
>
>
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v4 bpf-next 8/9] selftests/bpf: add kprobe/uprobe selftests
From: Yonghong Song @ 2019-07-01 17:22 UTC (permalink / raw)
To: Andrii Nakryiko, andrii.nakryiko@gmail.com, bpf@vger.kernel.org,
netdev@vger.kernel.org, Alexei Starovoitov, daniel@iogearbox.net,
Kernel Team, sdf@fomichev.me, Song Liu
In-Reply-To: <20190629034906.1209916-9-andriin@fb.com>
On 6/28/19 8:49 PM, Andrii Nakryiko wrote:
> Add tests verifying kprobe/kretprobe/uprobe/uretprobe APIs work as
> expected.
>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> Reviewed-by: Stanislav Fomichev <sdf@google.com>
> Acked-by: Song Liu <songliubraving@fb.com>
> ---
> .../selftests/bpf/prog_tests/attach_probe.c | 155 ++++++++++++++++++
> .../selftests/bpf/progs/test_attach_probe.c | 55 +++++++
> 2 files changed, 210 insertions(+)
> create mode 100644 tools/testing/selftests/bpf/prog_tests/attach_probe.c
> create mode 100644 tools/testing/selftests/bpf/progs/test_attach_probe.c
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/attach_probe.c b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
> new file mode 100644
> index 000000000000..f22929063c58
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
> @@ -0,0 +1,155 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <test_progs.h>
> +
> +ssize_t get_base_addr() {
> + size_t start;
> + char buf[256];
> + FILE *f;
> +
> + f = fopen("/proc/self/maps", "r");
> + if (!f)
> + return -errno;
> +
> + while (fscanf(f, "%zx-%*x %s %*s\n", &start, buf) == 2) {
> + if (strcmp(buf, "r-xp") == 0) {
> + fclose(f);
> + return start;
> + }
> + }
> +
> + fclose(f);
> + return -EINVAL;
> +}
> +
> +void test_attach_probe(void)
> +{
> + const char *kprobe_name = "kprobe/sys_nanosleep";
> + const char *kretprobe_name = "kretprobe/sys_nanosleep";
> + const char *uprobe_name = "uprobe/trigger_func";
> + const char *uretprobe_name = "uretprobe/trigger_func";
> + const int kprobe_idx = 0, kretprobe_idx = 1;
> + const int uprobe_idx = 2, uretprobe_idx = 3;
> + const char *file = "./test_attach_probe.o";
> + struct bpf_program *kprobe_prog, *kretprobe_prog;
> + struct bpf_program *uprobe_prog, *uretprobe_prog;
> + struct bpf_object *obj;
> + int err, prog_fd, duration = 0, res;
> + struct bpf_link *kprobe_link = NULL;
> + struct bpf_link *kretprobe_link = NULL;
> + struct bpf_link *uprobe_link = NULL;
> + struct bpf_link *uretprobe_link = NULL;
> + int results_map_fd;
> + size_t uprobe_offset;
> + ssize_t base_addr;
> +
> + base_addr = get_base_addr();
> + if (CHECK(base_addr < 0, "get_base_addr",
> + "failed to find base addr: %zd", base_addr))
> + return;
> + uprobe_offset = (size_t)&get_base_addr - base_addr;
> +
> + /* load programs */
> + err = bpf_prog_load(file, BPF_PROG_TYPE_KPROBE, &obj, &prog_fd);
> + if (CHECK(err, "obj_load", "err %d errno %d\n", err, errno))
> + return;
> +
> + kprobe_prog = bpf_object__find_program_by_title(obj, kprobe_name);
> + if (CHECK(!kprobe_prog, "find_probe",
> + "prog '%s' not found\n", kprobe_name))
> + goto cleanup;
> + kretprobe_prog = bpf_object__find_program_by_title(obj, kretprobe_name);
> + if (CHECK(!kretprobe_prog, "find_probe",
> + "prog '%s' not found\n", kretprobe_name))
> + goto cleanup;
> + uprobe_prog = bpf_object__find_program_by_title(obj, uprobe_name);
> + if (CHECK(!uprobe_prog, "find_probe",
> + "prog '%s' not found\n", uprobe_name))
> + goto cleanup;
> + uretprobe_prog = bpf_object__find_program_by_title(obj, uretprobe_name);
> + if (CHECK(!uretprobe_prog, "find_probe",
> + "prog '%s' not found\n", uretprobe_name))
> + goto cleanup;
> +
> + /* load maps */
> + results_map_fd = bpf_find_map(__func__, obj, "results_map");
> + if (CHECK(results_map_fd < 0, "find_results_map",
> + "err %d\n", results_map_fd))
> + goto cleanup;
> +
> + kprobe_link = bpf_program__attach_kprobe(kprobe_prog,
> + false /* retprobe */,
> + "sys_nanosleep");
Another thing, in current kernel, `sys_nanosleep`does not exist
on x64. It is `__x64_sys_nanosleep`. See samples/bpf/bpf_load.c
function load_and_attach(). You can use macros to differentiate
different architectures.
> + if (CHECK(IS_ERR(kprobe_link), "attach_kprobe",
> + "err %ld\n", PTR_ERR(kprobe_link)))
> + goto cleanup;
> +
> + kretprobe_link = bpf_program__attach_kprobe(kretprobe_prog,
> + true /* retprobe */,
> + "sys_nanosleep");
> + if (CHECK(IS_ERR(kretprobe_link), "attach_kretprobe",
> + "err %ld\n", PTR_ERR(kretprobe_link)))
> + goto cleanup;
> +
> + uprobe_link = bpf_program__attach_uprobe(uprobe_prog,
> + false /* retprobe */,
> + 0 /* self pid */,
> + "/proc/self/exe",
> + uprobe_offset);
> + if (CHECK(IS_ERR(uprobe_link), "attach_uprobe",
> + "err %ld\n", PTR_ERR(uprobe_link)))
> + goto cleanup;
> +
> + uretprobe_link = bpf_program__attach_uprobe(uretprobe_prog,
> + true /* retprobe */,
> + -1 /* any pid */,
> + "/proc/self/exe",
> + uprobe_offset);
> + if (CHECK(IS_ERR(uretprobe_link), "attach_uretprobe",
> + "err %ld\n", PTR_ERR(uretprobe_link)))
> + goto cleanup;
> +
> + /* trigger & validate kprobe && kretprobe */
> + usleep(1);
> +
> + err = bpf_map_lookup_elem(results_map_fd, &kprobe_idx, &res);
> + if (CHECK(err, "get_kprobe_res",
> + "failed to get kprobe res: %d\n", err))
> + goto cleanup;
> + if (CHECK(res != kprobe_idx + 1, "check_kprobe_res",
> + "wrong kprobe res: %d\n", res))
> + goto cleanup;
> +
> + err = bpf_map_lookup_elem(results_map_fd, &kretprobe_idx, &res);
> + if (CHECK(err, "get_kretprobe_res",
> + "failed to get kretprobe res: %d\n", err))
> + goto cleanup;
> + if (CHECK(res != kretprobe_idx + 1, "check_kretprobe_res",
> + "wrong kretprobe res: %d\n", res))
> + goto cleanup;
> +
> + /* trigger & validate uprobe & uretprobe */
> + get_base_addr();
> +
> + err = bpf_map_lookup_elem(results_map_fd, &uprobe_idx, &res);
> + if (CHECK(err, "get_uprobe_res",
> + "failed to get uprobe res: %d\n", err))
> + goto cleanup;
> + if (CHECK(res != uprobe_idx + 1, "check_uprobe_res",
> + "wrong uprobe res: %d\n", res))
> + goto cleanup;
> +
> + err = bpf_map_lookup_elem(results_map_fd, &uretprobe_idx, &res);
> + if (CHECK(err, "get_uretprobe_res",
> + "failed to get uretprobe res: %d\n", err))
> + goto cleanup;
> + if (CHECK(res != uretprobe_idx + 1, "check_uretprobe_res",
> + "wrong uretprobe res: %d\n", res))
> + goto cleanup;
> +
> +cleanup:
> + bpf_link__destroy(kprobe_link);
> + bpf_link__destroy(kretprobe_link);
> + bpf_link__destroy(uprobe_link);
> + bpf_link__destroy(uretprobe_link);
> + bpf_object__close(obj);
> +}
[...]
^ permalink raw reply
* Re: [PATCH net v2] ipv4: don't set IPv6 only flags to IPv4 addresses
From: David Ahern @ 2019-07-01 17:19 UTC (permalink / raw)
To: Matteo Croce, netdev
Cc: linux-kernel, David S. Miller, Alexey Kuznetsov,
Hideaki YOSHIFUJI
In-Reply-To: <20190701170155.1967-1-mcroce@redhat.com>
On 7/1/19 11:01 AM, Matteo Croce wrote:
> Avoid the situation where an IPV6 only flag is applied to an IPv4 address:
>
> # ip addr add 192.0.2.1/24 dev dummy0 nodad home mngtmpaddr noprefixroute
> # ip -4 addr show dev dummy0
> 2: dummy0: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue state UNKNOWN group default qlen 1000
> inet 192.0.2.1/24 scope global noprefixroute dummy0
> valid_lft forever preferred_lft forever
>
> Or worse, by sending a malicious netlink command:
>
> # ip -4 addr show dev dummy0
> 2: dummy0: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue state UNKNOWN group default qlen 1000
> inet 192.0.2.1/24 scope global nodad optimistic dadfailed home tentative mngtmpaddr noprefixroute stable-privacy dummy0
> valid_lft forever preferred_lft forever
>
> Signed-off-by: Matteo Croce <mcroce@redhat.com>
> ---
> net/ipv4/devinet.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
> index c6bd0f7a020a..c5ebfa199794 100644
> --- a/net/ipv4/devinet.c
> +++ b/net/ipv4/devinet.c
> @@ -62,6 +62,11 @@
> #include <net/net_namespace.h>
> #include <net/addrconf.h>
>
> +#define IPV6ONLY_FLAGS \
> + (IFA_F_NODAD | IFA_F_OPTIMISTIC | IFA_F_DADFAILED | \
> + IFA_F_HOMEADDRESS | IFA_F_TENTATIVE | \
> + IFA_F_MANAGETEMPADDR | IFA_F_STABLE_PRIVACY)
> +
> static struct ipv4_devconf ipv4_devconf = {
> .data = {
> [IPV4_DEVCONF_ACCEPT_REDIRECTS - 1] = 1,
> @@ -468,6 +473,9 @@ static int __inet_insert_ifa(struct in_ifaddr *ifa, struct nlmsghdr *nlh,
> ifa->ifa_flags &= ~IFA_F_SECONDARY;
> last_primary = &in_dev->ifa_list;
>
> + /* Don't set IPv6 only flags to IPv4 addresses */
> + ifa->ifa_flags &= ~IPV6ONLY_FLAGS;
> +
> for (ifap = &in_dev->ifa_list; (ifa1 = *ifap) != NULL;
> ifap = &ifa1->ifa_next) {
> if (!(ifa1->ifa_flags & IFA_F_SECONDARY) &&
>
I guess at this point we can fail the address add, so this is the best
option. rtm_to_ifaddr could set a message in extack about invalid flags
- not fail the change, just warn the user that flags will be ignored.
Reviewed-by: David Ahern <dsahern@gmail.com>
^ permalink raw reply
* Re: [PATCH v4 bpf-next 8/9] selftests/bpf: add kprobe/uprobe selftests
From: Yonghong Song @ 2019-07-01 17:18 UTC (permalink / raw)
To: Andrii Nakryiko, andrii.nakryiko@gmail.com, bpf@vger.kernel.org,
netdev@vger.kernel.org, Alexei Starovoitov, daniel@iogearbox.net,
Kernel Team, sdf@fomichev.me, Song Liu
In-Reply-To: <20190629034906.1209916-9-andriin@fb.com>
On 6/28/19 8:49 PM, Andrii Nakryiko wrote:
> Add tests verifying kprobe/kretprobe/uprobe/uretprobe APIs work as
> expected.
>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> Reviewed-by: Stanislav Fomichev <sdf@google.com>
> Acked-by: Song Liu <songliubraving@fb.com>
> ---
> .../selftests/bpf/prog_tests/attach_probe.c | 155 ++++++++++++++++++
> .../selftests/bpf/progs/test_attach_probe.c | 55 +++++++
> 2 files changed, 210 insertions(+)
> create mode 100644 tools/testing/selftests/bpf/prog_tests/attach_probe.c
> create mode 100644 tools/testing/selftests/bpf/progs/test_attach_probe.c
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/attach_probe.c b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
> new file mode 100644
> index 000000000000..f22929063c58
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
> @@ -0,0 +1,155 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <test_progs.h>
> +
> +ssize_t get_base_addr() {
> + size_t start;
> + char buf[256];
> + FILE *f;
> +
> + f = fopen("/proc/self/maps", "r");
> + if (!f)
> + return -errno;
> +
> + while (fscanf(f, "%zx-%*x %s %*s\n", &start, buf) == 2) {
> + if (strcmp(buf, "r-xp") == 0) {
> + fclose(f);
> + return start;
> + }
> + }
> +
> + fclose(f);
> + return -EINVAL;
> +}
> +
> +void test_attach_probe(void)
> +{
> + const char *kprobe_name = "kprobe/sys_nanosleep";
> + const char *kretprobe_name = "kretprobe/sys_nanosleep";
> + const char *uprobe_name = "uprobe/trigger_func";
> + const char *uretprobe_name = "uretprobe/trigger_func";
> + const int kprobe_idx = 0, kretprobe_idx = 1;
> + const int uprobe_idx = 2, uretprobe_idx = 3;
> + const char *file = "./test_attach_probe.o";
> + struct bpf_program *kprobe_prog, *kretprobe_prog;
> + struct bpf_program *uprobe_prog, *uretprobe_prog;
> + struct bpf_object *obj;
> + int err, prog_fd, duration = 0, res;
> + struct bpf_link *kprobe_link = NULL;
> + struct bpf_link *kretprobe_link = NULL;
> + struct bpf_link *uprobe_link = NULL;
> + struct bpf_link *uretprobe_link = NULL;
> + int results_map_fd;
> + size_t uprobe_offset;
> + ssize_t base_addr;
> +
> + base_addr = get_base_addr();
> + if (CHECK(base_addr < 0, "get_base_addr",
> + "failed to find base addr: %zd", base_addr))
> + return;
> + uprobe_offset = (size_t)&get_base_addr - base_addr;
> +
> + /* load programs */
> + err = bpf_prog_load(file, BPF_PROG_TYPE_KPROBE, &obj, &prog_fd);
> + if (CHECK(err, "obj_load", "err %d errno %d\n", err, errno))
> + return;
> +
> + kprobe_prog = bpf_object__find_program_by_title(obj, kprobe_name);
> + if (CHECK(!kprobe_prog, "find_probe",
> + "prog '%s' not found\n", kprobe_name))
> + goto cleanup;
> + kretprobe_prog = bpf_object__find_program_by_title(obj, kretprobe_name);
> + if (CHECK(!kretprobe_prog, "find_probe",
> + "prog '%s' not found\n", kretprobe_name))
> + goto cleanup;
> + uprobe_prog = bpf_object__find_program_by_title(obj, uprobe_name);
> + if (CHECK(!uprobe_prog, "find_probe",
> + "prog '%s' not found\n", uprobe_name))
> + goto cleanup;
> + uretprobe_prog = bpf_object__find_program_by_title(obj, uretprobe_name);
> + if (CHECK(!uretprobe_prog, "find_probe",
> + "prog '%s' not found\n", uretprobe_name))
> + goto cleanup;
> +
> + /* load maps */
> + results_map_fd = bpf_find_map(__func__, obj, "results_map");
> + if (CHECK(results_map_fd < 0, "find_results_map",
> + "err %d\n", results_map_fd))
> + goto cleanup;
> +
> + kprobe_link = bpf_program__attach_kprobe(kprobe_prog,
> + false /* retprobe */,
> + "sys_nanosleep");
> + if (CHECK(IS_ERR(kprobe_link), "attach_kprobe",
> + "err %ld\n", PTR_ERR(kprobe_link)))
> + goto cleanup;
> +
> + kretprobe_link = bpf_program__attach_kprobe(kretprobe_prog,
> + true /* retprobe */,
> + "sys_nanosleep");
> + if (CHECK(IS_ERR(kretprobe_link), "attach_kretprobe",
> + "err %ld\n", PTR_ERR(kretprobe_link)))
> + goto cleanup;
> +
> + uprobe_link = bpf_program__attach_uprobe(uprobe_prog,
> + false /* retprobe */,
> + 0 /* self pid */,
> + "/proc/self/exe",
> + uprobe_offset);
> + if (CHECK(IS_ERR(uprobe_link), "attach_uprobe",
> + "err %ld\n", PTR_ERR(uprobe_link)))
> + goto cleanup;
> +
> + uretprobe_link = bpf_program__attach_uprobe(uretprobe_prog,
> + true /* retprobe */,
> + -1 /* any pid */,
> + "/proc/self/exe",
> + uprobe_offset);
> + if (CHECK(IS_ERR(uretprobe_link), "attach_uretprobe",
> + "err %ld\n", PTR_ERR(uretprobe_link)))
> + goto cleanup;
> +
> + /* trigger & validate kprobe && kretprobe */
> + usleep(1);
> +
> + err = bpf_map_lookup_elem(results_map_fd, &kprobe_idx, &res);
> + if (CHECK(err, "get_kprobe_res",
> + "failed to get kprobe res: %d\n", err))
> + goto cleanup;
> + if (CHECK(res != kprobe_idx + 1, "check_kprobe_res",
> + "wrong kprobe res: %d\n", res))
> + goto cleanup;
> +
> + err = bpf_map_lookup_elem(results_map_fd, &kretprobe_idx, &res);
> + if (CHECK(err, "get_kretprobe_res",
> + "failed to get kretprobe res: %d\n", err))
> + goto cleanup;
> + if (CHECK(res != kretprobe_idx + 1, "check_kretprobe_res",
> + "wrong kretprobe res: %d\n", res))
> + goto cleanup;
> +
> + /* trigger & validate uprobe & uretprobe */
> + get_base_addr();
> +
> + err = bpf_map_lookup_elem(results_map_fd, &uprobe_idx, &res);
> + if (CHECK(err, "get_uprobe_res",
> + "failed to get uprobe res: %d\n", err))
> + goto cleanup;
> + if (CHECK(res != uprobe_idx + 1, "check_uprobe_res",
> + "wrong uprobe res: %d\n", res))
> + goto cleanup;
> +
> + err = bpf_map_lookup_elem(results_map_fd, &uretprobe_idx, &res);
> + if (CHECK(err, "get_uretprobe_res",
> + "failed to get uretprobe res: %d\n", err))
> + goto cleanup;
> + if (CHECK(res != uretprobe_idx + 1, "check_uretprobe_res",
> + "wrong uretprobe res: %d\n", res))
> + goto cleanup;
> +
> +cleanup:
> + bpf_link__destroy(kprobe_link);
> + bpf_link__destroy(kretprobe_link);
> + bpf_link__destroy(uprobe_link);
> + bpf_link__destroy(uretprobe_link);
if any error happens, kprobe_link etc. will be a non-NULL pointer
indicating an error. the above bpf_link__destroy() won't work
properly. The same for patch #9.
> + bpf_object__close(obj);
> +}
> diff --git a/tools/testing/selftests/bpf/progs/test_attach_probe.c b/tools/testing/selftests/bpf/progs/test_attach_probe.c
> new file mode 100644
> index 000000000000..7a7c5cd728c8
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/test_attach_probe.c
> @@ -0,0 +1,55 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (c) 2017 Facebook
> +
> +#include <linux/ptrace.h>
> +#include <linux/bpf.h>
> +#include "bpf_helpers.h"
> +
> +struct {
> + int type;
> + int max_entries;
> + int *key;
> + int *value;
> +} results_map SEC(".maps") = {
> + .type = BPF_MAP_TYPE_ARRAY,
> + .max_entries = 4,
> +};
> +
> +SEC("kprobe/sys_nanosleep")
> +int handle_sys_nanosleep_entry(struct pt_regs *ctx)
> +{
> + const int key = 0, value = 1;
> +
> + bpf_map_update_elem(&results_map, &key, &value, 0);
> + return 0;
> +}
> +
> +SEC("kretprobe/sys_nanosleep")
> +int handle_sys_getpid_return(struct pt_regs *ctx)
> +{
> + const int key = 1, value = 2;
> +
> + bpf_map_update_elem(&results_map, &key, &value, 0);
> + return 0;
> +}
> +
> +SEC("uprobe/trigger_func")
> +int handle_uprobe_entry(struct pt_regs *ctx)
> +{
> + const int key = 2, value = 3;
> +
> + bpf_map_update_elem(&results_map, &key, &value, 0);
> + return 0;
> +}
> +
> +SEC("uretprobe/trigger_func")
> +int handle_uprobe_return(struct pt_regs *ctx)
> +{
> + const int key = 3, value = 4;
> +
> + bpf_map_update_elem(&results_map, &key, &value, 0);
> + return 0;
> +}
> +
> +char _license[] SEC("license") = "GPL";
> +__u32 _version SEC("version") = 1;
>
^ permalink raw reply
* Re: [PATCH v4 bpf-next 7/9] selftests/bpf: switch test to new attach_perf_event API
From: Yonghong Song @ 2019-07-01 17:16 UTC (permalink / raw)
To: Andrii Nakryiko, andrii.nakryiko@gmail.com, bpf@vger.kernel.org,
netdev@vger.kernel.org, Alexei Starovoitov, daniel@iogearbox.net,
Kernel Team, sdf@fomichev.me, Song Liu
In-Reply-To: <20190629034906.1209916-8-andriin@fb.com>
On 6/28/19 8:49 PM, Andrii Nakryiko wrote:
> Use new bpf_program__attach_perf_event() in test previously relying on
> direct ioctl manipulations.
>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> Reviewed-by: Stanislav Fomichev <sdf@google.com>
> Acked-by: Song Liu <songliubraving@fb.com>
> ---
> .../bpf/prog_tests/stacktrace_build_id_nmi.c | 31 +++++++++----------
> 1 file changed, 15 insertions(+), 16 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id_nmi.c b/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id_nmi.c
> index 1c1a2f75f3d8..9557b7dfb782 100644
> --- a/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id_nmi.c
> +++ b/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id_nmi.c
> @@ -17,6 +17,7 @@ static __u64 read_perf_max_sample_freq(void)
> void test_stacktrace_build_id_nmi(void)
> {
> int control_map_fd, stackid_hmap_fd, stackmap_fd, stack_amap_fd;
> + const char *prog_name = "tracepoint/random/urandom_read";
> const char *file = "./test_stacktrace_build_id.o";
> int err, pmu_fd, prog_fd;
> struct perf_event_attr attr = {
> @@ -25,7 +26,9 @@ void test_stacktrace_build_id_nmi(void)
> .config = PERF_COUNT_HW_CPU_CYCLES,
> };
> __u32 key, previous_key, val, duration = 0;
> + struct bpf_program *prog;
> struct bpf_object *obj;
> + struct bpf_link *link;
> char buf[256];
> int i, j;
> struct bpf_stack_build_id id_offs[PERF_MAX_STACK_DEPTH];
> @@ -39,6 +42,10 @@ void test_stacktrace_build_id_nmi(void)
> if (CHECK(err, "prog_load", "err %d errno %d\n", err, errno))
> return;
>
> + prog = bpf_object__find_program_by_title(obj, prog_name);
> + if (CHECK(!prog, "find_prog", "prog '%s' not found\n", prog_name))
> + goto close_prog;
> +
> pmu_fd = syscall(__NR_perf_event_open, &attr, -1 /* pid */,
> 0 /* cpu 0 */, -1 /* group id */,
> 0 /* flags */);
> @@ -47,15 +54,12 @@ void test_stacktrace_build_id_nmi(void)
> pmu_fd, errno))
> goto close_prog;
>
> - err = ioctl(pmu_fd, PERF_EVENT_IOC_ENABLE, 0);
> - if (CHECK(err, "perf_event_ioc_enable", "err %d errno %d\n",
> - err, errno))
> - goto close_pmu;
> -
> - err = ioctl(pmu_fd, PERF_EVENT_IOC_SET_BPF, prog_fd);
> - if (CHECK(err, "perf_event_ioc_set_bpf", "err %d errno %d\n",
> - err, errno))
> - goto disable_pmu;
> + link = bpf_program__attach_perf_event(prog, pmu_fd);
> + if (CHECK(IS_ERR(link), "attach_perf_event",
> + "err %ld\n", PTR_ERR(link))) {
> + close(pmu_fd);
> + goto close_prog;
> + }
>
> /* find map fds */
> control_map_fd = bpf_find_map(__func__, obj, "control_map");
> @@ -134,8 +138,7 @@ void test_stacktrace_build_id_nmi(void)
> * try it one more time.
> */
> if (build_id_matches < 1 && retry--) {
> - ioctl(pmu_fd, PERF_EVENT_IOC_DISABLE);
> - close(pmu_fd);
> + bpf_link__destroy(link);
> bpf_object__close(obj);
> printf("%s:WARN:Didn't find expected build ID from the map, retrying\n",
> __func__);
> @@ -154,11 +157,7 @@ void test_stacktrace_build_id_nmi(void)
> */
>
> disable_pmu:
> - ioctl(pmu_fd, PERF_EVENT_IOC_DISABLE);
> -
> -close_pmu:
> - close(pmu_fd);
> -
> + bpf_link__destroy(link);
There is a problem in bpf_link__destroy(link).
The "link = bpf_program__attach_perf_event(prog, pmu_fd)"
may be an error pointer (IS_ERR(link) is true), in which
case, link should be reset to NULL and then call
bpf_link__destroy(link). Otherwise, the program may
segfault or function incorrectly.
> close_prog:
> bpf_object__close(obj);
> }
>
^ permalink raw reply
* Re: [PATCH v4 bpf-next 6/9] libbpf: add raw tracepoint attach API
From: Yonghong Song @ 2019-07-01 17:13 UTC (permalink / raw)
To: Andrii Nakryiko, andrii.nakryiko@gmail.com, bpf@vger.kernel.org,
netdev@vger.kernel.org, Alexei Starovoitov, daniel@iogearbox.net,
Kernel Team, sdf@fomichev.me, Song Liu
In-Reply-To: <20190629034906.1209916-7-andriin@fb.com>
On 6/28/19 8:49 PM, Andrii Nakryiko wrote:
> Add a wrapper utilizing bpf_link "infrastructure" to allow attaching BPF
> programs to raw tracepoints.
>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> Acked-by: Song Liu <songliubraving@fb.com>
> ---
> tools/lib/bpf/libbpf.c | 37 +++++++++++++++++++++++++++++++++++++
> tools/lib/bpf/libbpf.h | 3 +++
> tools/lib/bpf/libbpf.map | 1 +
> 3 files changed, 41 insertions(+)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 8ad4f915df38..f8c7a7ecb35e 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -4263,6 +4263,43 @@ struct bpf_link *bpf_program__attach_tracepoint(struct bpf_program *prog,
> return link;
> }
>
> +static int bpf_link__destroy_fd(struct bpf_link *link)
> +{
> + struct bpf_link_fd *l = (void *)link;
> +
> + return close(l->fd);
> +}
> +
> +struct bpf_link *bpf_program__attach_raw_tracepoint(struct bpf_program *prog,
> + const char *tp_name)
> +{
> + char errmsg[STRERR_BUFSIZE];
> + struct bpf_link_fd *link;
> + int prog_fd, pfd;
> +
> + prog_fd = bpf_program__fd(prog);
> + if (prog_fd < 0) {
> + pr_warning("program '%s': can't attach before loaded\n",
> + bpf_program__title(prog, false));
> + return ERR_PTR(-EINVAL);
> + }
> +
> + link = malloc(sizeof(*link));
> + link->link.destroy = &bpf_link__destroy_fd;
You can move the "link = malloc(...)" etc. after
bpf_raw_tracepoint_open(). That way, you do not need to free(link)
in the error case.
> +
> + pfd = bpf_raw_tracepoint_open(tp_name, prog_fd);
> + if (pfd < 0) {
> + pfd = -errno;
> + free(link);
> + pr_warning("program '%s': failed to attach to raw tracepoint '%s': %s\n",
> + bpf_program__title(prog, false), tp_name,
> + libbpf_strerror_r(pfd, errmsg, sizeof(errmsg)));
> + return ERR_PTR(pfd);
> + }
> + link->fd = pfd;
> + return (struct bpf_link *)link;
> +}
> +
> enum bpf_perf_event_ret
> bpf_perf_event_read_simple(void *mmap_mem, size_t mmap_size, size_t page_size,
> void **copy_mem, size_t *copy_size,
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index 60611f4b4e1d..f55933784f95 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -182,6 +182,9 @@ LIBBPF_API struct bpf_link *
> bpf_program__attach_tracepoint(struct bpf_program *prog,
> const char *tp_category,
> const char *tp_name);
> +LIBBPF_API struct bpf_link *
> +bpf_program__attach_raw_tracepoint(struct bpf_program *prog,
> + const char *tp_name);
>
> struct bpf_insn;
>
> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> index 3c618b75ef65..e6b7d4edbc93 100644
> --- a/tools/lib/bpf/libbpf.map
> +++ b/tools/lib/bpf/libbpf.map
> @@ -171,6 +171,7 @@ LIBBPF_0.0.4 {
> bpf_object__load_xattr;
> bpf_program__attach_kprobe;
> bpf_program__attach_perf_event;
> + bpf_program__attach_raw_tracepoint;
> bpf_program__attach_tracepoint;
> bpf_program__attach_uprobe;
> btf_dump__dump_type;
>
^ permalink raw reply
* Re: [PATCH v4 bpf-next 5/9] libbpf: add tracepoint attach API
From: Yonghong Song @ 2019-07-01 17:10 UTC (permalink / raw)
To: Andrii Nakryiko, andrii.nakryiko@gmail.com, bpf@vger.kernel.org,
netdev@vger.kernel.org, Alexei Starovoitov, daniel@iogearbox.net,
Kernel Team, sdf@fomichev.me, Song Liu
In-Reply-To: <20190629034906.1209916-6-andriin@fb.com>
On 6/28/19 8:49 PM, Andrii Nakryiko wrote:
> Allow attaching BPF programs to kernel tracepoint BPF hooks specified by
> category and name.
>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> Acked-by: Song Liu <songliubraving@fb.com>
> ---
> tools/lib/bpf/libbpf.c | 79 ++++++++++++++++++++++++++++++++++++++++
> tools/lib/bpf/libbpf.h | 4 ++
> tools/lib/bpf/libbpf.map | 1 +
> 3 files changed, 84 insertions(+)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 2f79e9563db9..8ad4f915df38 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -4184,6 +4184,85 @@ struct bpf_link *bpf_program__attach_uprobe(struct bpf_program *prog,
> return link;
> }
>
> +static int determine_tracepoint_id(const char *tp_category,
> + const char *tp_name)
> +{
> + char file[PATH_MAX];
> + int ret;
> +
> + ret = snprintf(file, sizeof(file),
> + "/sys/kernel/debug/tracing/events/%s/%s/id",
> + tp_category, tp_name);
> + if (ret < 0)
> + return -errno;
> + if (ret >= sizeof(file)) {
> + pr_debug("tracepoint %s/%s path is too long\n",
> + tp_category, tp_name);
> + return -E2BIG;
> + }
> + return parse_value_from_file(file, "%d\n");
> +}
> +
> +static int perf_event_open_tracepoint(const char *tp_category,
> + const char *tp_name)
> +{
> + struct perf_event_attr attr = {};
> + char errmsg[STRERR_BUFSIZE];
> + int tp_id, pfd, err;
> +
> + tp_id = determine_tracepoint_id(tp_category, tp_name);
> + if (tp_id < 0) {
> + pr_warning("failed to determine tracepoint '%s/%s' perf ID: %s\n",
nit: "perf ID" is not accurate. Maybe "event ID" or "perf event ID"?
> + tp_category, tp_name,
> + libbpf_strerror_r(tp_id, errmsg, sizeof(errmsg)));
> + return tp_id;
> + }
> +
> + attr.type = PERF_TYPE_TRACEPOINT;
> + attr.size = sizeof(attr);
> + attr.config = tp_id;
> +
> + pfd = syscall(__NR_perf_event_open, &attr, -1 /* pid */, 0 /* cpu */,
> + -1 /* group_fd */, PERF_FLAG_FD_CLOEXEC);
> + if (pfd < 0) {
> + err = -errno;
> + pr_warning("tracepoint '%s/%s' perf_event_open() failed: %s\n",
> + tp_category, tp_name,
> + libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
> + return err;
> + }
> + return pfd;
> +}
> +
> +struct bpf_link *bpf_program__attach_tracepoint(struct bpf_program *prog,
> + const char *tp_category,
> + const char *tp_name)
> +{
> + char errmsg[STRERR_BUFSIZE];
> + struct bpf_link *link;
> + int pfd, err;
> +
> + pfd = perf_event_open_tracepoint(tp_category, tp_name);
> + if (pfd < 0) {
> + pr_warning("program '%s': failed to create tracepoint '%s/%s' perf event: %s\n",
> + bpf_program__title(prog, false),
> + tp_category, tp_name,
> + libbpf_strerror_r(pfd, errmsg, sizeof(errmsg)));
> + return ERR_PTR(pfd);
> + }
> + link = bpf_program__attach_perf_event(prog, pfd);
> + if (IS_ERR(link)) {
> + close(pfd);
> + err = PTR_ERR(link);
> + pr_warning("program '%s': failed to attach to tracepoint '%s/%s': %s\n",
> + bpf_program__title(prog, false),
> + tp_category, tp_name,
> + libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
> + return link;
> + }
> + return link;
> +}
> +
> enum bpf_perf_event_ret
> bpf_perf_event_read_simple(void *mmap_mem, size_t mmap_size, size_t page_size,
> void **copy_mem, size_t *copy_size,
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index bd767cc11967..60611f4b4e1d 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -178,6 +178,10 @@ LIBBPF_API struct bpf_link *
> bpf_program__attach_uprobe(struct bpf_program *prog, bool retprobe,
> pid_t pid, const char *binary_path,
> size_t func_offset);
> +LIBBPF_API struct bpf_link *
> +bpf_program__attach_tracepoint(struct bpf_program *prog,
> + const char *tp_category,
> + const char *tp_name);
>
> struct bpf_insn;
>
> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> index 57a40fb60718..3c618b75ef65 100644
> --- a/tools/lib/bpf/libbpf.map
> +++ b/tools/lib/bpf/libbpf.map
> @@ -171,6 +171,7 @@ LIBBPF_0.0.4 {
> bpf_object__load_xattr;
> bpf_program__attach_kprobe;
> bpf_program__attach_perf_event;
> + bpf_program__attach_tracepoint;
> bpf_program__attach_uprobe;
> btf_dump__dump_type;
> btf_dump__free;
>
^ permalink raw reply
* [PATCH net-next 5/5] ipv4: use indirect call wrappers for {tcp,udp}_{recv,send}msg()
From: Paolo Abeni @ 2019-07-01 17:09 UTC (permalink / raw)
To: netdev; +Cc: David S. Miller
In-Reply-To: <cover.1561999976.git.pabeni@redhat.com>
This avoids an indirect call per syscall for common ipv4 transports
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
net/ipv4/af_inet.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 8421e2f5bbb3..9a2f17d0c5f5 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -797,6 +797,8 @@ int inet_send_prepare(struct sock *sk)
}
EXPORT_SYMBOL_GPL(inet_send_prepare);
+INDIRECT_CALLABLE_DECLARE(int udp_sendmsg(struct sock *, struct msghdr *,
+ size_t));
int inet_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
{
struct sock *sk = sock->sk;
@@ -804,7 +806,8 @@ int inet_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
if (unlikely(inet_send_prepare(sk)))
return -EAGAIN;
- return sk->sk_prot->sendmsg(sk, msg, size);
+ return INDIRECT_CALL_2(sk->sk_prot->sendmsg, tcp_sendmsg, udp_sendmsg,
+ sk, msg, size);
}
EXPORT_SYMBOL(inet_sendmsg);
@@ -822,6 +825,8 @@ ssize_t inet_sendpage(struct socket *sock, struct page *page, int offset,
}
EXPORT_SYMBOL(inet_sendpage);
+INDIRECT_CALLABLE_DECLARE(int udp_recvmsg(struct sock *, struct msghdr *,
+ size_t, int, int, int *));
int inet_recvmsg(struct socket *sock, struct msghdr *msg, size_t size,
int flags)
{
@@ -832,8 +837,9 @@ int inet_recvmsg(struct socket *sock, struct msghdr *msg, size_t size,
if (likely(!(flags & MSG_ERRQUEUE)))
sock_rps_record_flow(sk);
- err = sk->sk_prot->recvmsg(sk, msg, size, flags & MSG_DONTWAIT,
- flags & ~MSG_DONTWAIT, &addr_len);
+ err = INDIRECT_CALL_2(sk->sk_prot->recvmsg, tcp_recvmsg, udp_recvmsg,
+ sk, msg, size, flags & MSG_DONTWAIT,
+ flags & ~MSG_DONTWAIT, &addr_len);
if (err >= 0)
msg->msg_namelen = addr_len;
return err;
--
2.20.1
^ permalink raw reply related
* [PATCH net-next 4/5] ipv6: use indirect call wrappers for {tcp,udpv6}_{recv,send}msg()
From: Paolo Abeni @ 2019-07-01 17:09 UTC (permalink / raw)
To: netdev; +Cc: David S. Miller
In-Reply-To: <cover.1561999976.git.pabeni@redhat.com>
This avoids an indirect call per syscall for common ipv6 transports
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
net/ipv6/af_inet6.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/net/ipv6/af_inet6.c b/net/ipv6/af_inet6.c
index 4628681eca88..d5e98ee9fc79 100644
--- a/net/ipv6/af_inet6.c
+++ b/net/ipv6/af_inet6.c
@@ -564,6 +564,8 @@ int inet6_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
}
EXPORT_SYMBOL(inet6_ioctl);
+INDIRECT_CALLABLE_DECLARE(int udpv6_sendmsg(struct sock *, struct msghdr *,
+ size_t));
int inet6_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
{
struct sock *sk = sock->sk;
@@ -571,9 +573,12 @@ int inet6_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
if (unlikely(inet_send_prepare(sk)))
return -EAGAIN;
- return sk->sk_prot->sendmsg(sk, msg, size);
+ return INDIRECT_CALL_2(sk->sk_prot->sendmsg, tcp_sendmsg, udpv6_sendmsg,
+ sk, msg, size);
}
+INDIRECT_CALLABLE_DECLARE(int udpv6_recvmsg(struct sock *, struct msghdr *,
+ size_t, int, int, int *));
int inet6_recvmsg(struct socket *sock, struct msghdr *msg, size_t size,
int flags)
{
@@ -584,8 +589,9 @@ int inet6_recvmsg(struct socket *sock, struct msghdr *msg, size_t size,
if (likely(!(flags & MSG_ERRQUEUE)))
sock_rps_record_flow(sk);
- err = sk->sk_prot->recvmsg(sk, msg, size, flags & MSG_DONTWAIT,
- flags & ~MSG_DONTWAIT, &addr_len);
+ err = INDIRECT_CALL_2(sk->sk_prot->recvmsg, tcp_recvmsg, udpv6_recvmsg,
+ sk, msg, size, flags & MSG_DONTWAIT,
+ flags & ~MSG_DONTWAIT, &addr_len);
if (err >= 0)
msg->msg_namelen = addr_len;
return err;
--
2.20.1
^ permalink raw reply related
* [PATCH net-next 1/5] inet: factor out inet_send_prepare()
From: Paolo Abeni @ 2019-07-01 17:09 UTC (permalink / raw)
To: netdev; +Cc: David S. Miller
In-Reply-To: <cover.1561999976.git.pabeni@redhat.com>
The same code is replicated verbatim in multiple places, and the next
patches will introduce an additional user for it. Factor out a
helper and use it where appropriate. No functional change intended.
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
include/net/inet_common.h | 1 +
net/ipv4/af_inet.c | 21 +++++++++++++--------
2 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/include/net/inet_common.h b/include/net/inet_common.h
index 975901a95c0f..ae2ba897675c 100644
--- a/include/net/inet_common.h
+++ b/include/net/inet_common.h
@@ -25,6 +25,7 @@ int inet_dgram_connect(struct socket *sock, struct sockaddr *uaddr,
int addr_len, int flags);
int inet_accept(struct socket *sock, struct socket *newsock, int flags,
bool kern);
+int inet_send_prepare(struct sock *sk);
int inet_sendmsg(struct socket *sock, struct msghdr *msg, size_t size);
ssize_t inet_sendpage(struct socket *sock, struct page *page, int offset,
size_t size, int flags);
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 52bdb881a506..8421e2f5bbb3 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -784,10 +784,8 @@ int inet_getname(struct socket *sock, struct sockaddr *uaddr,
}
EXPORT_SYMBOL(inet_getname);
-int inet_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
+int inet_send_prepare(struct sock *sk)
{
- struct sock *sk = sock->sk;
-
sock_rps_record_flow(sk);
/* We may need to bind the socket. */
@@ -795,6 +793,17 @@ int inet_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
inet_autobind(sk))
return -EAGAIN;
+ return 0;
+}
+EXPORT_SYMBOL_GPL(inet_send_prepare);
+
+int inet_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
+{
+ struct sock *sk = sock->sk;
+
+ if (unlikely(inet_send_prepare(sk)))
+ return -EAGAIN;
+
return sk->sk_prot->sendmsg(sk, msg, size);
}
EXPORT_SYMBOL(inet_sendmsg);
@@ -804,11 +813,7 @@ ssize_t inet_sendpage(struct socket *sock, struct page *page, int offset,
{
struct sock *sk = sock->sk;
- sock_rps_record_flow(sk);
-
- /* We may need to bind the socket. */
- if (!inet_sk(sk)->inet_num && !sk->sk_prot->no_autobind &&
- inet_autobind(sk))
+ if (unlikely(inet_send_prepare(sk)))
return -EAGAIN;
if (sk->sk_prot->sendpage)
--
2.20.1
^ permalink raw reply related
* [PATCH net-next 3/5] net: adjust socket level ICW to cope with ipv6 variant of {recv,send}msg
From: Paolo Abeni @ 2019-07-01 17:09 UTC (permalink / raw)
To: netdev; +Cc: David S. Miller
In-Reply-To: <cover.1561999976.git.pabeni@redhat.com>
After the previous patch we have ipv{6,4} variants for {recv,send}msg,
we should use the generic _INET ICW variant to call into the proper
build-in.
This also allows dropping the now unused and rather ugly _INET4 ICW macro
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
net/socket.c | 17 ++++++-----------
1 file changed, 6 insertions(+), 11 deletions(-)
diff --git a/net/socket.c b/net/socket.c
index 963df5dbdd54..f5e0e460012b 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -103,13 +103,6 @@
#include <net/busy_poll.h>
#include <linux/errqueue.h>
-/* proto_ops for ipv4 and ipv6 use the same {recv,send}msg function */
-#if IS_ENABLED(CONFIG_INET)
-#define INDIRECT_CALL_INET4(f, f1, ...) INDIRECT_CALL_1(f, f1, __VA_ARGS__)
-#else
-#define INDIRECT_CALL_INET4(f, f1, ...) f(__VA_ARGS__)
-#endif
-
#ifdef CONFIG_NET_RX_BUSY_POLL
unsigned int sysctl_net_busy_read __read_mostly;
unsigned int sysctl_net_busy_poll __read_mostly;
@@ -643,8 +636,9 @@ INDIRECT_CALLABLE_DECLARE(int inet_sendmsg(struct socket *, struct msghdr *,
size_t));
static inline int sock_sendmsg_nosec(struct socket *sock, struct msghdr *msg)
{
- int ret = INDIRECT_CALL_INET4(sock->ops->sendmsg, inet_sendmsg, sock,
- msg, msg_data_left(msg));
+ int ret = INDIRECT_CALL_INET(sock->ops->sendmsg, inet6_sendmsg,
+ inet_sendmsg, sock, msg,
+ msg_data_left(msg));
BUG_ON(ret == -EIOCBQUEUED);
return ret;
}
@@ -874,8 +868,9 @@ INDIRECT_CALLABLE_DECLARE(int inet_recvmsg(struct socket *, struct msghdr *,
static inline int sock_recvmsg_nosec(struct socket *sock, struct msghdr *msg,
int flags)
{
- return INDIRECT_CALL_INET4(sock->ops->recvmsg, inet_recvmsg, sock, msg,
- msg_data_left(msg), flags);
+ return INDIRECT_CALL_INET(sock->ops->recvmsg, inet6_recvmsg,
+ inet_recvmsg, sock, msg, msg_data_left(msg),
+ flags);
}
/**
--
2.20.1
^ permalink raw reply related
* [PATCH net-next 2/5] ipv6: provide and use ipv6 specific version for {recv,send}msg
From: Paolo Abeni @ 2019-07-01 17:09 UTC (permalink / raw)
To: netdev; +Cc: David S. Miller
In-Reply-To: <cover.1561999976.git.pabeni@redhat.com>
This will simplify indirect call wrapper invocation in the following
patch.
No functional change intended, any - out-of-tree - IPv6 user of
inet_{recv,send}msg can keep using the existing functions.
SCTP code still uses the existing version even for ipv6: as this series
will not add ICW for SCTP, moving to the new helper would not give
any benefit.
The only other in-kernel user of inet_{recv,send}msg is
pvcalls_conn_back_read(), but psvcalls explicitly creates only IPv4 socket,
so no need to update that code path, too.
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
include/net/ipv6.h | 3 +++
net/ipv6/af_inet6.c | 35 +++++++++++++++++++++++++++++++----
2 files changed, 34 insertions(+), 4 deletions(-)
diff --git a/include/net/ipv6.h b/include/net/ipv6.h
index b41f6a0fa903..aecc28dff8f8 100644
--- a/include/net/ipv6.h
+++ b/include/net/ipv6.h
@@ -1089,6 +1089,9 @@ int inet6_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg);
int inet6_hash_connect(struct inet_timewait_death_row *death_row,
struct sock *sk);
+int inet6_sendmsg(struct socket *sock, struct msghdr *msg, size_t size);
+int inet6_recvmsg(struct socket *sock, struct msghdr *msg, size_t size,
+ int flags);
/*
* reassembly.c
diff --git a/net/ipv6/af_inet6.c b/net/ipv6/af_inet6.c
index 7382a927d1eb..4628681eca88 100644
--- a/net/ipv6/af_inet6.c
+++ b/net/ipv6/af_inet6.c
@@ -564,6 +564,33 @@ int inet6_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
}
EXPORT_SYMBOL(inet6_ioctl);
+int inet6_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
+{
+ struct sock *sk = sock->sk;
+
+ if (unlikely(inet_send_prepare(sk)))
+ return -EAGAIN;
+
+ return sk->sk_prot->sendmsg(sk, msg, size);
+}
+
+int inet6_recvmsg(struct socket *sock, struct msghdr *msg, size_t size,
+ int flags)
+{
+ struct sock *sk = sock->sk;
+ int addr_len = 0;
+ int err;
+
+ if (likely(!(flags & MSG_ERRQUEUE)))
+ sock_rps_record_flow(sk);
+
+ err = sk->sk_prot->recvmsg(sk, msg, size, flags & MSG_DONTWAIT,
+ flags & ~MSG_DONTWAIT, &addr_len);
+ if (err >= 0)
+ msg->msg_namelen = addr_len;
+ return err;
+}
+
const struct proto_ops inet6_stream_ops = {
.family = PF_INET6,
.owner = THIS_MODULE,
@@ -580,8 +607,8 @@ const struct proto_ops inet6_stream_ops = {
.shutdown = inet_shutdown, /* ok */
.setsockopt = sock_common_setsockopt, /* ok */
.getsockopt = sock_common_getsockopt, /* ok */
- .sendmsg = inet_sendmsg, /* ok */
- .recvmsg = inet_recvmsg, /* ok */
+ .sendmsg = inet6_sendmsg, /* retpoline's sake */
+ .recvmsg = inet6_recvmsg, /* retpoline's sake */
#ifdef CONFIG_MMU
.mmap = tcp_mmap,
#endif
@@ -614,8 +641,8 @@ const struct proto_ops inet6_dgram_ops = {
.shutdown = inet_shutdown, /* ok */
.setsockopt = sock_common_setsockopt, /* ok */
.getsockopt = sock_common_getsockopt, /* ok */
- .sendmsg = inet_sendmsg, /* ok */
- .recvmsg = inet_recvmsg, /* ok */
+ .sendmsg = inet6_sendmsg, /* retpoline's sake */
+ .recvmsg = inet6_recvmsg, /* retpoline's sake */
.mmap = sock_no_mmap,
.sendpage = sock_no_sendpage,
.set_peek_off = sk_set_peek_off,
--
2.20.1
^ permalink raw reply related
* [PATCH net-next 0/5] net: use ICW for sk_proto->{send,recv}msg
From: Paolo Abeni @ 2019-07-01 17:09 UTC (permalink / raw)
To: netdev; +Cc: David S. Miller
This series extends ICW usage to one of the few remaining spots in fast-path
still hitting per packet retpoline overhead, namely the sk_proto->{send,recv}msg
calls.
The first 3 patches in this series refactor the existing code so that applying
the ICW macros is straight-forward: we demux inet_{recv,send}msg in ipv4 and
ipv6 variants so that each of them can easily select the appropriate TCP or UDP
direct call. While at it, a new helper is created to avoid excessive code
duplication, and the current ICWs for inet_{recv,send}msg are adjusted
accordingly.
The last 2 patches really introduce the new ICW use-case, respectively for the
ipv6 and the ipv4 code path.
This gives up to 5% performance improvement under UDP flood, and smaller but
measurable gains for TCP RR workloads.
Paolo Abeni (5):
inet: factor out inet_send_prepare()
ipv6: provide and use ipv6 specific version for {recv,send}msg
net: adjust socket level ICW to cope with ipv6 variant of
{recv,send}msg
ipv6: use indirect call wrappers for {tcp,udpv6}_{recv,send}msg()
ipv4: use indirect call wrappers for {tcp,udp}_{recv,send}msg()
include/net/inet_common.h | 1 +
include/net/ipv6.h | 3 +++
net/ipv4/af_inet.c | 33 ++++++++++++++++++++-----------
net/ipv6/af_inet6.c | 41 +++++++++++++++++++++++++++++++++++----
net/socket.c | 17 ++++++----------
5 files changed, 69 insertions(+), 26 deletions(-)
--
2.20.1
^ permalink raw reply
* Re: [PATCH v4 bpf-next 4/9] libbpf: add kprobe/uprobe attach API
From: Yonghong Song @ 2019-07-01 17:09 UTC (permalink / raw)
To: Andrii Nakryiko, andrii.nakryiko@gmail.com, bpf@vger.kernel.org,
netdev@vger.kernel.org, Alexei Starovoitov, daniel@iogearbox.net,
Kernel Team, sdf@fomichev.me, Song Liu
In-Reply-To: <20190629034906.1209916-5-andriin@fb.com>
On 6/28/19 8:49 PM, Andrii Nakryiko wrote:
> Add ability to attach to kernel and user probes and retprobes.
> Implementation depends on perf event support for kprobes/uprobes.
>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> ---
> tools/lib/bpf/libbpf.c | 165 +++++++++++++++++++++++++++++++++++++++
> tools/lib/bpf/libbpf.h | 7 ++
> tools/lib/bpf/libbpf.map | 2 +
> 3 files changed, 174 insertions(+)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 98c155ec3bfa..2f79e9563db9 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -4019,6 +4019,171 @@ struct bpf_link *bpf_program__attach_perf_event(struct bpf_program *prog,
> return (struct bpf_link *)link;
> }
>
> +static int parse_value_from_file(const char *file, const char *fmt)
Here, the value from the file must be positive int values to avoid
confusion between valid value vs. error code.
Could you add a comment to state this fact for the current
uprobe/kprobe/tracepoint support?
> +{
> + char buf[STRERR_BUFSIZE];
> + int err, ret;
> + FILE *f;
> +
> + f = fopen(file, "r");
> + if (!f) {
> + err = -errno;
> + pr_debug("failed to open '%s': %s\n", file,
> + libbpf_strerror_r(err, buf, sizeof(buf)));
> + fclose(f);
fclose(f) is not needed. fopen has failed.
> + return err;
> + }
> + err = fscanf(f, fmt, &ret);
> + if (err != 1) {
> + err = err == EOF ? -EIO : -errno;
> + pr_debug("failed to parse '%s': %s\n", file,
> + libbpf_strerror_r(err, buf, sizeof(buf)));
> + fclose(f);
> + return err;
> + }
> + fclose(f);
> + return ret;
> +}
> +
> +static int determine_kprobe_perf_type(void)
> +{
> + const char *file = "/sys/bus/event_source/devices/kprobe/type";
> +
> + return parse_value_from_file(file, "%d\n");
> +}
> +
> +static int determine_uprobe_perf_type(void)
> +{
> + const char *file = "/sys/bus/event_source/devices/uprobe/type";
> +
> + return parse_value_from_file(file, "%d\n");
> +}
> +
> +static int determine_kprobe_retprobe_bit(void)
> +{
> + const char *file = "/sys/bus/event_source/devices/kprobe/format/retprobe";
> +
> + return parse_value_from_file(file, "config:%d\n");
> +}
> +
> +static int determine_uprobe_retprobe_bit(void)
> +{
> + const char *file = "/sys/bus/event_source/devices/uprobe/format/retprobe";
> +
> + return parse_value_from_file(file, "config:%d\n");
> +}
> +
> +static int perf_event_open_probe(bool uprobe, bool retprobe, const char *name,
> + uint64_t offset, int pid)
> +{
> + struct perf_event_attr attr = {};
> + char errmsg[STRERR_BUFSIZE];
> + int type, pfd, err;
> +
> + type = uprobe ? determine_uprobe_perf_type()
> + : determine_kprobe_perf_type();
> + if (type < 0) {
> + pr_warning("failed to determine %s perf type: %s\n",
> + uprobe ? "uprobe" : "kprobe",
> + libbpf_strerror_r(type, errmsg, sizeof(errmsg)));
> + return type;
> + }
> + if (retprobe) {
> + int bit = uprobe ? determine_uprobe_retprobe_bit()
> + : determine_kprobe_retprobe_bit();
> +
> + if (bit < 0) {
> + pr_warning("failed to determine %s retprobe bit: %s\n",
> + uprobe ? "uprobe" : "kprobe",
> + libbpf_strerror_r(bit, errmsg,
> + sizeof(errmsg)));
> + return bit;
> + }
> + attr.config |= 1 << bit;
> + }
> + attr.size = sizeof(attr);
> + attr.type = type;
> + attr.config1 = (uint64_t)(void *)name; /* kprobe_func or uprobe_path */
> + attr.config2 = offset; /* kprobe_addr or probe_offset */
> +
> + /* pid filter is meaningful only for uprobes */
> + pfd = syscall(__NR_perf_event_open, &attr,
> + pid < 0 ? -1 : pid /* pid */,
> + pid == -1 ? 0 : -1 /* cpu */,
> + -1 /* group_fd */, PERF_FLAG_FD_CLOEXEC);
> + if (pfd < 0) {
> + err = -errno;
> + pr_warning("%s perf_event_open() failed: %s\n",
> + uprobe ? "uprobe" : "kprobe",
> + libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
> + return err;
> + }
> + return pfd;
> +}
> +
> +struct bpf_link *bpf_program__attach_kprobe(struct bpf_program *prog,
> + bool retprobe,
> + const char *func_name)
> +{
> + char errmsg[STRERR_BUFSIZE];
> + struct bpf_link *link;
> + int pfd, err;
> +
> + pfd = perf_event_open_probe(false /* uprobe */, retprobe, func_name,
> + 0 /* offset */, -1 /* pid */);
> + if (pfd < 0) {
> + pr_warning("program '%s': failed to create %s '%s' perf event: %s\n",
> + bpf_program__title(prog, false),
> + retprobe ? "kretprobe" : "kprobe", func_name,
> + libbpf_strerror_r(pfd, errmsg, sizeof(errmsg)));
> + return ERR_PTR(pfd);
> + }
> + link = bpf_program__attach_perf_event(prog, pfd);
> + if (IS_ERR(link)) {
> + close(pfd);
> + err = PTR_ERR(link);
> + pr_warning("program '%s': failed to attach to %s '%s': %s\n",
> + bpf_program__title(prog, false),
> + retprobe ? "kretprobe" : "kprobe", func_name,
> + libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
> + return link;
> + }
> + return link;
> +}
> +
> +struct bpf_link *bpf_program__attach_uprobe(struct bpf_program *prog,
> + bool retprobe, pid_t pid,
> + const char *binary_path,
> + size_t func_offset)
> +{
> + char errmsg[STRERR_BUFSIZE];
> + struct bpf_link *link;
> + int pfd, err;
> +
> + pfd = perf_event_open_probe(true /* uprobe */, retprobe,
> + binary_path, func_offset, pid);
> + if (pfd < 0) {
> + pr_warning("program '%s': failed to create %s '%s:0x%zx' perf event: %s\n",
> + bpf_program__title(prog, false),
> + retprobe ? "uretprobe" : "uprobe",
> + binary_path, func_offset,
> + libbpf_strerror_r(pfd, errmsg, sizeof(errmsg)));
> + return ERR_PTR(pfd);
> + }
> + link = bpf_program__attach_perf_event(prog, pfd);
> + if (IS_ERR(link)) {
> + close(pfd);
> + err = PTR_ERR(link);
> + pr_warning("program '%s': failed to attach to %s '%s:0x%zx': %s\n",
> + bpf_program__title(prog, false),
> + retprobe ? "uretprobe" : "uprobe",
> + binary_path, func_offset,
> + libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
> + return link;
> + }
> + return link;
> +}
> +
> enum bpf_perf_event_ret
> bpf_perf_event_read_simple(void *mmap_mem, size_t mmap_size, size_t page_size,
> void **copy_mem, size_t *copy_size,
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index 1bf66c4a9330..bd767cc11967 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -171,6 +171,13 @@ LIBBPF_API int bpf_link__destroy(struct bpf_link *link);
>
> LIBBPF_API struct bpf_link *
> bpf_program__attach_perf_event(struct bpf_program *prog, int pfd);
> +LIBBPF_API struct bpf_link *
> +bpf_program__attach_kprobe(struct bpf_program *prog, bool retprobe,
> + const char *func_name);
> +LIBBPF_API struct bpf_link *
> +bpf_program__attach_uprobe(struct bpf_program *prog, bool retprobe,
> + pid_t pid, const char *binary_path,
> + size_t func_offset);
>
> struct bpf_insn;
>
> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> index 756f5aa802e9..57a40fb60718 100644
> --- a/tools/lib/bpf/libbpf.map
> +++ b/tools/lib/bpf/libbpf.map
> @@ -169,7 +169,9 @@ LIBBPF_0.0.4 {
> global:
> bpf_link__destroy;
> bpf_object__load_xattr;
> + bpf_program__attach_kprobe;
> bpf_program__attach_perf_event;
> + bpf_program__attach_uprobe;
> btf_dump__dump_type;
> btf_dump__free;
> btf_dump__new;
>
^ permalink raw reply
* Re: [PATCH net-next 0/7] net/rds: RDMA fixes
From: santosh.shilimkar @ 2019-07-01 17:06 UTC (permalink / raw)
To: Gerd Rausch, netdev; +Cc: David Miller
In-Reply-To: <f1f5ca90-a98a-4c7a-c918-ef26f02f6ee7@oracle.com>
On 7/1/19 9:39 AM, Gerd Rausch wrote:
> A number of net/rds fixes necessary to make "rds_rdma.ko"
> pass some basic Oracle internal tests.
>
> Gerd Rausch (7):
> net/rds: Give fr_state a chance to transition to FRMR_IS_FREE
> net/rds: Get rid of "wait_clean_list_grace" and add locking
> net/rds: Wait for the FRMR_IS_FREE (or FRMR_IS_STALE) transition after
> posting IB_WR_LOCAL_INV
> net/rds: Fix NULL/ERR_PTR inconsistency
> net/rds: Set fr_state only to FRMR_IS_FREE if IB_WR_LOCAL_INV had been
> successful
> net/rds: Keep track of and wait for FRWR segments in use upon shutdown
> net/rds: Initialize ic->i_fastreg_wrs upon allocation
>
Will apply these on top of earlier few fixes after going through them.
Thanks for posting them out.
Regards,
Santosh
^ permalink raw reply
* Re: [PATCH v4 bpf-next 3/9] libbpf: add ability to attach/detach BPF program to perf event
From: Yonghong Song @ 2019-07-01 17:03 UTC (permalink / raw)
To: Andrii Nakryiko, andrii.nakryiko@gmail.com, bpf@vger.kernel.org,
netdev@vger.kernel.org, Alexei Starovoitov, daniel@iogearbox.net,
Kernel Team, sdf@fomichev.me, Song Liu
In-Reply-To: <20190629034906.1209916-4-andriin@fb.com>
On 6/28/19 8:49 PM, Andrii Nakryiko wrote:
> bpf_program__attach_perf_event allows to attach BPF program to existing
> perf event hook, providing most generic and most low-level way to attach BPF
> programs. It returns struct bpf_link, which should be passed to
> bpf_link__destroy to detach and free resources, associated with a link.
>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> ---
> tools/lib/bpf/libbpf.c | 61 ++++++++++++++++++++++++++++++++++++++++
> tools/lib/bpf/libbpf.h | 3 ++
> tools/lib/bpf/libbpf.map | 1 +
> 3 files changed, 65 insertions(+)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 455795e6f8af..98c155ec3bfa 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -32,6 +32,7 @@
> #include <linux/limits.h>
> #include <linux/perf_event.h>
> #include <linux/ring_buffer.h>
> +#include <sys/ioctl.h>
> #include <sys/stat.h>
> #include <sys/types.h>
> #include <sys/vfs.h>
> @@ -3958,6 +3959,66 @@ int bpf_link__destroy(struct bpf_link *link)
> return err;
> }
>
> +struct bpf_link_fd {
> + struct bpf_link link; /* has to be at the top of struct */
> + int fd; /* hook FD */
> +};
> +
> +static int bpf_link__destroy_perf_event(struct bpf_link *link)
> +{
> + struct bpf_link_fd *l = (void *)link;
> + int err;
> +
> + if (l->fd < 0)
> + return 0;
> +
> + err = ioctl(l->fd, PERF_EVENT_IOC_DISABLE, 0);
> + if (err)
> + err = -errno;
> +
> + close(l->fd);
> + return err;
> +}
> +
> +struct bpf_link *bpf_program__attach_perf_event(struct bpf_program *prog,
> + int pfd)
> +{
> + char errmsg[STRERR_BUFSIZE];
> + struct bpf_link_fd *link;
> + int prog_fd, err;
> +
> + prog_fd = bpf_program__fd(prog);
> + if (prog_fd < 0) {
> + pr_warning("program '%s': can't attach before loaded\n",
> + bpf_program__title(prog, false));
> + return ERR_PTR(-EINVAL);
> + }
should we check validity of pfd here?
If pfd < 0, we just return ERR_PTR(-EINVAL)?
This way, in bpf_link__destroy_perf_event(), we do not need to check
l->fd < 0 since it will be always nonnegative.
> +
> + link = malloc(sizeof(*link));
> + if (!link)
> + return ERR_PTR(-ENOMEM);
> + link->link.destroy = &bpf_link__destroy_perf_event;
> + link->fd = pfd;
> +
> + if (ioctl(pfd, PERF_EVENT_IOC_SET_BPF, prog_fd) < 0) {
> + err = -errno;
> + free(link);
> + pr_warning("program '%s': failed to attach to pfd %d: %s\n",
> + bpf_program__title(prog, false), pfd,
> + libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
> + return ERR_PTR(err);
> + }
> + if (ioctl(pfd, PERF_EVENT_IOC_ENABLE, 0) < 0) {
> + err = -errno;
> + free(link);
> + pr_warning("program '%s': failed to enable pfd %d: %s\n",
> + bpf_program__title(prog, false), pfd,
> + libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
> + return ERR_PTR(err);
> + }
> + return (struct bpf_link *)link;
> +}
> +
> enum bpf_perf_event_ret
> bpf_perf_event_read_simple(void *mmap_mem, size_t mmap_size, size_t page_size,
> void **copy_mem, size_t *copy_size,
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index 5082a5ebb0c2..1bf66c4a9330 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -169,6 +169,9 @@ struct bpf_link;
>
> LIBBPF_API int bpf_link__destroy(struct bpf_link *link);
>
> +LIBBPF_API struct bpf_link *
> +bpf_program__attach_perf_event(struct bpf_program *prog, int pfd);
> +
> struct bpf_insn;
>
> /*
> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> index 3cde850fc8da..756f5aa802e9 100644
> --- a/tools/lib/bpf/libbpf.map
> +++ b/tools/lib/bpf/libbpf.map
> @@ -169,6 +169,7 @@ LIBBPF_0.0.4 {
> global:
> bpf_link__destroy;
> bpf_object__load_xattr;
> + bpf_program__attach_perf_event;
> btf_dump__dump_type;
> btf_dump__free;
> btf_dump__new;
>
^ permalink raw reply
* Re: [PATCH v2 0/3] vsock/virtio: several fixes in the .probe() and .remove()
From: Stefano Garzarella @ 2019-07-01 17:03 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: netdev, kvm, Michael S. Tsirkin, linux-kernel, virtualization,
Stefan Hajnoczi, David S. Miller
In-Reply-To: <20190701151113.GE11900@stefanha-x1.localdomain>
On Mon, Jul 01, 2019 at 04:11:13PM +0100, Stefan Hajnoczi wrote:
> On Fri, Jun 28, 2019 at 02:36:56PM +0200, Stefano Garzarella wrote:
> > During the review of "[PATCH] vsock/virtio: Initialize core virtio vsock
> > before registering the driver", Stefan pointed out some possible issues
> > in the .probe() and .remove() callbacks of the virtio-vsock driver.
> >
> > This series tries to solve these issues:
> > - Patch 1 adds RCU critical sections to avoid use-after-free of
> > 'the_virtio_vsock' pointer.
> > - Patch 2 stops workers before to call vdev->config->reset(vdev) to
> > be sure that no one is accessing the device.
> > - Patch 3 moves the works flush at the end of the .remove() to avoid
> > use-after-free of 'vsock' object.
> >
> > v2:
> > - Patch 1: use RCU to protect 'the_virtio_vsock' pointer
> > - Patch 2: no changes
> > - Patch 3: flush works only at the end of .remove()
> > - Removed patch 4 because virtqueue_detach_unused_buf() returns all the buffers
> > allocated.
> >
> > v1: https://patchwork.kernel.org/cover/10964733/
>
> This looks good to me.
Thanks for the review!
>
> Did you run any stress tests? For example an SMP guest constantly
> connecting and sending packets together with a script that
> hotplug/unplugs vhost-vsock-pci from the host side.
Yes, I started an SMP guest (-smp 4 -monitor tcp:127.0.0.1:1234,server,nowait)
and I run these scripts to stress the .probe()/.remove() path:
- guest
while true; do
cat /dev/urandom | nc-vsock -l 4321 > /dev/null &
cat /dev/urandom | nc-vsock -l 5321 > /dev/null &
cat /dev/urandom | nc-vsock -l 6321 > /dev/null &
cat /dev/urandom | nc-vsock -l 7321 > /dev/null &
wait
done
- host
while true; do
cat /dev/urandom | nc-vsock 3 4321 > /dev/null &
cat /dev/urandom | nc-vsock 3 5321 > /dev/null &
cat /dev/urandom | nc-vsock 3 6321 > /dev/null &
cat /dev/urandom | nc-vsock 3 7321 > /dev/null &
sleep 2
echo "device_del v1" | nc 127.0.0.1 1234
sleep 1
echo "device_add vhost-vsock-pci,id=v1,guest-cid=3" | nc 127.0.0.1 1234
sleep 1
done
Do you think is enough or is better to have a test more accurate?
Thanks,
Stefano
^ permalink raw reply
* Re: Use-after-free in br_multicast_rcv
From: Nikolay Aleksandrov @ 2019-07-01 17:03 UTC (permalink / raw)
To: Martin Weinelt, bridge, Roopa Prabhu; +Cc: netdev
In-Reply-To: <21ab085f-0f7f-88bc-b661-af74dd9eeea2@linuxlounge.net>
Hi Martin,
On 01/07/2019 19:53, Martin Weinelt wrote:
> Hi Nik,
>
> more info below.
>
> On 6/29/19 3:11 PM, nikolay@cumulusnetworks.com wrote:
>> On 29 June 2019 14:54:44 EEST, Martin Weinelt <martin@linuxlounge.net> wrote:
>>> Hello,
>>>
>>> we've recently been experiencing memory leaks on our Linux-based
>>> routers,
>>> at least as far back as v4.19.16.
>>>
>>> After rebuilding with KASAN it found a use-after-free in
>>> br_multicast_rcv which I could reproduce on v5.2.0-rc6.
>>>
>>> Please find the KASAN report below, I'm anot sure what else to provide
>>> so
>>> feel free to ask.
>>>
>>> Best,
>>> Martin
>>>
>>>
>>
>> Hi Martin,
>> I'll look into this, are there any specific steps to reproduce it?
>>
>> Thanks,
>> Nik
>>>
> Each server is a KVM Guest and has 18 bridges with the same master/slave
> relationships:
>
> bridge -> batman-adv -> {l2 tunnel, virtio device}
>
> Linus Lüssing from the batman-adv asked me to apply this patch to help
> debugging.
>
> v5.2-rc6-170-g728254541ebc with this patch yielded the following KASAN
> report, not sure if the additional information at the end is a result of
> the added patch though.
>
> Best,
> Martin
>
I see a couple of issues that can cause out-of-bounds accesses in br_multicast.c
more specifically there're pskb_may_pull calls and accesses to stale skb pointers.
I've had these on my "to fix" list for some time now, will prepare, test the fixes and
send them for review. In a few minutes I'll send a test patch for you.
That being said, I thought you said you've been experiencing memory leaks, but below
reports are for out-of-bounds accesses, could you please clarify if you were
speaking about these or is there another issue as well ?
If you're experiencing memory leaks, are you sure they're related to the bridge ?
You could try kmemleak for those.
Thank you,
Nik
> From 47a04e977311a0c45f26905588f563b55239da7f Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Linus=20L=C3=BCssing?= <linus.luessing@c0d3.blue>
> Date: Sat, 29 Jun 2019 20:24:23 +0200
> Subject: [PATCH] bridge: DEBUG: ipv6_addr_is_ll_all_nodes() wrappers for impr.
> call traces
>
> ---
> net/bridge/br_multicast.c | 70 +++++++++++++++++++++++++++++++++++----
> 1 file changed, 63 insertions(+), 7 deletions(-)
>
> diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
> index de22c8fbbb15..224a43318955 100644
> --- a/net/bridge/br_multicast.c
> +++ b/net/bridge/br_multicast.c
> @@ -57,6 +57,42 @@ static void br_ip6_multicast_leave_group(struct net_bridge *br,
> struct net_bridge_port *port,
> const struct in6_addr *group,
> __u16 vid, const unsigned char *src);
> +
> +static noinline void br_ip6_multicast_leave_group_mld2report(
> + struct net_bridge *br,
> + struct net_bridge_port *port,
> + const struct in6_addr *group,
> + __u16 vid,
> + const unsigned char *src)
> +{
> + br_ip6_multicast_leave_group(br, port, group, vid, src);
> +}
> +
> +static noinline void br_ip6_multicast_leave_group_ipv6rcv(
> + struct net_bridge *br,
> + struct net_bridge_port *port,
> + const struct in6_addr *group,
> + __u16 vid,
> + const unsigned char *src)
> +{
> + br_ip6_multicast_leave_group(br, port, group, vid, src);
> +}
> +
> +
> +static noinline bool ipv6_addr_is_ll_all_nodes_addgroup(const struct in6_addr *addr)
> +{
> + return ipv6_addr_is_ll_all_nodes(addr);
> +}
> +
> +static noinline bool ipv6_addr_is_ll_all_nodes_leavegroup(const struct in6_addr *addr)
> +{
> + return ipv6_addr_is_ll_all_nodes(addr);
> +}
> +
> +static noinline bool ipv6_addr_is_ll_all_nodes_mcastrcv(const struct in6_addr *addr)
> +{
> + return ipv6_addr_is_ll_all_nodes(addr);
> +}
> #endif
>
> static struct net_bridge_mdb_entry *br_mdb_ip_get_rcu(struct net_bridge *br,
> @@ -595,7 +631,7 @@ static int br_ip6_multicast_add_group(struct net_bridge *br,
> {
> struct br_ip br_group;
>
> - if (ipv6_addr_is_ll_all_nodes(group))
> + if (ipv6_addr_is_ll_all_nodes_addgroup(group))
> return 0;
>
> memset(&br_group, 0, sizeof(br_group));
> @@ -605,6 +641,26 @@ static int br_ip6_multicast_add_group(struct net_bridge *br,
>
> return br_multicast_add_group(br, port, &br_group, src);
> }
> +
> +static noinline int br_ip6_multicast_add_group_mld2report(
> + struct net_bridge *br,
> + struct net_bridge_port *port,
> + const struct in6_addr *group,
> + __u16 vid,
> + const unsigned char *src)
> +{
> + return br_ip6_multicast_add_group(br, port, group, vid, src);
> +}
> +
> +static noinline int br_ip6_multicast_add_group_ipv6rcv(
> + struct net_bridge *br,
> + struct net_bridge_port *port,
> + const struct in6_addr *group,
> + __u16 vid,
> + const unsigned char *src)
> +{
> + return br_ip6_multicast_add_group(br, port, group, vid, src);
> +}
> #endif
>
> static void br_multicast_router_expired(struct timer_list *t)
> @@ -1022,10 +1078,10 @@ static int br_ip6_multicast_mld2_report(struct net_bridge *br,
> if ((grec->grec_type == MLD2_CHANGE_TO_INCLUDE ||
> grec->grec_type == MLD2_MODE_IS_INCLUDE) &&
> ntohs(*nsrcs) == 0) {
> - br_ip6_multicast_leave_group(br, port, &grec->grec_mca,
> + br_ip6_multicast_leave_group_mld2report(br, port, &grec->grec_mca,
> vid, src);
> } else {
> - err = br_ip6_multicast_add_group(br, port,
> + err = br_ip6_multicast_add_group_mld2report(br, port,
> &grec->grec_mca, vid,
> src);
> if (err)
> @@ -1494,7 +1550,7 @@ static void br_ip6_multicast_leave_group(struct net_bridge *br,
> struct br_ip br_group;
> struct bridge_mcast_own_query *own_query;
>
> - if (ipv6_addr_is_ll_all_nodes(group))
> + if (ipv6_addr_is_ll_all_nodes_leavegroup(group))
> return;
>
> own_query = port ? &port->ip6_own_query : &br->ip6_own_query;
> @@ -1658,7 +1714,7 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br,
> err = ipv6_mc_check_mld(skb);
>
> if (err == -ENOMSG) {
> - if (!ipv6_addr_is_ll_all_nodes(&ipv6_hdr(skb)->daddr))
> + if (!ipv6_addr_is_ll_all_nodes_mcastrcv(&ipv6_hdr(skb)->daddr))
> BR_INPUT_SKB_CB(skb)->mrouters_only = 1;
>
> if (ipv6_addr_is_all_snoopers(&ipv6_hdr(skb)->daddr)) {
> @@ -1683,7 +1739,7 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br,
> case ICMPV6_MGM_REPORT:
> src = eth_hdr(skb)->h_source;
> BR_INPUT_SKB_CB(skb)->mrouters_only = 1;
> - err = br_ip6_multicast_add_group(br, port, &mld->mld_mca, vid,
> + err = br_ip6_multicast_add_group_ipv6rcv(br, port, &mld->mld_mca, vid,
> src);
> break;
> case ICMPV6_MLD2_REPORT:
> @@ -1694,7 +1750,7 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br,
> break;
> case ICMPV6_MGM_REDUCTION:
> src = eth_hdr(skb)->h_source;
> - br_ip6_multicast_leave_group(br, port, &mld->mld_mca, vid, src);
> + br_ip6_multicast_leave_group_ipv6rcv(br, port, &mld->mld_mca, vid, src);
> break;
> }
>
>
^ 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