* Compliment of the day to you Dear Friend.
From: Mrs. Amina Kadi @ 2018-09-13 13:22 UTC (permalink / raw)
Compliment of the day to you Dear Friend.
Dear Friend.
I am Mrs. Amina Kadi. am sending this brief letter to solicit your
partnership to transfer $5.5 million US Dollars. I shall send you
more information and procedures when I receive positive response from
you.
Mrs. Amina Kadi
^ permalink raw reply
* [PATCH v3] neighbour: confirm neigh entries when ARP packet is received
From: Vasily Khoruzhick @ 2018-09-13 18:12 UTC (permalink / raw)
To: David S. Miller, Roopa Prabhu, Alexey Dobriyan, Eric Dumazet,
Stephen Hemminger, Jim Westfall, Wolfgang Bumiller,
Vasily Khoruzhick, Kees Cook, Ihar Hrachyshka, netdev,
linux-kernel, Sergei Shtylyov
Cc: Vasily Khoruzhick
Update 'confirmed' timestamp when ARP packet is received. It shouldn't
affect locktime logic and anyway entry can be confirmed by any higher-layer
protocol. Thus it makes sense to confirm it when ARP packet is received.
Fixes: 77d7123342 ("neighbour: update neigh timestamps iff update is effective")
Signed-off-by: Vasily Khoruzhick <vasilykh@arista.com>
---
v2: - update comment to match new code.
v3: - fix wording in commit message, make 'Fixes' tag one line, remove extra new line
between 'Fixes' and 'Signed-off-by'.
net/core/neighbour.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index aa19d86937af..91592fceeaad 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -1180,6 +1180,12 @@ int neigh_update(struct neighbour *neigh, const u8 *lladdr, u8 new,
lladdr = neigh->ha;
}
+ /* Update confirmed timestamp for neighbour entry after we
+ * received ARP packet even if it doesn't change IP to MAC binding.
+ */
+ if (new & NUD_CONNECTED)
+ neigh->confirmed = jiffies;
+
/* If entry was valid and address is not changed,
do not change entry state, if new one is STALE.
*/
@@ -1201,15 +1207,12 @@ int neigh_update(struct neighbour *neigh, const u8 *lladdr, u8 new,
}
}
- /* Update timestamps only once we know we will make a change to the
+ /* Update timestamp only once we know we will make a change to the
* neighbour entry. Otherwise we risk to move the locktime window with
* noop updates and ignore relevant ARP updates.
*/
- if (new != old || lladdr != neigh->ha) {
- if (new & NUD_CONNECTED)
- neigh->confirmed = jiffies;
+ if (new != old || lladdr != neigh->ha)
neigh->updated = jiffies;
- }
if (new != old) {
neigh_del_timer(neigh);
--
2.18.0
^ permalink raw reply related
* Re: [PATCH net-next V2 00/11] vhost_net TX batching
From: David Miller @ 2018-09-13 18:05 UTC (permalink / raw)
To: mst; +Cc: netdev, linux-kernel, kvm, virtualization
In-Reply-To: <20180913135833-mutt-send-email-mst@kernel.org>
From: "Michael S. Tsirkin" <mst@redhat.com>
Date: Thu, 13 Sep 2018 14:02:13 -0400
> On Thu, Sep 13, 2018 at 09:28:19AM -0700, David Miller wrote:
>> From: Jason Wang <jasowang@redhat.com>
>> Date: Wed, 12 Sep 2018 11:16:58 +0800
>>
>> > This series tries to batch submitting packets to underlayer socket
>> > through msg_control during sendmsg(). This is done by:
>> ...
>>
>> Series applied, thanks Jason.
>
> Going over it now I don't see a lot to complain about, but I'd
> appreciate a bit more time for review in the future. 3 days ok?
I try to handle every patchwork entry within 48 hours, more
specifically 2 business days.
Given the rate at which patches flow through networking, I think
this is both reasonable and necessary.
One always have the option to send a quick reply to the list saying
"Please wait for my review, I'll get to it in the next day or two."
which I will certainly honor.
Thank you.
^ permalink raw reply
* Re: [PATCH net-next V2 11/11] vhost_net: batch submitting XDP buffers to underlayer sockets
From: Michael S. Tsirkin @ 2018-09-13 18:04 UTC (permalink / raw)
To: Jason Wang; +Cc: netdev, linux-kernel, kvm, virtualization
In-Reply-To: <20180912031709.14112-12-jasowang@redhat.com>
On Wed, Sep 12, 2018 at 11:17:09AM +0800, Jason Wang wrote:
> +static void vhost_tx_batch(struct vhost_net *net,
> + struct vhost_net_virtqueue *nvq,
> + struct socket *sock,
> + struct msghdr *msghdr)
> +{
> + struct tun_msg_ctl ctl = {
> + .type = TUN_MSG_PTR,
> + .num = nvq->batched_xdp,
> + .ptr = nvq->xdp,
> + };
> + int err;
> +
> + if (nvq->batched_xdp == 0)
> + goto signal_used;
> +
> + msghdr->msg_control = &ctl;
> + err = sock->ops->sendmsg(sock, msghdr, 0);
> + if (unlikely(err < 0)) {
> + vq_err(&nvq->vq, "Fail to batch sending packets\n");
> + return;
> + }
> +
> +signal_used:
> + vhost_net_signal_used(nvq);
> + nvq->batched_xdp = 0;
> +}
> +
Given it's all tun-specific now, how about just exporting tap_sendmsg
and calling that? Will get rid of some indirection too.
--
MST
^ permalink raw reply
* Re: [PATCH net-next V2 00/11] vhost_net TX batching
From: Michael S. Tsirkin @ 2018-09-13 18:02 UTC (permalink / raw)
To: David Miller; +Cc: jasowang, netdev, linux-kernel, kvm, virtualization
In-Reply-To: <20180913.092819.2099750169529223026.davem@davemloft.net>
On Thu, Sep 13, 2018 at 09:28:19AM -0700, David Miller wrote:
> From: Jason Wang <jasowang@redhat.com>
> Date: Wed, 12 Sep 2018 11:16:58 +0800
>
> > This series tries to batch submitting packets to underlayer socket
> > through msg_control during sendmsg(). This is done by:
> ...
>
> Series applied, thanks Jason.
Going over it now I don't see a lot to complain about, but I'd
appreciate a bit more time for review in the future. 3 days ok?
--
MST
^ permalink raw reply
* Re: [RFC PATCH iproute2-next] man: Add devlink health man page
From: Eran Ben Elisha @ 2018-09-13 12:49 UTC (permalink / raw)
To: Andrew Lunn
Cc: netdev, Jiri Pirko, Andy Gospodarek, Michael Chan, Jakub Kicinski,
Simon Horman, Alexander Duyck, Florian Fainelli, Tal Alon,
Ariel Almog
In-Reply-To: <20180913120815.GB11702@lunn.ch>
On 9/13/2018 3:08 PM, Andrew Lunn wrote:
>> devlink health sensor set pci/0000:01:00.0 name TX_COMP_ERROR action reset off action dump on
>> Sets TX_COMP_ERROR sensor parameters for a specific device.
>
> I hope the real sensors have more understandable names. If i remember
> correctly, the same sort of comment was given for resource
> management. It was pretty unclear what the resource names actually
> mean. Is an average user going to have any idea how to actually use
> these sensors and actions?
well, hopefully. the whole point is to have it fully controlled by the
user. However, names for the command should be short. I guess we shall
have it documented (challenge is to fit to multi vendors).
>
> Can you give more examples of sensors. We should understand if there
> are any overlaps with hwmon.
I restate here that we shall have SW sensors as well, and not only HW
sensors.
This is what I had in mind:
1. command interface error
2. command interface timeout
3. stuck TX queue (like tx_timeout)
4. stuck TX completion queue (driver did not process packets in a
reasonable time period)
5. stuck RX queue
6. RX completion error
7. TX completion error
8. HW / FW catastrophic error report
9. completion queue overrun
Eran
>
> Andrew
>
^ permalink raw reply
* Re: [PATCH v2,net-next 1/2] ip_gre: fix parsing gre header in ipgre_err
From: David Miller @ 2018-09-13 17:58 UTC (permalink / raw)
To: yanhaishuang; +Cc: kuznet, jbenc, netdev, linux-kernel
In-Reply-To: <1536744082-3568-1-git-send-email-yanhaishuang@cmss.chinamobile.com>
From: Haishuang Yan <yanhaishuang@cmss.chinamobile.com>
Date: Wed, 12 Sep 2018 17:21:21 +0800
> @@ -86,7 +86,7 @@ int gre_parse_header(struct sk_buff *skb, struct tnl_ptk_info *tpi,
>
> options = (__be32 *)(greh + 1);
> if (greh->flags & GRE_CSUM) {
> - if (skb_checksum_simple_validate(skb)) {
> + if (csum_err && skb_checksum_simple_validate(skb)) {
> *csum_err = true;
> return -EINVAL;
> }
You want to ignore csum errors, but you do not want to elide the side
effects of the skb_checksum_simple_validate() call which are to set
skb->csum_valid and skb->csum.
Therefore, the skb_checksum_simple_validate() call still needs to be
performed. We just wont return -EINVAL in the NULL csum_err case.
^ permalink raw reply
* Re: [PATCH 2/2] net: qcom/emac: add shared mdio bus support
From: Andrew Lunn @ 2018-09-13 12:42 UTC (permalink / raw)
To: Wang Dongsheng; +Cc: timur, davem, yu.zheng, netdev
In-Reply-To: <1536829493-10088-3-git-send-email-dongsheng.wang@hxt-semitech.com>
On Thu, Sep 13, 2018 at 05:04:53PM +0800, Wang Dongsheng wrote:
> Share the mii_bus for others MAC device because QDF2400 emac
> include MDIO, and the motherboard has more than one PHY connected
> to an MDIO bus.
>
> Tested: QDF2400 (ACPI), buildin/insmod/rmmod
>
> Signed-off-by: Wang Dongsheng <dongsheng.wang@hxt-semitech.com>
Hi Wang
This is a pretty big patch, and is hard to review. Could you try to
break it up into a number of smaller patches. You could for example
first refactor emacs_phy_config(), without making any functional
changes. Then add the sharing. Maybe do OF an ACPI in different
patches?
Thanks
Andrew
^ permalink raw reply
* [PATCH v2] socket: fix struct ifreq size in compat ioctl
From: Johannes Berg @ 2018-09-13 12:40 UTC (permalink / raw)
To: netdev; +Cc: Robert O'Callahan, Al Viro, Johannes Berg
From: Johannes Berg <johannes.berg@intel.com>
As reported by Reobert O'Callahan, since Viro's commit to kill
dev_ifsioc() we attempt to copy too much data in compat mode,
which may lead to EFAULT when the 32-bit version of struct ifreq
sits at/near the end of a page boundary, and the next page isn't
mapped.
Fix this by passing the approprate compat/non-compat size to copy
and using that, as before the dev_ifsioc() removal. This works
because only the embedded "struct ifmap" has different size, and
this is only used in SIOCGIFMAP/SIOCSIFMAP which has a different
handler. All other parts of the union are naturally compatible.
This fixes https://bugzilla.kernel.org/show_bug.cgi?id=199469.
Fixes: bf4405737f9f ("kill dev_ifsioc()")
Reported-by: Robert O'Callahan <robert@ocallahan.org>
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
net/socket.c | 22 ++++++++++++++--------
1 file changed, 14 insertions(+), 8 deletions(-)
diff --git a/net/socket.c b/net/socket.c
index e6945e318f02..01f3f8f32d6f 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -941,7 +941,8 @@ void dlci_ioctl_set(int (*hook) (unsigned int, void __user *))
EXPORT_SYMBOL(dlci_ioctl_set);
static long sock_do_ioctl(struct net *net, struct socket *sock,
- unsigned int cmd, unsigned long arg)
+ unsigned int cmd, unsigned long arg,
+ unsigned int ifreq_size)
{
int err;
void __user *argp = (void __user *)arg;
@@ -967,11 +968,11 @@ static long sock_do_ioctl(struct net *net, struct socket *sock,
} else {
struct ifreq ifr;
bool need_copyout;
- if (copy_from_user(&ifr, argp, sizeof(struct ifreq)))
+ if (copy_from_user(&ifr, argp, ifreq_size))
return -EFAULT;
err = dev_ioctl(net, cmd, &ifr, &need_copyout);
if (!err && need_copyout)
- if (copy_to_user(argp, &ifr, sizeof(struct ifreq)))
+ if (copy_to_user(argp, &ifr, ifreq_size))
return -EFAULT;
}
return err;
@@ -1070,7 +1071,8 @@ static long sock_ioctl(struct file *file, unsigned cmd, unsigned long arg)
err = open_related_ns(&net->ns, get_net_ns);
break;
default:
- err = sock_do_ioctl(net, sock, cmd, arg);
+ err = sock_do_ioctl(net, sock, cmd, arg,
+ sizeof(struct ifreq));
break;
}
return err;
@@ -2750,7 +2752,8 @@ static int do_siocgstamp(struct net *net, struct socket *sock,
int err;
set_fs(KERNEL_DS);
- err = sock_do_ioctl(net, sock, cmd, (unsigned long)&ktv);
+ err = sock_do_ioctl(net, sock, cmd, (unsigned long)&ktv,
+ sizeof(struct compat_ifreq));
set_fs(old_fs);
if (!err)
err = compat_put_timeval(&ktv, up);
@@ -2766,7 +2769,8 @@ static int do_siocgstampns(struct net *net, struct socket *sock,
int err;
set_fs(KERNEL_DS);
- err = sock_do_ioctl(net, sock, cmd, (unsigned long)&kts);
+ err = sock_do_ioctl(net, sock, cmd, (unsigned long)&kts,
+ sizeof(struct compat_ifreq));
set_fs(old_fs);
if (!err)
err = compat_put_timespec(&kts, up);
@@ -3072,7 +3076,8 @@ static int routing_ioctl(struct net *net, struct socket *sock,
}
set_fs(KERNEL_DS);
- ret = sock_do_ioctl(net, sock, cmd, (unsigned long) r);
+ ret = sock_do_ioctl(net, sock, cmd, (unsigned long) r,
+ sizeof(struct compat_ifreq));
set_fs(old_fs);
out:
@@ -3185,7 +3190,8 @@ static int compat_sock_ioctl_trans(struct file *file, struct socket *sock,
case SIOCBONDSETHWADDR:
case SIOCBONDCHANGEACTIVE:
case SIOCGIFNAME:
- return sock_do_ioctl(net, sock, cmd, arg);
+ return sock_do_ioctl(net, sock, cmd, arg,
+ sizeof(struct compat_ifreq));
}
return -ENOIOCTLCMD;
--
2.14.4
^ permalink raw reply related
* Re: [PATCH 1/2] r8169: Align ASPM/CLKREQ setting function with vendor driver
From: David Miller @ 2018-09-13 17:48 UTC (permalink / raw)
To: kai.heng.feng; +Cc: nic_swsd, hkallweit1, netdev, linux-kernel
In-Reply-To: <20180912065821.7198-1-kai.heng.feng@canonical.com>
From: Kai-Heng Feng <kai.heng.feng@canonical.com>
Date: Wed, 12 Sep 2018 14:58:20 +0800
> There's a small delay after setting ASPM in vendor drivers, r8101 and
> r8168.
> In addition, those drivers enable ASPM before ClkReq, also change that
> to align with vendor driver.
>
> I haven't seen anything bad becasue of this, but I think it's better to
> keep in sync with vendor driver.
>
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
I want Heiner Kallweit to review these before I apply them.
Thanks.
^ permalink raw reply
* Re: [PATCHv2] net: ipv4: Use BUG_ON directly instead of a if condition followed by BUG
From: David Miller @ 2018-09-13 17:42 UTC (permalink / raw)
To: zhongjiang; +Cc: edumazet, kuznet, netdev, linux-kernel
In-Reply-To: <1536671593-14746-1-git-send-email-zhongjiang@huawei.com>
From: zhong jiang <zhongjiang@huawei.com>
Date: Tue, 11 Sep 2018 21:13:13 +0800
> The if condition can be removed if we use BUG_ON directly.
> The issule is detected with the help of Coccinelle.
>
> Signed-off-by: zhong jiang <zhongjiang@huawei.com>
Taking Simon's feedback into consideration...
I very often see changes like this and have to check the implementation
of BUG_ON() et al. to make sure it evaluates it's arguments even when
debugging is disabled.
Even if it is always guaranteed to do so, like me people will be unsure
forever and have to check.
That makes auditing code and validating things more time consuming and
hard.
I also think the code as written now looks a lot nicer.
So I'm not applying this, sorry.
^ permalink raw reply
* Re: [PATCH net-next 1/3] net: rework SIOCGSTAMP ioctl handling
From: Arnd Bergmann @ 2018-09-13 12:28 UTC (permalink / raw)
To: Networking, David Miller
Cc: linux-arch, y2038 Mailman List, Eric Dumazet, Willem de Bruijn,
Linux Kernel Mailing List, linux-hams, Bluez mailing list,
linux-can, dccp, linux-wpan, linux-sctp, linux-x25
In-Reply-To: <20180829130308.3504560-1-arnd@arndb.de>
On Wed, Aug 29, 2018 at 3:03 PM Arnd Bergmann <arnd@arndb.de> wrote:
> diff --git a/net/core/sock.c b/net/core/sock.c
> index 3730eb855095..df17bbfaca27 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -2897,37 +2897,31 @@ bool lock_sock_fast(struct sock *sk)
> }
> EXPORT_SYMBOL(lock_sock_fast);
>
> -int sock_get_timestamp(struct sock *sk, struct timeval __user *userstamp)
> +int sock_gettstamp(struct socket *sock, void __user *userstamp,
> + bool timeval, bool time32)
> {
> - struct timeval tv;
> -
> - sock_enable_timestamp(sk, SOCK_TIMESTAMP);
> - tv = ktime_to_timeval(sk->sk_stamp);
> - if (tv.tv_sec == -1)
> - return -ENOENT;
> - if (tv.tv_sec == 0) {
> - sk->sk_stamp = ktime_get_real();
> - tv = ktime_to_timeval(sk->sk_stamp);
> - }
> - return copy_to_user(userstamp, &tv, sizeof(tv)) ? -EFAULT : 0;
> -}
> -EXPORT_SYMBOL(sock_get_timestamp);
As I just learned, sparc64 uses a 32-bit suseconds_t, so this
function always leaked 32 bits of kernel stack data by copying
the padding bytes of 'tv' into user space.
Linux-4.11 and higher could avoid that with
CONFIG_GCC_PLUGIN_STRUCTLEAK, but older kernels
have been affected since socket timestamps were first added.
The same thing is probably true of many other interfaces
that pass a timeval.
> -int sock_get_timestampns(struct sock *sk, struct timespec __user *userstamp)
> -{
> - struct timespec ts;
> + struct sock *sk = sock->sk;
> + struct timespec64 ts;
>
> sock_enable_timestamp(sk, SOCK_TIMESTAMP);
> - ts = ktime_to_timespec(sk->sk_stamp);
> + ts = ktime_to_timespec64(sk->sk_stamp);
> if (ts.tv_sec == -1)
> return -ENOENT;
> if (ts.tv_sec == 0) {
> sk->sk_stamp = ktime_get_real();
> - ts = ktime_to_timespec(sk->sk_stamp);
> + ts = ktime_to_timespec64(sk->sk_stamp);
> }
> - return copy_to_user(userstamp, &ts, sizeof(ts)) ? -EFAULT : 0;
> +
> + if (timeval)
> + ts.tv_nsec /= 1000;
> +#ifdef CONFIG_COMPAT_32BIT_TIME
> + if (time32)
> + return put_old_timespec32(&ts, userstamp);
> +#endif
> +
> + return put_timespec64(&ts, userstamp);
> }
My new implementation is worse here: it no longer leaks stack
data, but since we now write a big-endian 64-bit microseconds
value, the microseconds are in the wrong place and will
be interpreted as zero by user space...
I'll also have to revisit a few other similar patches I did for
y2038, to figure out what they should do on sparc64.
Arnd
^ permalink raw reply
* Re: [PATCH 0/2] net: qcom/emac: add shared mdio bus support
From: Andrew Lunn @ 2018-09-13 12:24 UTC (permalink / raw)
To: Wang Dongsheng; +Cc: timur, davem, yu.zheng, netdev, devicetree
In-Reply-To: <1536829493-10088-1-git-send-email-dongsheng.wang@hxt-semitech.com>
On Thu, Sep 13, 2018 at 05:04:51PM +0800, Wang Dongsheng wrote:
> Share the mii_bus for others MAC device because QDF2400 emac
> include MDIO, and the motherboard has more than one PHY connected
> to an MDIO bus.
>
> Tested: QDF2400 (ACPI), buildin/insmod/rmmod
>
> Wang Dongsheng (2):
> dt-bindings: net: qcom: Add binding for shared mdio bus
Hi Wang
Patch #1 did not seem to make it to netdev.
I probably have some comments on the binding...
Andrew
^ permalink raw reply
* Re: KMSAN: uninit-value in pppoe_rcv
From: Eric Dumazet @ 2018-09-13 17:31 UTC (permalink / raw)
To: Guillaume Nault, Alexander Potapenko
Cc: Eric Dumazet, syzbot+f5f6080811c849739212, LKML, mostrows,
Networking, syzkaller-bugs
In-Reply-To: <20180913172344.GB1507@alphalink.fr>
On 09/13/2018 10:23 AM, Guillaume Nault wrote:
> Nothing to change in tun.c. Just some more tests in pppoe.
> Can you try this patch? It only addresses this particular report, not
> the problems spotted by Eric.
>
> -------- 8< --------
> diff --git a/drivers/net/ppp/pppoe.c b/drivers/net/ppp/pppoe.c
> index 5aa59f41bf8c..77241b584dff 100644
> --- a/drivers/net/ppp/pppoe.c
> +++ b/drivers/net/ppp/pppoe.c
> @@ -429,6 +429,9 @@ static int pppoe_rcv(struct sk_buff *skb, struct net_device *dev,
> if (!skb)
> goto out;
>
> + if (skb_mac_header_len(skb) < ETH_HLEN)
> + goto drop;
> +
> if (!pskb_may_pull(skb, sizeof(struct pppoe_hdr)))
> goto drop;
>
>
Yeah this probably will help ;)
^ permalink raw reply
* Re: [PATCH] socket: fix struct ifreq size in compat ioctl
From: Johannes Berg @ 2018-09-13 12:16 UTC (permalink / raw)
To: Al Viro; +Cc: netdev, Robert O'Callahan
In-Reply-To: <20180913121353.GR19965@ZenIV.linux.org.uk>
On Thu, 2018-09-13 at 13:13 +0100, Al Viro wrote:
> On Thu, Sep 13, 2018 at 01:49:09PM +0200, Johannes Berg wrote:
> > From: Johannes Berg <johannes.berg@intel.com>
> >
> > As reported by Reobert O'Callahan, since Viro's commit to kill
> > dev_ifsioc() we attempt to copy too much data in compat mode,
> > which may lead to EFAULT when the 32-bit version of struct ifreq
> > sits at/near the end of a page boundary, and the next page isn't
> > mapped.
> >
> > Fix this by passing whether or not we're doing a compat call and
> > copying the appropriate size in/out, as we did before. This works
> > because only the embedded "struct ifmap" has different size, and
> > this is only used in SIOCGIFMAP/SIOCSIFMAP which has a different
> > handler. All other parts of the union are naturally compatible.
>
> Might be better to pass explicit size instead of this bool compat...
Yeah, that's a bit better perhaps, I should resend to have the bugzilla
link anyway.
johannes
^ permalink raw reply
* Re: [PATCH 2/2] netlink: add ethernet address policy types
From: Johannes Berg @ 2018-09-13 12:16 UTC (permalink / raw)
To: Michal Kubecek; +Cc: linux-wireless, netdev
In-Reply-To: <20180913121227.GH29691@unicorn.suse.cz>
On Thu, 2018-09-13 at 14:12 +0200, Michal Kubecek wrote:
> On Thu, Sep 13, 2018 at 02:02:53PM +0200, Johannes Berg wrote:
> > On Thu, 2018-09-13 at 13:58 +0200, Michal Kubecek wrote:
> >
> > > The code looks correct to me but I have some doubts. Having a special
> > > policy for MAC addresses may lead to adding one for IPv4 address (maybe
> > > not, we can use NLA_U32 for them), IPv6 addresses and other data types
> > > with fixed length. Wouldn't it be more helpful to add a variant of
> > > NLA_BINARY (NLA_BINARY_EXACT?) which would fail/warn if attribute length
> > > isn't equal to .len?
> >
> > Yeah, I guess we could do that, and then
> >
> > #define NLA_ETH_ADDR .len = ETH_ALEN, .type = NLA_BINARY_EXACT
> > #define NLA_IP6_ADDR .len = 16, .type = NLA_BINARY_EXACT
> >
> > or so?
>
> Maybe rather
>
> #define NLA_ETH_ADDR NLA_BINARY_EXACT, .len = ETH_ALEN
> #define NLA_IP6_ADDR NLA_BINARY_EXACT, .len = sizeof(struct in6_addr)
>
> so that one could write
>
> { .type = NLA_ETH_ADDR }
Yeah, that's possible. I considered it for a second, but it was slightly
too magical for my taste :-)
Better pick a different "namespace", perhaps NLA_POLICY_ETH_ADDR or so?
johannes
^ permalink raw reply
* Re: KMSAN: uninit-value in pppoe_rcv
From: Guillaume Nault @ 2018-09-13 17:23 UTC (permalink / raw)
To: Alexander Potapenko
Cc: Eric Dumazet, syzbot+f5f6080811c849739212, LKML, mostrows,
Networking, syzkaller-bugs
In-Reply-To: <20180913161929.GA1507@alphalink.fr>
On Thu, Sep 13, 2018 at 06:19:29PM +0200, Guillaume Nault wrote:
> On Thu, Sep 13, 2018 at 04:12:38PM +0200, Alexander Potapenko wrote:
> > On Thu, Sep 13, 2018 at 3:57 PM Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > >
> > >
> > >
> > > On 09/12/2018 03:38 AM, Alexander Potapenko wrote:
> > > > On Wed, Sep 12, 2018 at 12:24 PM syzbot
> > > > <syzbot+f5f6080811c849739212@syzkaller.appspotmail.com> wrote:
> > > >>
> > > >> Hello,
> > > >>
> > > >> syzbot found the following crash on:
> > > >>
> > > >> HEAD commit: d2d741e5d189 kmsan: add initialization for shmem pages
> > > >> git tree: https://github.com/google/kmsan.git/master
> > > >> console output: https://syzkaller.appspot.com/x/log.txt?x=1465fc37800000
> > > >> kernel config: https://syzkaller.appspot.com/x/.config?x=48f9de3384bcd0f
> > > >> dashboard link: https://syzkaller.appspot.com/bug?extid=f5f6080811c849739212
> > > >> compiler: clang version 7.0.0 (trunk 329391)
> > > >> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=14d6e607800000
> > > >> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=10a15b5b800000
> > > >>
> > > >> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> > > >> Reported-by: syzbot+f5f6080811c849739212@syzkaller.appspotmail.com
> > > >>
> > > >> IPVS: ftp: loaded support on port[0] = 21
> > > >> ==================================================================
> > > >> BUG: KMSAN: uninit-value in __get_item drivers/net/ppp/pppoe.c:172 [inline]
> > > >> BUG: KMSAN: uninit-value in get_item drivers/net/ppp/pppoe.c:236 [inline]
> > > >> BUG: KMSAN: uninit-value in pppoe_rcv+0xcef/0x10e0
> > > >> drivers/net/ppp/pppoe.c:450
> > > >> CPU: 0 PID: 4543 Comm: syz-executor355 Not tainted 4.16.0+ #87
> > > >> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> > > >> Google 01/01/2011
> > > >> Call Trace:
> > > >> __dump_stack lib/dump_stack.c:17 [inline]
> > > >> dump_stack+0x185/0x1d0 lib/dump_stack.c:53
> > > >> kmsan_report+0x142/0x240 mm/kmsan/kmsan.c:1067
> > > >> __msan_warning_32+0x6c/0xb0 mm/kmsan/kmsan_instr.c:683
> > > >> __get_item drivers/net/ppp/pppoe.c:172 [inline]
> > > >> get_item drivers/net/ppp/pppoe.c:236 [inline]
> > > >> pppoe_rcv+0xcef/0x10e0 drivers/net/ppp/pppoe.c:450
> > > >> __netif_receive_skb_core+0x47df/0x4a90 net/core/dev.c:4562
> > > >> __netif_receive_skb net/core/dev.c:4627 [inline]
> > > >> netif_receive_skb_internal+0x49d/0x630 net/core/dev.c:4701
> > > >> netif_receive_skb+0x230/0x240 net/core/dev.c:4725
> > > >> tun_rx_batched drivers/net/tun.c:1555 [inline]
> > > >> tun_get_user+0x740f/0x7c60 drivers/net/tun.c:1962
> > > >> tun_chr_write_iter+0x1d4/0x330 drivers/net/tun.c:1990
> > > >> call_write_iter include/linux/fs.h:1782 [inline]
> > > >> new_sync_write fs/read_write.c:469 [inline]
> > > >> __vfs_write+0x7fb/0x9f0 fs/read_write.c:482
> > > >> vfs_write+0x463/0x8d0 fs/read_write.c:544
> > > >> SYSC_write+0x172/0x360 fs/read_write.c:589
> > > >> SyS_write+0x55/0x80 fs/read_write.c:581
> > > >> do_syscall_64+0x309/0x430 arch/x86/entry/common.c:287
> > > >> entry_SYSCALL_64_after_hwframe+0x3d/0xa2
> > > >> RIP: 0033:0x4447c9
> > > >> RSP: 002b:00007fff64c8fc28 EFLAGS: 00000297 ORIG_RAX: 0000000000000001
> > > >> RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00000000004447c9
> > > >> RDX: 000000000000fd87 RSI: 0000000020000600 RDI: 0000000000000004
> > > >> RBP: 00000000006cf018 R08: 00007fff64c8fda8 R09: 00007fff00006bda
> > > >> R10: 0000000000005fe7 R11: 0000000000000297 R12: 00000000004020d0
> > > >> R13: 0000000000402160 R14: 0000000000000000 R15: 0000000000000000
> > > >>
> > > >> Uninit was created at:
> > > >> kmsan_save_stack_with_flags mm/kmsan/kmsan.c:278 [inline]
> > > >> kmsan_internal_poison_shadow+0xb8/0x1b0 mm/kmsan/kmsan.c:188
> > > >> kmsan_kmalloc+0x94/0x100 mm/kmsan/kmsan.c:314
> > > >> kmsan_slab_alloc+0x11/0x20 mm/kmsan/kmsan.c:321
> > > >> slab_post_alloc_hook mm/slab.h:445 [inline]
> > > >> slab_alloc_node mm/slub.c:2737 [inline]
> > > >> __kmalloc_node_track_caller+0xaed/0x11c0 mm/slub.c:4369
> > > >> __kmalloc_reserve net/core/skbuff.c:138 [inline]
> > > >> __alloc_skb+0x2cf/0x9f0 net/core/skbuff.c:206
> > > >> alloc_skb include/linux/skbuff.h:984 [inline]
> > > >> alloc_skb_with_frags+0x1d4/0xb20 net/core/skbuff.c:5234
> > > >> sock_alloc_send_pskb+0xb56/0x1190 net/core/sock.c:2085
> > > >> tun_alloc_skb drivers/net/tun.c:1532 [inline]
> > > >> tun_get_user+0x2242/0x7c60 drivers/net/tun.c:1829
> > > >> tun_chr_write_iter+0x1d4/0x330 drivers/net/tun.c:1990
> > > >> call_write_iter include/linux/fs.h:1782 [inline]
> > > >> new_sync_write fs/read_write.c:469 [inline]
> > > >> __vfs_write+0x7fb/0x9f0 fs/read_write.c:482
> > > >> vfs_write+0x463/0x8d0 fs/read_write.c:544
> > > >> SYSC_write+0x172/0x360 fs/read_write.c:589
> > > >> SyS_write+0x55/0x80 fs/read_write.c:581
> > > >> do_syscall_64+0x309/0x430 arch/x86/entry/common.c:287
> > > >> entry_SYSCALL_64_after_hwframe+0x3d/0xa2
> > > >> ==================================================================
> > > > I did a little digging before sending the bug upstream.
> > > > If I add memset(obj, 0xfe, size) to __kmalloc_reserve(), these 0xfe
> > > > bytes are visible in __get_item() at the place where KMSAN reports an
> > > > error.
> > > >
> > > > The problem is somehow related to tun_get_user() creating a fragmented
> > > > sk_buff - when I change the call to tun_alloc_skb() so that it
> > > > allocates a single buffer the bug goes away.
> > > >
> > >
> > > I guess the following patch would fix the issue
> > >
> > > (I will submit it more formally)
> > No, as far as I can see it doesn't.
> > Saving sid before __skb_pull() is still a good idea, but in this
> > particular case |ph| doesn't change.
>
> Yes, we probably need to save sid.
>
> But I think the problem found by syzbot is related to
> eth_hdr(skb)->h_source. PPPoE expects that Ethernet header has already
> been parsed and is accessible at skb_mac_header(skb).
> But here skb_mac_header(skb) == skb->data, and we may pull only 6 bytes
> (sizeof(truct pppoe_hdr)). Therefore eth_hdr(skb)->h_source points past
> skb's head length.
>
> Not sure if something needs to be changed in tun.c for properly setting
> skb_mac_header. But PPPoE has no reason to consider packets from
> non-Ethernet devices anyway.
Nothing to change in tun.c. Just some more tests in pppoe.
Can you try this patch? It only addresses this particular report, not
the problems spotted by Eric.
-------- 8< --------
diff --git a/drivers/net/ppp/pppoe.c b/drivers/net/ppp/pppoe.c
index 5aa59f41bf8c..77241b584dff 100644
--- a/drivers/net/ppp/pppoe.c
+++ b/drivers/net/ppp/pppoe.c
@@ -429,6 +429,9 @@ static int pppoe_rcv(struct sk_buff *skb, struct net_device *dev,
if (!skb)
goto out;
+ if (skb_mac_header_len(skb) < ETH_HLEN)
+ goto drop;
+
if (!pskb_may_pull(skb, sizeof(struct pppoe_hdr)))
goto drop;
^ permalink raw reply related
* Re: [PATCH] socket: fix struct ifreq size in compat ioctl
From: Al Viro @ 2018-09-13 12:13 UTC (permalink / raw)
To: Johannes Berg; +Cc: netdev, Robert O'Callahan, Johannes Berg
In-Reply-To: <20180913114909.8331-1-johannes@sipsolutions.net>
On Thu, Sep 13, 2018 at 01:49:09PM +0200, Johannes Berg wrote:
> From: Johannes Berg <johannes.berg@intel.com>
>
> As reported by Reobert O'Callahan, since Viro's commit to kill
> dev_ifsioc() we attempt to copy too much data in compat mode,
> which may lead to EFAULT when the 32-bit version of struct ifreq
> sits at/near the end of a page boundary, and the next page isn't
> mapped.
>
> Fix this by passing whether or not we're doing a compat call and
> copying the appropriate size in/out, as we did before. This works
> because only the embedded "struct ifmap" has different size, and
> this is only used in SIOCGIFMAP/SIOCSIFMAP which has a different
> handler. All other parts of the union are naturally compatible.
Might be better to pass explicit size instead of this bool compat...
^ permalink raw reply
* Re: Regression: kernel 4.14 an later very slow with many ipsec tunnels
From: David Miller @ 2018-09-13 17:23 UTC (permalink / raw)
To: fw; +Cc: linux, netdev, steffen.klassert, linux-kernel, torvalds
In-Reply-To: <20180913163848.ni5xc4gc4d6uusdn@breakpoint.cc>
From: Florian Westphal <fw@strlen.de>
Date: Thu, 13 Sep 2018 18:38:48 +0200
> Wolfgang Walter <linux@stwm.de> wrote:
>> What I can say is that it depends mainly on number of policy rules and SA.
>
> Thats already a good hint, I guess we're hitting long hash chains in
> xfrm_policy_lookup_bytype().
I don't really see how recent changes can influence that.
And the bydst hashes have been dynamically sized for a very long time.
I might have missed something...
^ permalink raw reply
* Re: [PATCH 2/2] netlink: add ethernet address policy types
From: Michal Kubecek @ 2018-09-13 12:12 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless, netdev
In-Reply-To: <1536840173.4160.4.camel@sipsolutions.net>
On Thu, Sep 13, 2018 at 02:02:53PM +0200, Johannes Berg wrote:
> On Thu, 2018-09-13 at 13:58 +0200, Michal Kubecek wrote:
>
> > The code looks correct to me but I have some doubts. Having a special
> > policy for MAC addresses may lead to adding one for IPv4 address (maybe
> > not, we can use NLA_U32 for them), IPv6 addresses and other data types
> > with fixed length. Wouldn't it be more helpful to add a variant of
> > NLA_BINARY (NLA_BINARY_EXACT?) which would fail/warn if attribute length
> > isn't equal to .len?
>
> Yeah, I guess we could do that, and then
>
> #define NLA_ETH_ADDR .len = ETH_ALEN, .type = NLA_BINARY_EXACT
> #define NLA_IP6_ADDR .len = 16, .type = NLA_BINARY_EXACT
>
> or so?
Maybe rather
#define NLA_ETH_ADDR NLA_BINARY_EXACT, .len = ETH_ALEN
#define NLA_IP6_ADDR NLA_BINARY_EXACT, .len = sizeof(struct in6_addr)
so that one could write
{ .type = NLA_ETH_ADDR }
as with other types.
Michal Kubecek
^ permalink raw reply
* Re: [PATCH net-next] net: dsa: b53: Do not fail when IRQ are not initialized
From: David Miller @ 2018-09-13 17:19 UTC (permalink / raw)
To: f.fainelli; +Cc: netdev, andrew, vivien.didelot, linux-kernel
In-Reply-To: <20180913165045.3802-1-f.fainelli@gmail.com>
From: Florian Fainelli <f.fainelli@gmail.com>
Date: Thu, 13 Sep 2018 09:50:45 -0700
> When the Device Tree is not providing the per-port interrupts, do not fail
> during b53_srab_irq_enable() but instead bail out gracefully. The SRAB driver
> is used on the BCM5301X (Northstar) platforms which do not yet have the SRAB
> interrupts wired up.
>
> Fixes: 16994374a6fc ("net: dsa: b53: Make SRAB driver manage port interrupts")
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Applied, thanks Florian.
^ permalink raw reply
* Re: [RFC PATCH iproute2-next] man: Add devlink health man page
From: Andrew Lunn @ 2018-09-13 12:08 UTC (permalink / raw)
To: Eran Ben Elisha
Cc: netdev, Jiri Pirko, Andy Gospodarek, Michael Chan, Jakub Kicinski,
Simon Horman, Alexander Duyck, Florian Fainelli, Tal Alon,
Ariel Almog
In-Reply-To: <1536826696-9413-2-git-send-email-eranbe@mellanox.com>
> devlink health sensor set pci/0000:01:00.0 name TX_COMP_ERROR action reset off action dump on
> Sets TX_COMP_ERROR sensor parameters for a specific device.
I hope the real sensors have more understandable names. If i remember
correctly, the same sort of comment was given for resource
management. It was pretty unclear what the resource names actually
mean. Is an average user going to have any idea how to actually use
these sensors and actions?
Can you give more examples of sensors. We should understand if there
are any overlaps with hwmon.
Andrew
^ permalink raw reply
* Re: [PATCH 1/2] netlink: add NLA_REJECT policy type
From: Michal Kubecek @ 2018-09-13 12:05 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless, netdev
In-Reply-To: <1536837915.4160.1.camel@sipsolutions.net>
On Thu, Sep 13, 2018 at 01:25:15PM +0200, Johannes Berg wrote:
> On Thu, 2018-09-13 at 12:49 +0200, Michal Kubecek wrote:
>
> > > if (type > 0 && type <= maxtype) {
> > > if (policy) {
> > > - err = validate_nla(nla, maxtype, policy);
> > > + err = validate_nla(nla, maxtype, policy,
> > > + extack);
> > > if (err < 0) {
> > > - NL_SET_ERR_MSG_ATTR(extack, nla,
> > > - "Attribute failed policy validation");
> > > + NL_SET_BAD_ATTR(extack, nla);
> > > + if (extack && !extack->_msg)
> > > + NL_SET_ERR_MSG(extack,
> > > + "Attribute failed policy validation");
> > > goto errout;
> > > }
> > > }
> > > --
> >
> > Technically, this would change the outcome when nla_parse() is called
> > with extack->_msg already set nad validate_nla() fails on something else
> > than NLA_REJECT; it will preserve the previous message in such case.
> > But I don't think this is a serious problem.
>
> Yes, that's true. I looked at quite a few of the setters just now (there
> are ~500 already, wow!), and they all set & return, so this shouldn't be
> an issue.
In ethtool (work in progress) I sometimes use extack message for
non-fatal warnings but AFAICS never before parsing the userspace
request (actually always shortly before returning). So I don't have
a problem with it.
Michal Kubecek
^ permalink raw reply
* Re: [PATCH 2/2] netlink: add ethernet address policy types
From: Johannes Berg @ 2018-09-13 12:02 UTC (permalink / raw)
To: Michal Kubecek; +Cc: linux-wireless, netdev
In-Reply-To: <20180913115849.GF29691@unicorn.suse.cz>
On Thu, 2018-09-13 at 13:58 +0200, Michal Kubecek wrote:
> The code looks correct to me but I have some doubts. Having a special
> policy for MAC addresses may lead to adding one for IPv4 address (maybe
> not, we can use NLA_U32 for them), IPv6 addresses and other data types
> with fixed length. Wouldn't it be more helpful to add a variant of
> NLA_BINARY (NLA_BINARY_EXACT?) which would fail/warn if attribute length
> isn't equal to .len?
Yeah, I guess we could do that, and then
#define NLA_ETH_ADDR .len = ETH_ALEN, .type = NLA_BINARY_EXACT
#define NLA_IP6_ADDR .len = 16, .type = NLA_BINARY_EXACT
or so?
johannes
^ permalink raw reply
* Re: [RFC PATCH iproute2-next] man: Add devlink health man page
From: Eran Ben Elisha @ 2018-09-13 11:58 UTC (permalink / raw)
To: Tobin C. Harding
Cc: netdev, Jiri Pirko, Andy Gospodarek, Michael Chan, Jakub Kicinski,
Simon Horman, Alexander Duyck, Andrew Lunn, Florian Fainelli,
Tal Alon, Ariel Almog
In-Reply-To: <20180913102732.GG11198@eros>
On 9/13/2018 1:27 PM, Tobin C. Harding wrote:
> On Thu, Sep 13, 2018 at 11:18:16AM +0300, Eran Ben Elisha wrote:
>> Add devlink-health man page. Devlink-health tool will control device
>> health attributes, sensors, actions and logging.
>>
>> Signed-off-by: Eran Ben Elisha <eranbe@mellanox.com>
>>
>> -------------------------------------------------------
>> Copy paste man output to here for easier review process of the RFC.
>>
>> DEVLINK-HEALTH(8) Linux DEVLINK-HEALTH(8)
>>
>> NAME
>> devlink-health - devlink health configuration
>>
>> SYNOPSIS
>> devlink [ OPTIONS ] health { COMMAND | help }
>>
>> OPTIONS := { -V[ersion] | -n[no-nice-names] }
>>
>> devlink health show [ DEV ] [ sensor NAME ]
>>
>> devlink health sensor set DEV name NAME [ action NAME { active | inactive } ]"
>>
>> devlink health action set DEV name NAME period PERIOD count COUNT fail { ignore | down }
>>
>> devlink health action reinit DEV name NAME
>>
>> devlink health help
>>
>> DESCRIPTION
>> devlink-health tool allows user to configure the way driver treats unexpected status. The tool allows configuration of the sensors that can trigger health activity. Set for each sensor the follow up operations, such as,
>> reset and dump of info. In addition, set the health activity termination action.
>>
>> devlink health show - Display devlink health sensors and actions attributes
>> DEV - Specifies the devlink device to show. If this argument is omitted, all devices are listed.
>>
>> Format is:
>> BUS_NAME/BUS_ADDRESS
>>
>> sensor NAME - Specifies the devlink sensor to show.
>>
>
> Perhaps the commands should include the optional arguments so when
> reading the description one doesn't have to scroll to the top of the
> page all the time
>
> e.g
> devlink health show [ DEV ] [ sensor NAME ] - Display devlink health sensors and actions attributes
>
I followed the scheme presented in all other devlink man pages.
see devlink-region, devlink-port, etc.
From my perspective, I am fine with adding it to devlink-health, need
ack from the devlink maintainer to see if he likes it...
>> devlink health sensor set - sets devlink health sensor attributes
>> DEV Specifies the devlink device to show.
>
> set
>
>> name NAME
>> Name of the sensor to set.
>>
>> action NAME { active | inactive }
>> Specify which actions to activate and which to deactivate once a sensor was triggered. actions can be dump, reset, etc.
>>
>> devlink health action set - sets devlink action attributes
>> DEV Specifies the devlink device to set.
>>
>> name NAME
>> Specifies the devlink action to set.
>
> This is a little unclear to me?
what is not clear? the term 'action' or the naming? can you elaborate?
>
>> period PERIOD
>> The period on which we limit the amount of performed actions, measured in seconds.
>>
>> count COUNT
>> The maximum amount of actions performed in a limit time frame.
>
> Perhaps
> The maximum number of actions performed in a limited time frame.
ack
>
>> fail { ignore | down }
>> Specify the behavior once count limit was reached.
>>
>> ignore - Ignore errors without execution of any action.
>>
>> down - Driver will remain in nonoperational state.
>>
>> devlink health action reinit - reset devlink action attributes (period, count, fail, etc)
>> DEV Specifies the devlink device to set.
>>
>> name NAME
>> Specifies the devlink action to set.
>
> Perhaps s/set/reinitialise/g for the above two descriptions.
ack
>
> Hope this helps,
> Tobin.
thanks
^ 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