* Re: [PATCH net-next] net: hns3: Fix for warning uninitialized symbol hw_err_lst3
From: David Miller @ 2018-10-24 21:26 UTC (permalink / raw)
To: salil.mehta
Cc: yisen.zhuang, lipeng321, mehta.salil, dan.carpenter, netdev,
linux-kernel, linuxarm, shiju.jose
In-Reply-To: <20181024143218.4204-1-salil.mehta@huawei.com>
From: Salil Mehta <salil.mehta@huawei.com>
Date: Wed, 24 Oct 2018 15:32:18 +0100
> From: Shiju Jose <shiju.jose@huawei.com>
>
> This patch fixes the smatch warning,
>
> drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_err.c:700
> hclge_log_and_clear_ppp_error() error: uninitialized symbol
> 'hw_err_lst3'
>
> Link: https://lkml.org/lkml/2018/10/23/430
>
> Fixes: da2d072a9ea7 ("net: hns3: Add enable and process hw errors from PPP")
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: Shiju Jose <shiju.jose@huawei.com>
> Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
Applied.
^ permalink raw reply
* [PATCH net] ipv6/ndisc: Preserve IPv6 control buffer if protocol error handlers are called
From: Stefano Brivio @ 2018-10-24 12:37 UTC (permalink / raw)
To: David S. Miller; +Cc: Hideaki YOSHIFUJI, netdev
Commit a61bbcf28a8c ("[NET]: Store skb->timestamp as offset to a base
timestamp") introduces a neighbour control buffer and zeroes it out in
ndisc_rcv(), as ndisc_recv_ns() uses it.
Commit f2776ff04722 ("[IPV6]: Fix address/interface handling in UDP and
DCCP, according to the scoping architecture.") introduces the usage of the
IPv6 control buffer in protocol error handlers (e.g. inet6_iif() in
present-day __udp6_lib_err()).
Now, with commit b94f1c0904da ("ipv6: Use icmpv6_notify() to propagate
redirect, instead of rt6_redirect()."), we call protocol error handlers
from ndisc_redirect_rcv(), after the control buffer is already stolen and
some parts are already zeroed out. This implies that inet6_iif() on this
path will always return zero.
This gives unexpected results on UDP socket lookup in __udp6_lib_err(), as
we might actually need to match sockets for a given interface.
Instead of always claiming the control buffer in ndisc_rcv(), do that only
when needed.
Fixes: b94f1c0904da ("ipv6: Use icmpv6_notify() to propagate redirect, instead of rt6_redirect().")
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
net/ipv6/ndisc.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
index 0ec273997d1d..673a4a932f2a 100644
--- a/net/ipv6/ndisc.c
+++ b/net/ipv6/ndisc.c
@@ -1732,10 +1732,9 @@ int ndisc_rcv(struct sk_buff *skb)
return 0;
}
- memset(NEIGH_CB(skb), 0, sizeof(struct neighbour_cb));
-
switch (msg->icmph.icmp6_type) {
case NDISC_NEIGHBOUR_SOLICITATION:
+ memset(NEIGH_CB(skb), 0, sizeof(struct neighbour_cb));
ndisc_recv_ns(skb);
break;
--
2.19.1
^ permalink raw reply related
* Re: [PATCH] Change judgment len position
From: Willy Tarreau @ 2018-10-24 20:48 UTC (permalink / raw)
To: Joe Perches; +Cc: David Miller, wanghaifine, netdev, LKML
In-Reply-To: <61d94f2a5563db4d6580c8385c3b93c8eeb3669a.camel@perches.com>
On Wed, Oct 24, 2018 at 11:28:11AM -0700, Joe Perches wrote:
> On Wed, 2018-10-24 at 10:10 -0700, David Miller wrote:
> > From: Wang Hai <wanghaifine@gmail.com>
> > Date: Wed, 24 Oct 2018 23:47:29 +0800
> >
> > > To determine whether len is less than zero, it should be put before
> > > the function min_t, because the return value of min_t is not likely
> > > to be less than zero.
> []
> > > diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> []
> > > @@ -3301,11 +3301,11 @@ static int do_tcp_getsockopt(struct sock *sk, int level,
> > > struct net *net = sock_net(sk);
> > > int val, len;
> > >
> > > + len = min_t(unsigned int, len, sizeof(int));
> > > +
> > > if (get_user(len, optlen))
> > > return -EFAULT;
> >
> > You can't be serious?
>
> I'm not personally taken aback by this but
> there is the new Code of
> Conduct to consider.
>
> John McEnroe earned quite a bit of his
> reputation as an 'enfant terrible' via a
> similar statement.
>
> https://www.youtube.com/watch?v=t0hK1wyrrAU
>
> Perhaps a different word choice next time in
> reply to submitters of ill-considered and/or
> defective patches could be useful.
Maybe but on this one I think we're really out of the scope of the CoC.
When you read this patch from an apparent first-time contributor (no
trace in either LKML, netdev or even google), the level of assurance
in the commit message is pretty good, showing that he's not at all a
beginner, which doesn't match at all the type of error seen in the
code, which doesn't even need to be compiled to see that it will emit
a warning and not work as advertised. Moreover, the commit message is
vague enough to seem it tries to cover the patch, and doesn't even
match what's done in the patch.
When you factor in the fact that the code opens a big but very discrete
vulnerability, I tend to think that in fact we're not facing a newbie
at all but someone deliberately trying to inject a subtle backdoor into
the kernel and disguise it as a vague bug fix, possibly even hoping that
it would find its way to -stable. I would not be surprised if this e-mail
address is a throw-away anonymous address created just for this occasion.
I could totally be wrong of course, but the clues are quite heavy here
as I find it hard to argue for a series of beginner's mistakes. If this
person really exists and can explain how we ended up there, I will of
course happily retract my suspicion and apologize.
Willy
^ permalink raw reply
* Re: [PATCH v2] bpf: btf: Fix a missing-check bug
From: Martin Lau @ 2018-10-24 20:42 UTC (permalink / raw)
To: Wenwen Wang
Cc: Kangjie Lu, Alexei Starovoitov, Daniel Borkmann,
open list:BPF (Safe dynamic programs and tools),
open list:BPF (Safe dynamic programs and tools)
In-Reply-To: <20181024182239.lz7uicceihzmxabh@kafai-mbp>
On Wed, Oct 24, 2018 at 06:22:46PM +0000, Martin Lau wrote:
> On Wed, Oct 24, 2018 at 05:26:23PM +0000, Martin Lau wrote:
> > On Wed, Oct 24, 2018 at 08:00:19AM -0500, Wenwen Wang wrote:
> > > In btf_parse(), the header of the user-space btf data 'btf_data' is firstly
> > > parsed and verified through btf_parse_hdr(). In btf_parse_hdr(), the header
> > > is copied from user-space 'btf_data' to kernel-space 'btf->hdr' and then
> > > verified. If no error happens during the verification process, the whole
> > > data of 'btf_data', including the header, is then copied to 'data' in
> > > btf_parse(). It is obvious that the header is copied twice here. More
> > > importantly, no check is enforced after the second copy to make sure the
> > > headers obtained in these two copies are same. Given that 'btf_data'
> > > resides in the user space, a malicious user can race to modify the header
> > > between these two copies. By doing so, the user can inject inconsistent
> > > data, which can cause undefined behavior of the kernel and introduce
> > > potential security risk.
> btw, I am working on a patch that copies the btf_data before parsing/verifying
> the header. That should avoid this from happening but that will
> require a bit more code churns for the bpf branch.
>
It is what I have in mind:
It is not a good idea to check the BTF header before copying the
user btf_data. The verified header may not be the one actually
copied to btf->data (e.g. userspace may modify the passed in
btf_data in between). Like the one fixed in
commit 8af03d1ae2e1 ("bpf: btf: Fix a missing check bug").
This patch copies the user btf_data before parsing/verifying
the BTF header.
Fixes: 69b693f0aefa ("bpf: btf: Introduce BPF Type Format (BTF)")
Signed-off-by: Martin KaFai Lau <kafai@fb.com>
---
kernel/bpf/btf.c | 58 +++++++++++++++++++++---------------------------
1 file changed, 25 insertions(+), 33 deletions(-)
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 378cef70341c..ee4c82667d65 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -2067,56 +2067,47 @@ static int btf_check_sec_info(struct btf_verifier_env *env,
return 0;
}
-static int btf_parse_hdr(struct btf_verifier_env *env, void __user *btf_data,
- u32 btf_data_size)
+static int btf_parse_hdr(struct btf_verifier_env *env)
{
+ u32 hdr_len, hdr_copy, btf_data_size;
const struct btf_header *hdr;
- u32 hdr_len, hdr_copy;
- /*
- * Minimal part of the "struct btf_header" that
- * contains the hdr_len.
- */
- struct btf_min_header {
- u16 magic;
- u8 version;
- u8 flags;
- u32 hdr_len;
- } __user *min_hdr;
struct btf *btf;
int err;
btf = env->btf;
- min_hdr = btf_data;
+ btf_data_size = btf->data_size;
- if (btf_data_size < sizeof(*min_hdr)) {
+ if (btf_data_size <
+ offsetof(struct btf_header, hdr_len) + sizeof(hdr->hdr_len)) {
btf_verifier_log(env, "hdr_len not found");
return -EINVAL;
}
- if (get_user(hdr_len, &min_hdr->hdr_len))
- return -EFAULT;
-
+ hdr = btf->data;
+ hdr_len = hdr->hdr_len;
if (btf_data_size < hdr_len) {
btf_verifier_log(env, "btf_header not found");
return -EINVAL;
}
- err = bpf_check_uarg_tail_zero(btf_data, sizeof(btf->hdr), hdr_len);
- if (err) {
- if (err == -E2BIG)
- btf_verifier_log(env, "Unsupported btf_header");
- return err;
+ /* Ensure the unsupported header fields are zero */
+ if (hdr_len > sizeof(btf->hdr)) {
+ u8 *expected_zero = btf->data + sizeof(btf->hdr);
+ u8 *end = btf->data + hdr_len;
+
+ for (; expected_zero < end; expected_zero++) {
+ if (*expected_zero) {
+ btf_verifier_log(env, "Unsupported btf_header");
+ return -E2BIG;
+ }
+ }
}
hdr_copy = min_t(u32, hdr_len, sizeof(btf->hdr));
- if (copy_from_user(&btf->hdr, btf_data, hdr_copy))
- return -EFAULT;
+ memcpy(&btf->hdr, btf->data, hdr_copy);
hdr = &btf->hdr;
- if (hdr->hdr_len != hdr_len)
- return -EINVAL;
-
btf_verifier_log_hdr(env, btf_data_size);
if (hdr->magic != BTF_MAGIC) {
@@ -2186,10 +2177,6 @@ static struct btf *btf_parse(void __user *btf_data, u32 btf_data_size,
}
env->btf = btf;
- err = btf_parse_hdr(env, btf_data, btf_data_size);
- if (err)
- goto errout;
-
data = kvmalloc(btf_data_size, GFP_KERNEL | __GFP_NOWARN);
if (!data) {
err = -ENOMEM;
@@ -2198,13 +2185,18 @@ static struct btf *btf_parse(void __user *btf_data, u32 btf_data_size,
btf->data = data;
btf->data_size = btf_data_size;
- btf->nohdr_data = btf->data + btf->hdr.hdr_len;
if (copy_from_user(data, btf_data, btf_data_size)) {
err = -EFAULT;
goto errout;
}
+ err = btf_parse_hdr(env);
+ if (err)
+ goto errout;
+
+ btf->nohdr_data = btf->data + btf->hdr.hdr_len;
+
err = btf_parse_str_sec(env);
if (err)
goto errout;
--
2.17.1
^ permalink raw reply related
* Re: [PATCH net-next 3/4] net: phy-c45: Implement reset/suspend/resume callbacks
From: Andrew Lunn @ 2018-10-24 12:03 UTC (permalink / raw)
To: Jose Abreu
Cc: Russell King - ARM Linux, Florian Fainelli, netdev,
David S. Miller, Joao Pinto
In-Reply-To: <5e65254c-04dc-6dba-eb1c-10bafadf95c5@synopsys.com>
> Since 719655a14971 ("net: phy: Replace phy driver features u32
> with link_mode bitmap"), phy_probe() calls
> ethtool_convert_link_mode_to_legacy_u32() with phydrv->features
> as argument. Since features are NULL, we will get NULL pointer
> dereference.
Hi Jose
Thanks for pointing that out. I will fix it ASAP.
Andrew
^ permalink raw reply
* Hello My Dear Friend,
From: Mr Marc Joseph Hebert @ 2018-10-24 11:53 UTC (permalink / raw)
In-Reply-To: <643125976.71607.1540382036243.ref@mail.yahoo.com>
I am Mr Marc Joseph Hebert a I work in the Finance Risk control/Accounts Broker Unit of a prestigious bank in London. Under varying state laws in United Kingdom, financial institutions and other companies are required to turn over any funds considered "abandoned," including uncashed paychecks, forgotten bank account balances, unclaimed refunds, insurance payouts and contents of safe deposit boxes. I have the official duty to process and release unclaimed funds in the bank to government treasury.
Recently, there are multiple abandoned accounts in the bank which I have transferred some to the government treasury. Some of these funds are what I want to transfer (10.6m GBP) out of the bank to a sincere and
trustworthy person for either investment purpose or sharing between us. Can you handle this with confidentiality, sincerity and seriousness?
Please indicate your interest by simply replying to this email with your full personal details below.
(1) Your Full Name:
(2) Full Residential Address:
(3) Phone And Fax Number:
(4) Occupation:
(5) Whatsapp Number:
I anticipate your urgent response to this financial deal.
Your responds should be forwarded to my private email below.
marc.joseph.hebert1@gmail.com
Sincerely,
Mr Marc Joseph Hebert
Finance Risk control/Accounts Broker Unit.
^ permalink raw reply
* Re: [net-next 03/11] igc: Add netdev
From: Neftin, Sasha @ 2018-10-24 11:43 UTC (permalink / raw)
To: Jakub Kicinski, Jeff Kirsher
Cc: davem, netdev, nhorman, sassmann, Neftin, Sasha
In-Reply-To: <20181018101512.25588895@cakuba.netronome.com>
On 10/18/2018 20:15, Jakub Kicinski wrote:
> On Wed, 17 Oct 2018 15:23:14 -0700, Jeff Kirsher wrote:
>> +/**
>> + * igc_ioctl - I/O control method
>> + * @netdev: network interface device structure
>> + * @ifreq: frequency
>
> Is it? :)
>
Ah... Good catch. I will fix that and submit patch.
>> + * @cmd: command
>> + */
>> +static int igc_ioctl(struct net_device *netdev, struct ifreq *ifr, int cmd)
>> +{
>> + switch (cmd) {
>> + default:
>> + return -EOPNOTSUPP;
>> + }
>> +}
>
> You don't seem to be adding anything to this function in the series.
> Why add the stub?
>
Right. Still not in use. I will remove and add per demand.
Thanks for your comments.
^ permalink raw reply
* Re: [PATCH] Change judgment len position
From: Willy Tarreau @ 2018-10-24 20:09 UTC (permalink / raw)
To: Eric Dumazet
Cc: joe, wanghaifine, David Miller, Alexey Kuznetsov,
Hideaki YOSHIFUJI, netdev, LKML
In-Reply-To: <CANn89iLH=VP1i=KS5QV1x41EpQM5-o1TJfDh01Y++bMpFpfBRg@mail.gmail.com>
On Wed, Oct 24, 2018 at 10:03:08AM -0700, Eric Dumazet wrote:
> On Wed, Oct 24, 2018 at 9:54 AM Joe Perches <joe@perches.com> wrote:
>
> > I think if the point is to test for negative numbers,
> > it's clearer to do that before using min_t.and it's
> > probably clearer not to use min_t at all.
> >
>
> ...
>
> >
> > if (len > sizeof(int))
> > len = sizeof(int);
>
> It is a matter of taste really, I know some people (like me) sometimes
> mixes min() and max()
I do mix them up a lot as well because I tend to read "x=min(y,4)" as
"take y with a minimum value of 4" which in fact would be "max(y,4)".
> I would suggest that if someones wants to change the current code, a
> corresponding test
> would be added in tools/testing/selftests/net
In any case, what matters to me is that for now the only risk the existing
code represents is to overwrite up to one int of some userspace if the size
is negative, and we don't want that a wrong fix results in doing something
worse by accident like reading 2GB of kernel memory. I agree that Joe's
test with len<0 then len>sizeof(int) seems to work, but a test is probably
useful at least to ensure that the next person who passes there and wants
to turn this into min_t() again clearly catches all bad cases.
Regards,
Willy
^ permalink raw reply
* [PATCH v2 bpf] bpf: devmap: fix wrong interface selection in notifier_call
From: Taehee Yoo @ 2018-10-24 11:15 UTC (permalink / raw)
To: daniel, ast; +Cc: netdev, john.fastabend, ap420073
The dev_map_notification() removes interface in devmap if
unregistering interface's ifindex is same.
But only checking ifindex is not enough because other netns can have
same ifindex. so that wrong interface selection could occurred.
Hence netdev pointer comparison code is added.
v2: compare netdev pointer instead of using net_eq() (Daniel Borkmann)
v1: Initial patch
Fixes: 2ddf71e23cc2 ("net: add notifier hooks for devmap bpf map")
Signed-off-by: Taehee Yoo <ap420073@gmail.com>
---
kernel/bpf/devmap.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c
index 141710b82a6c..191b79948424 100644
--- a/kernel/bpf/devmap.c
+++ b/kernel/bpf/devmap.c
@@ -512,8 +512,7 @@ static int dev_map_notification(struct notifier_block *notifier,
struct bpf_dtab_netdev *dev, *odev;
dev = READ_ONCE(dtab->netdev_map[i]);
- if (!dev ||
- dev->dev->ifindex != netdev->ifindex)
+ if (!dev || netdev != dev->dev)
continue;
odev = cmpxchg(&dtab->netdev_map[i], dev, NULL);
if (dev == odev)
--
2.17.1
^ permalink raw reply related
* [PATCH RFC] net: dsa: Make switches VLAN aware when enslaved into a bridge
From: Florian Fainelli @ 2018-10-24 19:36 UTC (permalink / raw)
To: netdev
Cc: jiri, petr, idosch, privat, Woojung.Huh, tristram.ha,
Florian Fainelli, Andrew Lunn, Vivien Didelot, David S. Miller,
open list
Commit 2ea7a679ca2a ("net: dsa: Don't add vlans when vlan filtering is
disabled") changed the behavior of DSA switches when the switch ports
are enslaved into the bridge and only pushed the VLAN configuration down
to the switch if the bridge is configured with VLAN filtering enabled.
This is unfortunately wrong, because what vlan_filtering configures is a
policy on the acceptance of VLAN tagged frames with an unknown VID.
vlan_filtering=0 means a frame with a VLAN tag that is not part of the
VLAN table should be allowed to ingress the switch, and vlan_fltering=1
would reject that frame.
Fixes: 2ea7a679ca2a ("net: dsa: Don't add vlans when vlan filtering is disabled")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
Andrew,
I checked with Jiri and he confirmed that our interpretention of
vlan_filtering in DSA was incorrect and that it does denote whether the
switch should be doing VID ingress policy checking.
You mentioned in the commit message some problems without being too
specific about them which is why I am putting the same checks in
mv88e6xxx in order not to break your use cases. Let me know if you want
to drop that hunk entirely.
Thanks!
drivers/net/dsa/mv88e6xxx/chip.c | 7 ++++++-
net/dsa/port.c | 10 ++--------
2 files changed, 8 insertions(+), 9 deletions(-)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 8da3d39e3218..df411e776911 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -1684,13 +1684,14 @@ static int _mv88e6xxx_port_vlan_add(struct mv88e6xxx_chip *chip, int port,
static void mv88e6xxx_port_vlan_add(struct dsa_switch *ds, int port,
const struct switchdev_obj_port_vlan *vlan)
{
+ struct dsa_port *dp = &ds->ports[i];
struct mv88e6xxx_chip *chip = ds->priv;
bool untagged = vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED;
bool pvid = vlan->flags & BRIDGE_VLAN_INFO_PVID;
u8 member;
u16 vid;
- if (!chip->info->max_vid)
+ if (!chip->info->max_vid || br_vlan_enabled(dp->bridge_dev))
return;
if (dsa_is_dsa_port(ds, port) || dsa_is_cpu_port(ds, port))
@@ -1751,12 +1752,16 @@ static int mv88e6xxx_port_vlan_del(struct dsa_switch *ds, int port,
const struct switchdev_obj_port_vlan *vlan)
{
struct mv88e6xxx_chip *chip = ds->priv;
+ struct dsa_port *dp = &ds->ports[i];
u16 pvid, vid;
int err = 0;
if (!chip->info->max_vid)
return -EOPNOTSUPP;
+ if (br_vlan_enabled(dp->bridge_dev))
+ return 0;
+
mutex_lock(&chip->reg_lock);
err = mv88e6xxx_port_get_pvid(chip, port, &pvid);
diff --git a/net/dsa/port.c b/net/dsa/port.c
index ed0595459df1..111d7cfc8982 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -255,10 +255,7 @@ int dsa_port_vlan_add(struct dsa_port *dp,
if (netif_is_bridge_master(vlan->obj.orig_dev))
return -EOPNOTSUPP;
- if (br_vlan_enabled(dp->bridge_dev))
- return dsa_port_notify(dp, DSA_NOTIFIER_VLAN_ADD, &info);
-
- return 0;
+ return dsa_port_notify(dp, DSA_NOTIFIER_VLAN_ADD, &info);
}
int dsa_port_vlan_del(struct dsa_port *dp,
@@ -273,10 +270,7 @@ int dsa_port_vlan_del(struct dsa_port *dp,
if (netif_is_bridge_master(vlan->obj.orig_dev))
return -EOPNOTSUPP;
- if (br_vlan_enabled(dp->bridge_dev))
- return dsa_port_notify(dp, DSA_NOTIFIER_VLAN_DEL, &info);
-
- return 0;
+ return dsa_port_notify(dp, DSA_NOTIFIER_VLAN_DEL, &info);
}
static struct phy_device *dsa_port_get_phy_device(struct dsa_port *dp)
--
2.17.1
^ permalink raw reply related
* Re: [PATCH v3 net-next 03/11] net: dsa: microchip: Initialize mutex before use
From: Pavel Machek @ 2018-10-24 11:07 UTC (permalink / raw)
To: Tristram.Ha
Cc: Andrew Lunn, Florian Fainelli, Ruediger Schmitt,
Arkadi Sharshevsky, UNGLinuxDriver, netdev
In-Reply-To: <1540261575-1889-4-git-send-email-Tristram.Ha@microchip.com>
[-- Attachment #1: Type: text/plain, Size: 1324 bytes --]
On Mon 2018-10-22 19:26:07, Tristram.Ha@microchip.com wrote:
> From: Tristram Ha <Tristram.Ha@microchip.com>
>
> Initialize mutex before use.
>
> Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
> ---
> drivers/net/dsa/microchip/ksz_common.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
> index 8c5853e..88e8d2a 100644
> --- a/drivers/net/dsa/microchip/ksz_common.c
> +++ b/drivers/net/dsa/microchip/ksz_common.c
> @@ -1118,7 +1118,6 @@ static int ksz_switch_init(struct ksz_device *dev)
> {
> int i;
>
> - mutex_init(&dev->reg_mutex);
> mutex_init(&dev->stats_mutex);
> mutex_init(&dev->alu_mutex);
> mutex_init(&dev->vlan_mutex);
> @@ -1207,6 +1206,9 @@ int ksz_switch_register(struct ksz_device *dev)
> if (dev->pdata)
> dev->chip_id = dev->pdata->chip_id;
>
> + /* mutex is used in next function call. */
> + mutex_init(&dev->reg_mutex);
> +
> if (ksz_switch_detect(dev))
> return -EINVAL;
Actually, would it make sense to move all mutex_init's there? No harm
in doing them sooner...
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* Re: [PATCH v3 net-next 01/11] net: dsa: microchip: Replace license with GPL
From: Pavel Machek @ 2018-10-24 11:05 UTC (permalink / raw)
To: Tristram.Ha
Cc: Andrew Lunn, Florian Fainelli, Ruediger Schmitt, UNGLinuxDriver,
netdev
In-Reply-To: <1540261575-1889-2-git-send-email-Tristram.Ha@microchip.com>
[-- Attachment #1: Type: text/plain, Size: 1246 bytes --]
On Mon 2018-10-22 19:26:05, Tristram.Ha@microchip.com wrote:
> From: Tristram Ha <Tristram.Ha@microchip.com>
>
> Replace license with GPL.
>
> Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
> Reviewed-by: Woojung Huh <Woojung.Huh@microchip.com>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Acked-by: Pavel Machek <pavel@ucw.cz>
> @@ -1,19 +1,20 @@
> /*
> * Microchip KSZ9477 register definitions
> *
> - * Copyright (C) 2017
> + * Copyright (C) 2017-2018 Microchip Technology Inc.
> *
> - * Permission to use, copy, modify, and/or distribute this software for any
> - * purpose with or without fee is hereby granted, provided that the above
> - * copyright notice and this permission notice appear in all copies.
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version 2
> + * of the License, or (at your option) any later version.
> *
But going to SPDX and getting rid of this legaleese would be even
better.
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* Re: [PATCH bpf] bpf: devmap: fix wrong interface selection in notifier_call
From: Taehee Yoo @ 2018-10-24 11:03 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: ast, Netdev, john.fastabend
In-Reply-To: <82dbe1ae-bf7b-2eca-1d19-b6c72c5a79c8@iogearbox.net>
Hi Daniel!
On Wed, 24 Oct 2018 at 18:07, Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> [ +John ]
>
> On 10/22/2018 05:41 PM, Taehee Yoo wrote:
> > The dev_map_notification() removes interface in devmap if
> > unregistering interface's ifindex is same.
> > But only checking ifindex is not enough because other netns can have
> > same ifindex. so that wrong interface selection could occurred.
> > Hence the net_eq() is needed.
> >
> > Fixes: 2ddf71e23cc2 ("net: add notifier hooks for devmap bpf map")
> > Signed-off-by: Taehee Yoo <ap420073@gmail.com>
> > ---
> > kernel/bpf/devmap.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c
> > index 141710b82a6c..0d9211e49a4a 100644
> > --- a/kernel/bpf/devmap.c
> > +++ b/kernel/bpf/devmap.c
> > @@ -496,6 +496,7 @@ static int dev_map_notification(struct notifier_block *notifier,
> > ulong event, void *ptr)
> > {
> > struct net_device *netdev = netdev_notifier_info_to_dev(ptr);
> > + struct net *net = dev_net(netdev);
> > struct bpf_dtab *dtab;
> > int i;
> >
> > @@ -512,7 +513,7 @@ static int dev_map_notification(struct notifier_block *notifier,
> > struct bpf_dtab_netdev *dev, *odev;
> >
> > dev = READ_ONCE(dtab->netdev_map[i]);
> > - if (!dev ||
> > + if (!dev || !net_eq(net, dev_net(dev->dev)) ||
> > dev->dev->ifindex != netdev->ifindex)
>
> Can't we just compare netdev pointers directly here?
>
Thank you for review!
I tested what you suggested, It works well and it's more simple.
So that I will send v2 patch that just compares netdev pointer instead of
using ifindex and net_eq().
> > continue;
> > odev = cmpxchg(&dtab->netdev_map[i], dev, NULL);
> >
>
Thanks!
^ permalink raw reply
* Re: [RFC PATCH v2 00/10] udp: implement GRO support
From: Steffen Klassert @ 2018-10-24 10:55 UTC (permalink / raw)
To: Paolo Abeni; +Cc: netdev, Willem de Bruijn
In-Reply-To: <7aecb7871f3e330debdeb81fd8a401ec5692fa81.camel@redhat.com>
On Tue, Oct 23, 2018 at 02:22:12PM +0200, Paolo Abeni wrote:
> On Tue, 2018-10-23 at 14:10 +0200, Steffen Klassert wrote:
>
> > Some quick benchmark numbers with UDP packet forwarding
> > (1460 byte packets) through two gateways:
> >
> > net-next: 16.4 Gbps
> >
> > net-next + UDP GRO: 20.3 Gbps
>
> uhmmm... what do you think about this speed-up ?
skb_segment() burns a lot of cycles. If I do the same test with
TCP and disable HW TSO, throughput drops also down to similar
values.
In case of software segmentation, the skb chain appropach
is likely faster because packets are not mangled. So no need
to allocate skbs, no new checksum calculations, less memcpy etc.
If we have an early route lookup in GRO, we could have a good
guess on the offload capabilities of the outgoing device.
So in case that software segmentation is likely, use the
skb chaining method. If HW segmentation is likely, merge
IP packets.
The chaining method might be also faster on non UDP GRO enabled
sockets.
I'll try to implement the skb chaining method on top of this
to see what we get from that.
^ permalink raw reply
* Re: [PATCH][next] igc: fix error return handling from call to netif_set_real_num_tx_queues
From: Neftin, Sasha @ 2018-10-24 10:38 UTC (permalink / raw)
To: Colin King, Jeff Kirsher, David S . Miller, intel-wired-lan
Cc: kernel-janitors, netdev, Neftin, Sasha
In-Reply-To: <20181019181615.27528-1-colin.king@canonical.com>
On 10/19/2018 21:16, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> The call to netif_set_real_num_tx_queues is not assigning the error
> return to variable err even though the next line checks err for an
> error. Fix this by adding the missing err assignment.
>
> Detected by CoverityScan, CID#1474551 ("Logically dead code")
>
> Fixes: 3df25e4c1e66 ("igc: Add interrupt support")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
> drivers/net/ethernet/intel/igc/igc_main.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
> index 9d85707e8a81..80ddbd987764 100644
> --- a/drivers/net/ethernet/intel/igc/igc_main.c
> +++ b/drivers/net/ethernet/intel/igc/igc_main.c
> @@ -3358,7 +3358,7 @@ static int __igc_open(struct net_device *netdev, bool resuming)
> goto err_req_irq;
>
> /* Notify the stack of the actual queue counts. */
> - netif_set_real_num_tx_queues(netdev, adapter->num_tx_queues);
> + err = netif_set_real_num_tx_queues(netdev, adapter->num_tx_queues);
> if (err)
> goto err_set_queues;
>
>
Thanks for the patch. Good catch of my typo.
Acked-by: Sasha Neftin <sasha.neftin@intel.com>
^ permalink raw reply
* Re: [PATCH net-next] igc: Remove set but not used variables 'ctrl_ext, link_mode'
From: Neftin, Sasha @ 2018-10-24 10:20 UTC (permalink / raw)
To: YueHaibing, Jeff Kirsher
Cc: intel-wired-lan, netdev, kernel-janitors, Neftin, Sasha
In-Reply-To: <1539952830-187358-1-git-send-email-yuehaibing@huawei.com>
On 10/19/2018 15:40, YueHaibing wrote:
> Fixes gcc '-Wunused-but-set-variable' warning:
>
> drivers/net/ethernet/intel/igc/igc_base.c: In function 'igc_init_phy_params_base':
> drivers/net/ethernet/intel/igc/igc_base.c:240:6: warning:
> variable 'ctrl_ext' set but not used [-Wunused-but-set-variable]
> u32 ctrl_ext;
>
> drivers/net/ethernet/intel/igc/igc_base.c: In function 'igc_get_invariants_base':
> drivers/net/ethernet/intel/igc/igc_base.c:290:6: warning:
> variable 'link_mode' set but not used [-Wunused-but-set-variable]
> u32 link_mode = 0;
>
> It never used since introduction in
> commit c0071c7aa5fe ("igc: Add HW initialization code")
>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> ---
> I'm not sure that reading IGC_CTRL_EXT is necessary.
> ---
> drivers/net/ethernet/intel/igc/igc_base.c | 8 --------
> 1 file changed, 8 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/igc/igc_base.c b/drivers/net/ethernet/intel/igc/igc_base.c
> index 832da609..df40af7 100644
> --- a/drivers/net/ethernet/intel/igc/igc_base.c
> +++ b/drivers/net/ethernet/intel/igc/igc_base.c
> @@ -237,7 +237,6 @@ static s32 igc_init_phy_params_base(struct igc_hw *hw)
> {
> struct igc_phy_info *phy = &hw->phy;
> s32 ret_val = 0;
> - u32 ctrl_ext;
>
> if (hw->phy.media_type != igc_media_type_copper) {
> phy->type = igc_phy_none;
> @@ -247,8 +246,6 @@ static s32 igc_init_phy_params_base(struct igc_hw *hw)
> phy->autoneg_mask = AUTONEG_ADVERTISE_SPEED_DEFAULT_2500;
> phy->reset_delay_us = 100;
>
> - ctrl_ext = rd32(IGC_CTRL_EXT);
> -
> /* set lan id */
> hw->bus.func = (rd32(IGC_STATUS) & IGC_STATUS_FUNC_MASK) >>
> IGC_STATUS_FUNC_SHIFT;
> @@ -287,8 +284,6 @@ static s32 igc_init_phy_params_base(struct igc_hw *hw)
> static s32 igc_get_invariants_base(struct igc_hw *hw)
> {
> struct igc_mac_info *mac = &hw->mac;
> - u32 link_mode = 0;
> - u32 ctrl_ext = 0;
> s32 ret_val = 0;
>
> switch (hw->device_id) {
> @@ -302,9 +297,6 @@ static s32 igc_get_invariants_base(struct igc_hw *hw)
>
> hw->phy.media_type = igc_media_type_copper;
>
> - ctrl_ext = rd32(IGC_CTRL_EXT);
> - link_mode = ctrl_ext & IGC_CTRL_EXT_LINK_MODE_MASK;
> -
> /* mac initialization and operations */
> ret_val = igc_init_mac_params_base(hw);
> if (ret_val)
>
Thanks for the patch. Good.
Acked-by: Sasha Neftin <sasha.neftin@intel.com>
^ permalink raw reply
* Re: [PATCH] Change judgment len position
From: Joe Perches @ 2018-10-24 18:28 UTC (permalink / raw)
To: David Miller, wanghaifine; +Cc: netdev, LKML
In-Reply-To: <20181024.101028.1211941922121897721.davem@davemloft.net>
On Wed, 2018-10-24 at 10:10 -0700, David Miller wrote:
> From: Wang Hai <wanghaifine@gmail.com>
> Date: Wed, 24 Oct 2018 23:47:29 +0800
>
> > To determine whether len is less than zero, it should be put before
> > the function min_t, because the return value of min_t is not likely
> > to be less than zero.
[]
> > diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
[]
> > @@ -3301,11 +3301,11 @@ static int do_tcp_getsockopt(struct sock *sk, int level,
> > struct net *net = sock_net(sk);
> > int val, len;
> >
> > + len = min_t(unsigned int, len, sizeof(int));
> > +
> > if (get_user(len, optlen))
> > return -EFAULT;
>
> You can't be serious?
I'm not personally taken aback by this but
there is the new Code of
Conduct to consider.
John McEnroe earned quite a bit of his
reputation as an 'enfant terrible' via a
similar statement.
https://www.youtube.com/watch?v=t0hK1wyrrAU
Perhaps a different word choice next time in
reply to submitters of ill-considered and/or
defective patches could be useful.
^ permalink raw reply
* Attribute failed policy validation
From: Marco Berizzi @ 2018-10-24 9:56 UTC (permalink / raw)
To: netdev
Hi Folks,
I'm getting this message after the upgrade from linux 4.18
to 4.19
root@Calimero:~# tc qdisc add dev eth0 root handle 1:0 hfsc default 1
Error: Attribute failed policy validation.
root@Calimero:~# lsmod | grep hfsc
sch_hfsc 24576 0
root@Calimero:~# tc -V
tc utility, iproute2-ss181023
Am I doing anything wrong?
^ permalink raw reply
* Re: [PATCH v2] bpf: btf: Fix a missing-check bug
From: Martin Lau @ 2018-10-24 18:22 UTC (permalink / raw)
To: Wenwen Wang
Cc: Kangjie Lu, Alexei Starovoitov, Daniel Borkmann,
open list:BPF (Safe dynamic programs and tools),
open list:BPF (Safe dynamic programs and tools)
In-Reply-To: <20181024172514.l33dsaqdvs5yewvm@kafai-mbp>
On Wed, Oct 24, 2018 at 05:26:23PM +0000, Martin Lau wrote:
> On Wed, Oct 24, 2018 at 08:00:19AM -0500, Wenwen Wang wrote:
> > In btf_parse(), the header of the user-space btf data 'btf_data' is firstly
> > parsed and verified through btf_parse_hdr(). In btf_parse_hdr(), the header
> > is copied from user-space 'btf_data' to kernel-space 'btf->hdr' and then
> > verified. If no error happens during the verification process, the whole
> > data of 'btf_data', including the header, is then copied to 'data' in
> > btf_parse(). It is obvious that the header is copied twice here. More
> > importantly, no check is enforced after the second copy to make sure the
> > headers obtained in these two copies are same. Given that 'btf_data'
> > resides in the user space, a malicious user can race to modify the header
> > between these two copies. By doing so, the user can inject inconsistent
> > data, which can cause undefined behavior of the kernel and introduce
> > potential security risk.
btw, I am working on a patch that copies the btf_data before parsing/verifying
the header. That should avoid this from happening but that will
require a bit more code churns for the bpf branch.
> >
> > To avoid the above issue, this patch copies the parsed header from
> > 'btf->hdr' to 'data'. The remaining part in 'data' is still copied from the
> > user-space 'btf_data'.
> LGTM.
>
> Acked-by: Martin KaFai Lau <kafai@fb.com>
>
> >
> > Signed-off-by: Wenwen Wang <wang6495@umn.edu>
> > ---
> > kernel/bpf/btf.c | 11 ++++++++++-
> > 1 file changed, 10 insertions(+), 1 deletion(-)
> >
> > diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> > index 378cef7..b52a834a 100644
> > --- a/kernel/bpf/btf.c
> > +++ b/kernel/bpf/btf.c
> > @@ -2152,6 +2152,7 @@ static struct btf *btf_parse(void __user *btf_data, u32 btf_data_size,
> > struct btf_verifier_env *env = NULL;
> > struct bpf_verifier_log *log;
> > struct btf *btf = NULL;
> > + u32 hdr_len;
> > u8 *data;
> > int err;
> >
> > @@ -2200,7 +2201,15 @@ static struct btf *btf_parse(void __user *btf_data, u32 btf_data_size,
> > btf->data_size = btf_data_size;
> > btf->nohdr_data = btf->data + btf->hdr.hdr_len;
> >
> > - if (copy_from_user(data, btf_data, btf_data_size)) {
> > + /*
> > + * The header at btf_data could be modified by a malicious user
> > + * after it is parsed. So we copy the parsed header here. The
> > + * remaining part is still copied from btf_data.
> > + */
> > + hdr_len = min_t(u32, btf->hdr.hdr_len, sizeof(btf->hdr));
> > + memcpy(data, &btf->hdr, hdr_len);
> > + if (copy_from_user(data + hdr_len, (u8 __user *)btf_data + hdr_len,
> > + btf_data_size - hdr_len)) {
> > err = -EFAULT;
> > goto errout;
> > }
> > --
> > 2.7.4
> >
^ permalink raw reply
* Re: [PATCH net-next] igc: Remove set but not used variable 'pci_using_dac'
From: Neftin, Sasha @ 2018-10-24 9:55 UTC (permalink / raw)
To: YueHaibing, Jeff Kirsher, David S. Miller
Cc: intel-wired-lan, netdev, kernel-janitors, Neftin, Sasha
In-Reply-To: <1539953291-189201-1-git-send-email-yuehaibing@huawei.com>
On 10/19/2018 15:48, YueHaibing wrote:
> Fixes gcc '-Wunused-but-set-variable' warning:
>
> drivers/net/ethernet/intel/igc/igc_main.c: In function 'igc_probe':
> drivers/net/ethernet/intel/igc/igc_main.c:3535:11: warning:
> variable 'pci_using_dac' set but not used [-Wunused-but-set-variable]
>
> It never used since introduction in commit
> d89f88419f99 ("igc: Add skeletal frame for Intel(R) 2.5G Ethernet Controller support")
>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> ---
> drivers/net/ethernet/intel/igc/igc_main.c | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
> index 9d85707..06a4afbe 100644
> --- a/drivers/net/ethernet/intel/igc/igc_main.c
> +++ b/drivers/net/ethernet/intel/igc/igc_main.c
> @@ -3532,19 +3532,16 @@ static int igc_probe(struct pci_dev *pdev,
> struct net_device *netdev;
> struct igc_hw *hw;
> const struct igc_info *ei = igc_info_tbl[ent->driver_data];
> - int err, pci_using_dac;
> + int err;
>
> err = pci_enable_device_mem(pdev);
> if (err)
> return err;
>
> - pci_using_dac = 0;
> err = dma_set_mask(&pdev->dev, DMA_BIT_MASK(64));
> if (!err) {
> err = dma_set_coherent_mask(&pdev->dev,
> DMA_BIT_MASK(64));
> - if (!err)
> - pci_using_dac = 1;
> } else {
> err = dma_set_mask(&pdev->dev, DMA_BIT_MASK(32));
> if (err) {
>
Thanks for the patch. Good.
Acked-by: Sasha Neftin <sasha.neftin@intel.com>
^ permalink raw reply
* [net-next][PATCH] net/ipv4: fix a net leak
From: Li RongQing @ 2018-10-24 9:36 UTC (permalink / raw)
To: netdev, dsahern
put net when input a invalid ifindex, otherwise it will be leaked
Fixes: 5fcd266a9f64("net/ipv4: Add support for dumping addresses for a specific device")
Cc: David Ahern <dsahern@gmail.com>
Signed-off-by: Zhang Yu <zhangyu31@baidu.com>
Signed-off-by: Li RongQing <lirongqing@baidu.com>
---
net/ipv4/devinet.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index 63d5b58fbfdb..fd0c5a47e742 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -1775,8 +1775,10 @@ static int inet_dump_ifaddr(struct sk_buff *skb, struct netlink_callback *cb)
if (fillargs.ifindex) {
dev = __dev_get_by_index(tgt_net, fillargs.ifindex);
- if (!dev)
+ if (!dev) {
+ put_net(tgt_net);
return -ENODEV;
+ }
in_dev = __in_dev_get_rtnl(dev);
if (in_dev) {
--
2.16.2
^ permalink raw reply related
* Re: [PATCH] bpf: btf: Fix a missing-check bug
From: Daniel Borkmann @ 2018-10-24 9:16 UTC (permalink / raw)
To: Y Song, wang6495; +Cc: kjlu, Alexei Starovoitov, netdev, LKML
In-Reply-To: <CAH3MdRV2qLj_3KHFYmCKYHMx2wZk_4mQrdvRJe+znXr3Astm8g@mail.gmail.com>
Hi Wenwen,
On 10/22/2018 05:57 PM, Y Song wrote:
> On Fri, Oct 19, 2018 at 3:30 PM Wenwen Wang <wang6495@umn.edu> wrote:
>>
>> In btf_parse(), the header of the user-space btf data 'btf_data' is firstly
>> parsed and verified through btf_parse_hdr(). In btf_parse_hdr(), the header
>> is copied from user-space 'btf_data' to kernel-space 'btf->hdr' and then
>> verified. If no error happens during the verification process, the whole
>> data of 'btf_data', including the header, is then copied to 'data' in
>> btf_parse(). It is obvious that the header is copied twice here. More
>> importantly, no check is enforced after the second copy to make sure the
>> headers obtained in these two copies are same. Given that 'btf_data'
>> resides in the user space, a malicious user can race to modify the header
>> between these two copies. By doing so, the user can inject inconsistent
>> data, which can cause undefined behavior of the kernel and introduce
>> potential security risk.
>>
>> To avoid the above issue, this patch rewrites the header after the second
>> copy, using 'btf->hdr', which is obtained in the first copy.
>>
>> Signed-off-by: Wenwen Wang <wang6495@umn.edu>
>> ---
>> kernel/bpf/btf.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
>> index 138f030..2a85f91 100644
>> --- a/kernel/bpf/btf.c
>> +++ b/kernel/bpf/btf.c
>> @@ -2202,6 +2202,9 @@ static struct btf *btf_parse(void __user *btf_data, u32 btf_data_size,
>> goto errout;
>> }
>>
>> + memcpy(data, &btf->hdr,
>> + min_t(u32, btf->hdr.hdr_len, sizeof(btf->hdr)));
>
> Could you restructure the code to memcpy the header followed by copying
> the rest of btf_data with copy_from_user? This way, each byte is only
> copied once.
> Could you add some comments right before memcpy so later people will know
> why we implement this way?
Thanks for the fix! Agree with Yonghong that we should rework this a bit, so
please respin a v2 with the feedback addressed, thanks.
>> +
>> err = btf_parse_str_sec(env);
>> if (err)
>> goto errout;
>> --
>> 2.7.4
>>
^ permalink raw reply
* Re: [PATCH bpf] bpf: devmap: fix wrong interface selection in notifier_call
From: Daniel Borkmann @ 2018-10-24 9:07 UTC (permalink / raw)
To: Taehee Yoo, ast; +Cc: netdev, john.fastabend
In-Reply-To: <20181022154143.15396-1-ap420073@gmail.com>
[ +John ]
On 10/22/2018 05:41 PM, Taehee Yoo wrote:
> The dev_map_notification() removes interface in devmap if
> unregistering interface's ifindex is same.
> But only checking ifindex is not enough because other netns can have
> same ifindex. so that wrong interface selection could occurred.
> Hence the net_eq() is needed.
>
> Fixes: 2ddf71e23cc2 ("net: add notifier hooks for devmap bpf map")
> Signed-off-by: Taehee Yoo <ap420073@gmail.com>
> ---
> kernel/bpf/devmap.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c
> index 141710b82a6c..0d9211e49a4a 100644
> --- a/kernel/bpf/devmap.c
> +++ b/kernel/bpf/devmap.c
> @@ -496,6 +496,7 @@ static int dev_map_notification(struct notifier_block *notifier,
> ulong event, void *ptr)
> {
> struct net_device *netdev = netdev_notifier_info_to_dev(ptr);
> + struct net *net = dev_net(netdev);
> struct bpf_dtab *dtab;
> int i;
>
> @@ -512,7 +513,7 @@ static int dev_map_notification(struct notifier_block *notifier,
> struct bpf_dtab_netdev *dev, *odev;
>
> dev = READ_ONCE(dtab->netdev_map[i]);
> - if (!dev ||
> + if (!dev || !net_eq(net, dev_net(dev->dev)) ||
> dev->dev->ifindex != netdev->ifindex)
Can't we just compare netdev pointers directly here?
> continue;
> odev = cmpxchg(&dtab->netdev_map[i], dev, NULL);
>
^ permalink raw reply
* Re: [PATCH] Change judgment len position
From: kbuild test robot @ 2018-10-24 17:34 UTC (permalink / raw)
To: Wang Hai
Cc: kbuild-all, edumazet, davem, kuznet, yoshfuji, netdev,
linux-kernel, Wang Hai
In-Reply-To: <20181024154729.5312-1-wanghaifine@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2276 bytes --]
Hi Wang,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on v4.19]
[also build test WARNING on next-20181019]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Wang-Hai/Change-judgment-len-position/20181025-010812
config: i386-randconfig-x002-201842 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All warnings (new ones prefixed by >>):
In file included from include/linux/compiler_types.h:64:0,
from <command-line>:0:
net/ipv4/tcp.c: In function 'do_tcp_getsockopt':
>> include/linux/compiler-gcc.h:82:45: warning: 'len' is used uninitialized in this function [-Wuninitialized]
#define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
^~~~~~~~~~~~
net/ipv4/tcp.c:3302:11: note: 'len' was declared here
int val, len;
^~~
--
In file included from include/linux/compiler_types.h:64:0,
from <command-line>:0:
net//ipv4/tcp.c: In function 'do_tcp_getsockopt':
>> include/linux/compiler-gcc.h:82:45: warning: 'len' is used uninitialized in this function [-Wuninitialized]
#define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
^~~~~~~~~~~~
net//ipv4/tcp.c:3302:11: note: 'len' was declared here
int val, len;
^~~
vim +/len +82 include/linux/compiler-gcc.h
87358710 David Woodhouse 2018-02-19 81
cb984d10 Joe Perches 2015-06-25 @82 #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
cb984d10 Joe Perches 2015-06-25 83
:::::: The code at line 82 was first introduced by commit
:::::: cb984d101b30eb7478d32df56a0023e4603cba7f compiler-gcc: integrate the various compiler-gcc[345].h files
:::::: TO: Joe Perches <joe@perches.com>
:::::: CC: Linus Torvalds <torvalds@linux-foundation.org>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 26475 bytes --]
^ permalink raw reply
* [RFC] [PATCH] netfilter: Fix kmemleak false positive reports
From: zhe.he @ 2018-10-24 17:29 UTC (permalink / raw)
To: pablo, kadlec, fw, davem, netfilter-devel, coreteam, netdev,
linux-kernel, catalin.marinas, linux-mm, zhe.he
From: He Zhe <zhe.he@windriver.com>
unreferenced object 0xffff9643edb89900 (size 256):
comm "sd-resolve", pid 220, jiffies 4295016710 (age 208.256s)
hex dump (first 32 bytes):
01 00 00 00 00 00 00 00 03 00 74 f3 ba b1 b6 b5 ..........t.....
65 3e 00 00 00 00 00 00 90 f9 a0 ed 43 96 ff ff e>..........C...
backtrace:
[<0000000070d5b185>] kmem_cache_alloc+0x146/0x200
[<0000000007a27faa>] __nf_conntrack_alloc.isra.13+0x4d/0x170 [nf_conntrack]
[<00000000ecc5b0ec>] init_conntrack+0x6a/0x2f0 [nf_conntrack]
[<000000003d38809f>] nf_conntrack_in+0x2c5/0x360 [nf_conntrack]
[<000000001fe154e3>] ipv4_conntrack_local+0x5d/0x70 [nf_conntrack_ipv4]
[<0000000027adadb2>] nf_hook_slow+0x48/0xd0
[<000000009893511f>] __ip_local_out+0xbd/0xf0
[<00000000d68cbd2f>] ip_local_out+0x1c/0x50
[<00000000995e2f37>] ip_send_skb+0x19/0x40
[<000000003d95f220>] udp_send_skb.isra.5+0x157/0x360
[<00000000ebc25968>] udp_sendmsg+0x9d8/0xc10
[<000000003bef56ec>] inet_sendmsg+0x3e/0xf0
[<000000008d23e405>] sock_sendmsg+0x1d/0x30
[<000000008c297097>] ___sys_sendmsg+0x108/0x2b0
[<00000000f15a806c>] __sys_sendmmsg+0xba/0x1c0
[<00000000e195d2cf>] __x64_sys_sendmmsg+0x24/0x30
In __nf_conntrack_confirm, object ct can be referenced to by the stack variable
ct and the members of ct->tuplehash. kmemleak needs at least one of them to find
the ct object during scan.
When the ct object is moved from the unconfirmed hlist to the confirmed hlist.
kmemleak cannot see ct object if things happen in the following order and thus
give the above false positive report.
1) The ct object is removed from the unconfirmed hlist.
2) kmemleak scans data/bss sections(heap scan passes without heap reference).
3) The ct object is added to confirmed hlist and the variable ct is destroyed as
the function returns.
4) kmemleak scans task stacks(stack scan passes without stack reference).
This patch marks ct object as not a leak.
Signed-off-by: He Zhe <zhe.he@windriver.com>
Cc: pablo@netfilter.org
Cc: kadlec@blackhole.kfki.hu
Cc: fw@strlen.de
Cc: davem@davemloft.net
Cc: catalin.marinas@arm.com
---
So far this is only observed in v4.18, not in v4.19. But the case seems apply
to both.
net/netfilter/nf_conntrack_core.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index a676d5f..067365d 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -35,6 +35,7 @@
#include <linux/mm.h>
#include <linux/nsproxy.h>
#include <linux/rculist_nulls.h>
+#include <linux/kmemleak.h>
#include <net/netfilter/nf_conntrack.h>
#include <net/netfilter/nf_conntrack_l4proto.h>
@@ -1282,6 +1283,8 @@ __nf_conntrack_alloc(struct net *net,
if (ct == NULL)
goto out;
+ kmemleak_not_leak(ct);
+
spin_lock_init(&ct->lock);
ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple = *orig;
ct->tuplehash[IP_CT_DIR_ORIGINAL].hnnode.pprev = NULL;
--
2.7.4
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox