* Re: [PATCH 1/2] samples: pktgen: add some helper functions for port parsing
From: David Miller @ 2019-07-01 18:02 UTC (permalink / raw)
To: danieltimlee; +Cc: brouer, netdev
In-Reply-To: <20190629133358.8251-1-danieltimlee@gmail.com>
From: "Daniel T. Lee" <danieltimlee@gmail.com>
Date: Sat, 29 Jun 2019 22:33:57 +0900
> This commit adds port parsing and port validate helper function to parse
> single or range of port(s) from a given string. (e.g. 1234, 443-444)
>
> Helpers will be used in prior to set target port(s) in samples/pktgen.
>
> Signed-off-by: Daniel T. Lee <danieltimlee@gmail.com>
Applied.
^ permalink raw reply
* Re: 答复: [PATCH v4] net: netfilter: Fix rpfilter dropping vrf packets by mistake
From: Pablo Neira Ayuso @ 2019-07-01 18:01 UTC (permalink / raw)
To: linmiaohe
Cc: David Ahern, kadlec@blackhole.kfki.hu, fw@strlen.de,
davem@davemloft.net, kuznet@ms2.inr.ac.ru,
yoshfuji@linux-ipv6.org, netfilter-devel@vger.kernel.org,
coreteam@netfilter.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, Mingfangsen
In-Reply-To: <cef929f9a14f462f9f7d3fa475f84e76@huawei.com>
On Sat, Jun 29, 2019 at 02:13:59PM +0000, linmiaohe wrote:
> On 6/29/19 20:20 PM, David Ahern wrote:
> > On 6/28/19 8:13 PM, linmiaohe wrote:
> > > You're right. Fib rules code would set FLOWI_FLAG_SKIP_NH_OIF flag.
> > > But I set it here for distinguish with the flags & XT_RPFILTER_LOOSE
> > > branch. Without this, they do the same work and maybe should be
> > > combined. I don't want to do that as that makes code confusing.
> > > Is this code snipet below ok ? If so, I would delete this flag setting.
> > >
> > > } else if (netif_is_l3_master(dev) || netif_is_l3_slave(dev)) {
> > > fl6.flowi6_oif = dev->ifindex;
> > > } else if ((flags & XT_RPFILTER_LOOSE) == 0)
> > > fl6.flowi6_oif = dev->ifindex;
>
> > that looks fine to me, but it is up to Pablo.
>
> @David Ahern Many thanks for your valuable advice.
>
> @ Pablo Hi, could you please tell me if this code snipet is ok?
> If not, which code would you prefer? It's very nice of you to
> figure it out for me. Thanks a lot.
Probably this?
} else if (netif_is_l3_master(dev) || netif_is_l3_slave(dev) ||
(flags & XT_RPFILTER_LOOSE) == 0) {
fl6.flowi6_oif = dev->ifindex;
}
Thanks.
^ permalink raw reply
* Re: [PATCH v2 net-next 04/19] ionic: Add basic lif support
From: Shannon Nelson @ 2019-07-01 18:00 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: netdev
In-Reply-To: <20190629114526.4da68321@cakuba.netronome.com>
On 6/29/19 11:45 AM, Jakub Kicinski wrote:
> On Fri, 28 Jun 2019 14:39:19 -0700, Shannon Nelson wrote:
>> @@ -64,4 +66,49 @@ int ionic_debugfs_add_ident(struct ionic *ionic)
>> ionic, &identity_fops) ? 0 : -ENOTSUPP;
>> }
>>
>> +int ionic_debugfs_add_sizes(struct ionic *ionic)
>> +{
>> + debugfs_create_u32("nlifs", 0400, ionic->dentry,
>> + (u32 *)&ionic->ident.dev.nlifs);
>> + debugfs_create_u32("nintrs", 0400, ionic->dentry, &ionic->nintrs);
>> +
>> + debugfs_create_u32("ntxqs_per_lif", 0400, ionic->dentry,
>> + (u32 *)&ionic->ident.lif.eth.config.queue_count[IONIC_QTYPE_TXQ]);
>> + debugfs_create_u32("nrxqs_per_lif", 0400, ionic->dentry,
>> + (u32 *)&ionic->ident.lif.eth.config.queue_count[IONIC_QTYPE_RXQ]);
> Are these __le32s? Does the driver build cleanly with W=1 C=1?
Aside from a couple of "expression using sizeof(void)" messages that I
haven't been able to fully clean out, yes, the driver builds pretty
clean with sparse. In this particular case, I think the typecast stops
the complaint that you are expecting here. But yes, these would be
better off with le32_to_cpu().
sln
>
>> + return 0;
>> +}
^ permalink raw reply
* Re: [PATCH] net:gue.h:Fix shifting signed 32-bit value by 31 bits problem
From: David Miller @ 2019-07-01 18:00 UTC (permalink / raw)
To: bnvandana; +Cc: netdev, linux-kernel, skhan, gregkh, linux-kernel-mentees
In-Reply-To: <20190701141610.3681-1-bnvandana@gmail.com>
From: Vandana BN <bnvandana@gmail.com>
Date: Mon, 1 Jul 2019 19:46:10 +0530
> Fix GUE_PFLAG_REMCSUM to use "U" cast to avoid shifting signed
> 32-bit value by 31 bits problem.
>
> Signed-off-by: Vandana BN <bnvandana@gmail.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next] ipv6: icmp: allow flowlabel reflection in echo replies
From: David Miller @ 2019-07-01 17:59 UTC (permalink / raw)
To: edumazet; +Cc: netdev, eric.dumazet
In-Reply-To: <20190701133936.15238-1-edumazet@google.com>
From: Eric Dumazet <edumazet@google.com>
Date: Mon, 1 Jul 2019 06:39:36 -0700
> Extend flowlabel_reflect bitmask to allow conditional
> reflection of incoming flowlabels in echo replies.
>
> Note this has precedence against auto flowlabels.
>
> Add flowlabel_reflect enum to replace hard coded
> values.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Applied, thanks Eric.
^ permalink raw reply
* Re: net: check before dereferencing netdev_ops during busy poll
From: Greg Kroah-Hartman @ 2019-07-01 17:52 UTC (permalink / raw)
To: Matteo Croce
Cc: Josh Elsasser, Sasha Levin, stable, netdev, LKML, David Miller
In-Reply-To: <CAGnkfhzmGbeQe7L55nEv575XyubWqCLz=7NQPpH+TajDkkDiXg@mail.gmail.com>
On Sat, Jun 29, 2019 at 09:39:39PM +0200, Matteo Croce wrote:
> On Sat, Jun 29, 2019 at 9:45 AM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > On Fri, Jun 28, 2019 at 07:03:01PM -0700, Josh Elsasser wrote:
> > > On Jun 28, 2019, at 3:55 PM, Sasha Levin <sashal@kernel.org> wrote:
> > >
> > > > What's the upstream commit id?
> > >
> > > The commit wasn't needed upstream, as I only sent the original patch after
> > > 79e7fff47b7b ("net: remove support for per driver ndo_busy_poll()") had
> > > made the fix unnecessary in Linus' tree.
> > >
> > > May've gotten lost in the shuffle due to my poor Fixes tags. The patch in
> > > question applied only on top of the 4.9 stable release at the time, but the
> > > actual NPE had been around in some form since 3.11 / 0602129286705 ("net: add
> > > low latency socket poll").
> >
> > Ok, can people then resend this and be very explicit as to why this is
> > needed only in a stable kernel tree and get reviews from people agreeing
> > that this really is the correct fix?
> >
> > thanks,
> >
> > greg k-h
>
> Hi Greg,
>
> I think that David alredy reviewed the patch here:
>
> https://lore.kernel.org/netdev/20180313.105115.682846171057663636.davem@davemloft.net/
>
> Anyway, I tested the patch and it fixes the panic, at least on my
> iwlwifi card, so:
>
> Tested-by: Matteo Croce <mcroce@redhat.com>
Ok, but what can I do with this? I need a real patch, in mail form,
that I can apply. Not a web link to an email archive.
You have read the stable kernel rules, right? :)
greg k-h
^ permalink raw reply
* Re: net: check before dereferencing netdev_ops during busy poll
From: Greg Kroah-Hartman @ 2019-07-01 17:52 UTC (permalink / raw)
To: Matteo Croce; +Cc: stable, Sasha Levin, Josh Elsasser, netdev, LKML
In-Reply-To: <CAGnkfhxxw9keiNj_Qm=2GBYpY38HAq28cOROMRqXfbqq8wNbWQ@mail.gmail.com>
On Fri, Jun 28, 2019 at 06:34:58PM +0200, Matteo Croce wrote:
> Hi,
>
> Is there any reason for this panic fix not being applied in stable?
>
> https://lore.kernel.org/netdev/20180313053248.13654-1-jelsasser@appneta.com/T/
I can't apply patches from random urls :)
^ permalink raw reply
* Re: [PATCH bpf-next v3 3/3] selftests/bpf: add verifier tests for wide stores
From: Yonghong Song @ 2019-07-01 17:50 UTC (permalink / raw)
To: Stanislav Fomichev, netdev@vger.kernel.org, bpf@vger.kernel.org
Cc: davem@davemloft.net, ast@kernel.org, daniel@iogearbox.net,
Andrii Nakryiko
In-Reply-To: <20190701173841.32249-4-sdf@google.com>
On 7/1/19 10:38 AM, Stanislav Fomichev wrote:
> Make sure that wide stores are allowed at proper (aligned) addresses.
> Note that user_ip6 is naturally aligned on 8-byte boundary, so
> correct addresses are user_ip6[0] and user_ip6[2]. msg_src_ip6 is,
> however, aligned on a 4-byte bondary, so only msg_src_ip6[1]
> can be wide-stored.
>
> Cc: Andrii Nakryiko <andriin@fb.com>
> Cc: Yonghong Song <yhs@fb.com>
> Acked-by: Andrii Nakryiko <andriin@fb.com>
> Signed-off-by: Stanislav Fomichev <sdf@google.com>
Acked-by: Yonghong Song <yhs@fb.com>
^ permalink raw reply
* Re: [PATCH] net: dst.h: Fix shifting signed 32-bit value by 31 bits problem
From: David Miller @ 2019-07-01 17:49 UTC (permalink / raw)
To: bnvandana; +Cc: netdev, linux-kernel, skhan, gregkh, linux-kernel-mentees
In-Reply-To: <20190701115539.6738-1-bnvandana@gmail.com>
From: Vandana BN <bnvandana@gmail.com>
Date: Mon, 1 Jul 2019 17:25:39 +0530
> Fix DST_FEATURE_ECN_CA to use "U" cast to avoid shifting signed
> 32-bit value by 31 bits problem.
>
> Signed-off-by: Vandana BN <bnvandana@gmail.com>
Applied.
^ permalink raw reply
* [PATCH net-next] bonding/main: fix NULL dereference in bond_select_active_slave()
From: Eric Dumazet @ 2019-07-01 17:48 UTC (permalink / raw)
To: David S . Miller
Cc: netdev, Eric Dumazet, Eric Dumazet, John Sperbeck, Jarod Wilson,
Jay Vosburgh, Veaceslav Falico, Andy Gospodarek
A bonding master can be up while best_slave is NULL.
[12105.636318] BUG: unable to handle kernel NULL pointer dereference at 0000000000000000
[12105.638204] mlx4_en: eth1: Linkstate event 1 -> 1
[12105.648984] IP: bond_select_active_slave+0x125/0x250
[12105.653977] PGD 0 P4D 0
[12105.656572] Oops: 0000 [#1] SMP PTI
[12105.660487] gsmi: Log Shutdown Reason 0x03
[12105.664620] Modules linked in: kvm_intel loop act_mirred uhaul vfat fat stg_standard_ftl stg_megablocks stg_idt stg_hdi stg elephant_dev_num stg_idt_eeprom w1_therm wire i2c_mux_pca954x i2c_mux mlx4_i2c i2c_usb cdc_acm ehci_pci ehci_hcd i2c_iimc mlx4_en mlx4_ib ib_uverbs ib_core mlx4_core [last unloaded: kvm_intel]
[12105.685686] mlx4_core 0000:03:00.0: dispatching link up event for port 2
[12105.685700] mlx4_en: eth2: Linkstate event 2 -> 1
[12105.685700] mlx4_en: eth2: Link Up (linkstate)
[12105.724452] Workqueue: bond0 bond_mii_monitor
[12105.728854] RIP: 0010:bond_select_active_slave+0x125/0x250
[12105.734355] RSP: 0018:ffffaf146a81fd88 EFLAGS: 00010246
[12105.739637] RAX: 0000000000000003 RBX: ffff8c62b03c6900 RCX: 0000000000000000
[12105.746838] RDX: 0000000000000000 RSI: ffffaf146a81fd08 RDI: ffff8c62b03c6000
[12105.754054] RBP: ffffaf146a81fdb8 R08: 0000000000000001 R09: ffff8c517d387600
[12105.761299] R10: 00000000001075d9 R11: ffffffffaceba92f R12: 0000000000000000
[12105.768553] R13: ffff8c8240ae4800 R14: 0000000000000000 R15: 0000000000000000
[12105.775748] FS: 0000000000000000(0000) GS:ffff8c62bfa40000(0000) knlGS:0000000000000000
[12105.783892] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[12105.789716] CR2: 0000000000000000 CR3: 0000000d0520e001 CR4: 00000000001626f0
[12105.796976] Call Trace:
[12105.799446] [<ffffffffac31d387>] bond_mii_monitor+0x497/0x6f0
[12105.805317] [<ffffffffabd42643>] process_one_work+0x143/0x370
[12105.811225] [<ffffffffabd42c7a>] worker_thread+0x4a/0x360
[12105.816761] [<ffffffffabd48bc5>] kthread+0x105/0x140
[12105.821865] [<ffffffffabd42c30>] ? rescuer_thread+0x380/0x380
[12105.827757] [<ffffffffabd48ac0>] ? kthread_associate_blkcg+0xc0/0xc0
[12105.834266] [<ffffffffac600241>] ret_from_fork+0x51/0x60
Fixes: e2a7420df2e0 ("bonding/main: convert to using slave printk macros")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: John Sperbeck <jsperbeck@google.com>
Cc: Jarod Wilson <jarod@redhat.com>
CC: Jay Vosburgh <j.vosburgh@gmail.com>
CC: Veaceslav Falico <vfalico@gmail.com>
CC: Andy Gospodarek <andy@greyhouse.net>
---
drivers/net/bonding/bond_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index a30595955a37a485b9e045a31969313f8336b668..84168455aded96dfd85b310841dee2a0d917b580 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -937,7 +937,7 @@ void bond_select_active_slave(struct bonding *bond)
return;
if (netif_carrier_ok(bond->dev))
- slave_info(bond->dev, best_slave->dev, "active interface up!\n");
+ netdev_info(bond->dev, "active interface up!\n");
else
netdev_info(bond->dev, "now running without any active interface!\n");
}
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* Re: [PATCH net-next v2 00/10] net: stmmac: 10GbE using XGMAC
From: David Miller @ 2019-07-01 17:46 UTC (permalink / raw)
To: Jose.Abreu
Cc: linux-kernel, netdev, Joao.Pinto, peppe.cavallaro,
alexandre.torgue
In-Reply-To: <BN8PR12MB32662DA0B5733E93D88E7D7DD3F90@BN8PR12MB3266.namprd12.prod.outlook.com>
From: Jose Abreu <Jose.Abreu@synopsys.com>
Date: Mon, 1 Jul 2019 10:19:55 +0000
> From: David Miller <davem@davemloft.net>
>
>> About the Kconfig change, maybe it just doesn't make sense to list all
>> of the various speeds the chip supports... just a thought.
>
> What about: "STMicroelectronics Multi-Gigabit Ethernet driver" ?
>
> Or, just "STMicroelectronics Ethernet driver" ?
Maybe the first one is better, I don't know. What are the chances of there
being more STMicroelectronics ethernet NICs? :-)
^ permalink raw reply
* Re: [PATCH net] Documentation/networking: fix default_ttl typo in mpls-sysctl
From: David Miller @ 2019-07-01 17:42 UTC (permalink / raw)
To: liuhangbin; +Cc: netdev
In-Reply-To: <20190701084528.25872-1-liuhangbin@gmail.com>
From: Hangbin Liu <liuhangbin@gmail.com>
Date: Mon, 1 Jul 2019 16:45:28 +0800
> default_ttl should be integer instead of bool
>
> Reported-by: Ying Xu <yinxu@redhat.com>
> Fixes: a59166e47086 ("mpls: allow TTL propagation from IP packets to be configured")
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
Applied, thank you.
^ permalink raw reply
* Re: [PATCH bpf-next 1/2] bpf: allow wide (u64) aligned stores for some fields of bpf_sock_addr
From: Yonghong Song @ 2019-07-01 17:40 UTC (permalink / raw)
To: Stanislav Fomichev, Andrii Nakryiko
Cc: Stanislav Fomichev, netdev@vger.kernel.org, bpf@vger.kernel.org,
davem@davemloft.net, ast@kernel.org, daniel@iogearbox.net,
Andrii Nakryiko, kernel test robot
In-Reply-To: <20190701160434.GD6757@mini-arch>
On 7/1/19 9:04 AM, Stanislav Fomichev wrote:
> On 07/01, Andrii Nakryiko wrote:
>> On Sat, Jun 29, 2019 at 10:53 PM Yonghong Song <yhs@fb.com> wrote:
>>>
>>>
>>>
>>> On 6/28/19 4:10 PM, Stanislav Fomichev 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>
>>>> Signed-off-by: Stanislav Fomichev <sdf@google.com>
>>>
>>> The change looks good to me with the following nits:
>>> 1. could you add a cover letter for the patch set?
>>> typically if the number of patches is more than one,
>>> it would be a good practice with a cover letter.
>>> See bpf_devel_QA.rst .
>>> 2. with this change, the comments in uapi bpf.h
>>> are not accurate any more.
>>> __u32 user_ip6[4]; /* Allows 1,2,4-byte read an 4-byte write.
>>> * Stored in network byte order.
>>>
>>> */
>>> __u32 msg_src_ip6[4]; /* Allows 1,2,4-byte read an 4-byte write.
>>> * Stored in network byte order.
>>> */
>>> now for stores, aligned 8-byte write is permitted.
>>> could you update this as well?
>>>
>>> From the typical usage pattern, I did not see a need
>>> for 8-tye read of user_ip6 and msg_src_ip6 yet. So let
>>> us just deal with write for now.
>>
>> But I guess it's still possible for clang to optimize two consecutive
>> 4-byte reads into single 8-byte read in some circumstances? If that's
>> the case, maybe it's a good idea to have corresponding read checks as
>> well?
> I guess clang can do those kinds of optimizations. I can put it on my
> todo and address later (or when we actually see it out in the wild).
Okay, I find a Facebook internal app. does trying to read the 4 bytes
and compare to a predefined loopback address. We may need to handle
read cases as well. But this can be a followup after actual tryout.
>
>> But overall this looks good to me:
>>
>> Acked-by: Andrii Nakryiko <andriin@fb.com>
> Thanks for a review!
>
>>>
>>> With the above two nits,
>>> Acked-by: Yonghong Song <yhs@fb.com>
>>>
>>>> ---
>>>> include/linux/filter.h | 6 ++++++
>>>> net/core/filter.c | 22 ++++++++++++++--------
>>>> 2 files changed, 20 insertions(+), 8 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/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); \
>>>>
^ permalink raw reply
* [PATCH bpf-next v3 3/3] selftests/bpf: add verifier tests for wide stores
From: Stanislav Fomichev @ 2019-07-01 17:38 UTC (permalink / raw)
To: netdev, bpf
Cc: davem, ast, daniel, Stanislav Fomichev, Andrii Nakryiko,
Yonghong Song
In-Reply-To: <20190701173841.32249-1-sdf@google.com>
Make sure that wide stores are allowed at proper (aligned) addresses.
Note that user_ip6 is naturally aligned on 8-byte boundary, so
correct addresses are user_ip6[0] and user_ip6[2]. msg_src_ip6 is,
however, aligned on a 4-byte bondary, so only msg_src_ip6[1]
can be wide-stored.
Cc: Andrii Nakryiko <andriin@fb.com>
Cc: Yonghong Song <yhs@fb.com>
Acked-by: Andrii Nakryiko <andriin@fb.com>
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
tools/testing/selftests/bpf/test_verifier.c | 17 +++++++--
.../selftests/bpf/verifier/wide_store.c | 36 +++++++++++++++++++
2 files changed, 50 insertions(+), 3 deletions(-)
create mode 100644 tools/testing/selftests/bpf/verifier/wide_store.c
diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
index c5514daf8865..b0773291012a 100644
--- a/tools/testing/selftests/bpf/test_verifier.c
+++ b/tools/testing/selftests/bpf/test_verifier.c
@@ -105,6 +105,7 @@ struct bpf_test {
__u64 data64[TEST_DATA_LEN / 8];
};
} retvals[MAX_TEST_RUNS];
+ enum bpf_attach_type expected_attach_type;
};
/* Note we want this to be 64 bit aligned so that the end of our array is
@@ -850,6 +851,7 @@ static void do_test_single(struct bpf_test *test, bool unpriv,
int fd_prog, expected_ret, alignment_prevented_execution;
int prog_len, prog_type = test->prog_type;
struct bpf_insn *prog = test->insns;
+ struct bpf_load_program_attr attr;
int run_errs, run_successes;
int map_fds[MAX_NR_MAPS];
const char *expected_err;
@@ -881,8 +883,17 @@ static void do_test_single(struct bpf_test *test, bool unpriv,
pflags |= BPF_F_STRICT_ALIGNMENT;
if (test->flags & F_NEEDS_EFFICIENT_UNALIGNED_ACCESS)
pflags |= BPF_F_ANY_ALIGNMENT;
- fd_prog = bpf_verify_program(prog_type, prog, prog_len, pflags,
- "GPL", 0, bpf_vlog, sizeof(bpf_vlog), 4);
+
+ memset(&attr, 0, sizeof(attr));
+ attr.prog_type = prog_type;
+ attr.expected_attach_type = test->expected_attach_type;
+ attr.insns = prog;
+ attr.insns_cnt = prog_len;
+ attr.license = "GPL";
+ attr.log_level = 4;
+ attr.prog_flags = pflags;
+
+ fd_prog = bpf_load_program_xattr(&attr, bpf_vlog, sizeof(bpf_vlog));
if (fd_prog < 0 && !bpf_probe_prog_type(prog_type, 0)) {
printf("SKIP (unsupported program type %d)\n", prog_type);
skips++;
@@ -912,7 +923,7 @@ static void do_test_single(struct bpf_test *test, bool unpriv,
printf("FAIL\nUnexpected success to load!\n");
goto fail_log;
}
- if (!strstr(bpf_vlog, expected_err)) {
+ if (!expected_err || !strstr(bpf_vlog, expected_err)) {
printf("FAIL\nUnexpected error message!\n\tEXP: %s\n\tRES: %s\n",
expected_err, bpf_vlog);
goto fail_log;
diff --git a/tools/testing/selftests/bpf/verifier/wide_store.c b/tools/testing/selftests/bpf/verifier/wide_store.c
new file mode 100644
index 000000000000..8fe99602ded4
--- /dev/null
+++ b/tools/testing/selftests/bpf/verifier/wide_store.c
@@ -0,0 +1,36 @@
+#define BPF_SOCK_ADDR(field, off, res, err) \
+{ \
+ "wide store to bpf_sock_addr." #field "[" #off "]", \
+ .insns = { \
+ BPF_MOV64_IMM(BPF_REG_0, 1), \
+ BPF_STX_MEM(BPF_DW, BPF_REG_1, BPF_REG_0, \
+ offsetof(struct bpf_sock_addr, field[off])), \
+ BPF_EXIT_INSN(), \
+ }, \
+ .result = res, \
+ .prog_type = BPF_PROG_TYPE_CGROUP_SOCK_ADDR, \
+ .expected_attach_type = BPF_CGROUP_UDP6_SENDMSG, \
+ .errstr = err, \
+}
+
+/* user_ip6[0] is u64 aligned */
+BPF_SOCK_ADDR(user_ip6, 0, ACCEPT,
+ NULL),
+BPF_SOCK_ADDR(user_ip6, 1, REJECT,
+ "invalid bpf_context access off=12 size=8"),
+BPF_SOCK_ADDR(user_ip6, 2, ACCEPT,
+ NULL),
+BPF_SOCK_ADDR(user_ip6, 3, REJECT,
+ "invalid bpf_context access off=20 size=8"),
+
+/* msg_src_ip6[0] is _not_ u64 aligned */
+BPF_SOCK_ADDR(msg_src_ip6, 0, REJECT,
+ "invalid bpf_context access off=44 size=8"),
+BPF_SOCK_ADDR(msg_src_ip6, 1, ACCEPT,
+ NULL),
+BPF_SOCK_ADDR(msg_src_ip6, 2, REJECT,
+ "invalid bpf_context access off=52 size=8"),
+BPF_SOCK_ADDR(msg_src_ip6, 3, REJECT,
+ "invalid bpf_context access off=56 size=8"),
+
+#undef BPF_SOCK_ADDR
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCH bpf-next v3 2/3] bpf: sync bpf.h to tools/
From: Stanislav Fomichev @ 2019-07-01 17:38 UTC (permalink / raw)
To: netdev, bpf; +Cc: davem, ast, daniel, Stanislav Fomichev
In-Reply-To: <20190701173841.32249-1-sdf@google.com>
Sync user_ip6 & msg_src_ip6 comments.
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
tools/include/uapi/linux/bpf.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index a396b516a2b2..c59dc921ce83 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/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 and 4,8-byte write.
* Stored in network byte order.
*/
__u32 user_port; /* Allows 4-byte read and write.
@@ -3246,10 +3246,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);
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [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
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