Re: [PATCH bpf] bpf: Fix integer overflow in queue_stack_map_alloc.

[Greg KH]

On Thu, Nov 22, 2018 at 11:59:02PM +0800, Wei Wu wrote:
> Integer overflow in queue_stack_map_alloc when calculating size may
> lead to heap overflow of arbitrary length.
> The patch fix it by checking whether attr->max_entries+1 <
> attr->max_entries and bailing out if it is the case.
> The vulnerability is discovered with the assistance of syzkaller.
> 
> Reported-by: Wei Wu 
> To: Alexei Starovoitov 
> Cc: Daniel Borkmann 
> Cc: netdev 
> Cc: Eric Dumazet 
> Cc: Greg KH 
> Signed-off-by: Wei Wu 
> ---
>  kernel/bpf/queue_stack_maps.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/kernel/bpf/queue_stack_maps.c b/kernel/bpf/queue_stack_maps.c
> index 8bbd72d3a121..c35a8a4721c8 100644
> --- a/kernel/bpf/queue_stack_maps.c
> +++ b/kernel/bpf/queue_stack_maps.c
> @@ -67,6 +67,8 @@ static struct bpf_map *queue_stack_map_alloc(union
> bpf_attr *attr)
>   u64 queue_size, cost;
> 
>   size = attr->max_entries + 1;
> + if (size < attr->max_entries)
> + return ERR_PTR(-EINVAL);
>   value_size = attr->value_size;

Your tabs got eaten by your email client and they all disappeared,
making the patch impossible to apply :(

Care to try again?

thanks,

greg k-h

Re: Application of f8b39039cbf2a15f ("net: fs_enet: do not call phy_stop() in interrupts") to 4.9 and 4.14 stable

[Greg KH]

On Wed, Oct 17, 2018 at 02:08:41PM +0200, Christophe LEROY wrote:
> Hi,
> 
> Could you please apply f8b39039cbf2a15f2b8c9f081e1cbd5dee00aaf5 to 4.9 and
> 4.14 ?
> 
> It fixes an Oops when Ethernet transmission times out.
> 
> As you can observe in the commit log, the Oops what initially observed in
> 4.9.
> 
> The fix has been in mainline since 4.15

And it was released in 4.9.136 and 4.14.80 already :)

thanks,

greg k-h

Re: [PATCH stable 4.4 V2 0/6] fix SegmentSmack in stable branch (CVE-2018-5390)

[Greg KH]

On Wed, Sep 26, 2018 at 10:21:21PM +0200, Greg KH wrote:
> On Tue, Sep 25, 2018 at 10:10:15PM +0800, maowenan wrote:
> > Hi Greg:
> > 
> > can you review this patch set?
> 
> It is still in the queue, don't worry.  It will take some more time to
> properly review and test it.
> 
> Ideally you could get someone else to test this and provide a
> "tested-by:" tag for it?

All now queued up, let's see what breaks :)

thanks,

greg k-h

Re: [PATCH v3 00/30] backport of IP fragmentation fixes

[Greg KH]

On Thu, Sep 13, 2018 at 07:58:32AM -0700, Stephen Hemminger wrote:
> Took the set of patches from 4.19 to handle IP fragmentation DoS
> and applied them against 4.14.69.  Most of these are from Eric.
> In a couple case, it required some manual merge conflict resolution.
> 
> Tested normal IP fragmentation with iperf3 and malicious IP fragments
> with fragmentsmack. Under fragmentation attack (700Kpps) the original
> 4.14.69 consumes 97% CPU; with this patch it drops to 5%.

All now queued up, thanks for doing the backport.

greg k-h

Re: [PATCH stable 4.4 0/9] fix SegmentSmack in stable branch (CVE-2018-5390)

[Greg KH]

On Thu, Aug 16, 2018 at 05:24:09PM +0200, Greg KH wrote:
> On Thu, Aug 16, 2018 at 02:33:56PM +0200, Michal Kubecek wrote:
> > On Thu, Aug 16, 2018 at 08:05:50PM +0800, maowenan wrote:
> > > On 2018/8/16 19:39, Michal Kubecek wrote:
> > > > 
> > > > I suspect you may be doing something wrong with your tests. I checked
> > > > the segmentsmack testcase and the CPU utilization on receiving side
> > > > (with sending 10 times as many packets as default) went down from ~100%
> > > > to ~3% even when comparing what is in stable 4.4 now against older 4.4
> > > > kernel.
> > > 
> > > There seems no obvious problem when you send packets with default
> > > parameter in Segmentsmack POC, Which is also very related with your
> > > server's hardware configuration. Please try with below parameter to
> > > form OFO packets
> > 
> > I did and even with these (questionable, see below) changes, I did not
> > get more than 10% (of one core) by receiving ksoftirqd.
> > 
> > >   for (i = 0; i < 1024; i++)  // 128->1024
> > ...
> > >   usleep(10*1000); // Adjust this and packet count to match the 
> > > target!, sleep 100ms->10ms
> > 
> > The comment in the testcase source suggests to do _one_ of these two
> > changes so that you generate 10 times as many packets as the original
> > testcase. You did both so that you end up sending 102400 packets per
> > second. With 55 byte long packets, this kind of attack requires at least
> > 5.5 MB/s (44 Mb/s) of throughput. This is no longer a "low packet rate
> > DoS", I'm afraid.
> > 
> > Anyway, even at this rate, I only get ~10% of one core (Intel E5-2697).
> > 
> > What I can see, though, is that with current stable 4.4 code, modified
> > testcase which sends something like
> > 
> >   2:3, 3:4, ..., 3001:3002, 3003:3004, 3004:3005, ... 6001:6002, ...
> > 
> > I quickly eat 6 MB of memory for receive queue of one socket while
> > earlier 4.4 kernels only take 200-300 KB. I didn't test latest 4.4 with
> > Takashi's follow-up yet but I'm pretty sure it will help while
> > preserving nice performance when using the original segmentsmack
> > testcase (with increased packet ratio).
> 
> Ok, for now I've applied Takashi's fix to the 4.4 stable queue and will
> push out a new 4.4-rc later tonight.  Can everyone standardize on that
> and test and let me know if it does, or does not, fix the reported
> issues?
> 
> If not, we can go from there and evaluate this much larger patch series.
> But let's try the simple thing first.

So, is the issue still present on the latest 4.4 release?  Has anyone
tested it?  If not, I'm more than willing to look at backported patches,
but I want to ensure that they really are needed here.

thanks,

greg k-h

Re: [PATCH 0/2] staging: Fix some warnings from strncpy()

[Greg KH]

On Mon, Aug 20, 2018 at 12:51:22PM -0500, Larry Finger wrote:
> When the size argument in a call to strncpy() is the size of the
> destimation, gcc8 issues a warning. These patches fix the potential
> problem by replacing the strncpy() with strlcpy().
> 
> Signed-off-by: Larry Finger 
> 
> 
> Larry Finger (2):
>   staging: rtl8192e: Fix compiler warning about strncpy
>   staging: rtl8712u: Fix compiler warning about strncpy
> 
>  drivers/staging/rtl8192e/rtllib_softmac.c | 4 ++--
>  drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)

I'm guessing you will send a second set of this patch series, right?
This one is now dropped, thanks.

greg k-h

Re: [PATCH stable 4.4 0/9] fix SegmentSmack in stable branch (CVE-2018-5390)

[Greg KH]

On Thu, Aug 16, 2018 at 06:06:16PM +0200, Michal Kubecek wrote:
> > If not, we can go from there and evaluate this much larger patch
> > series.  But let's try the simple thing first.
> 
> At high packet rates (say 30K pkt/s and more), we can still saturate the
> CPU. This is also mentioned in the announcement with claim that switch
> to rbtree based queue would be necessary to fully address that. My tests
> seem to confirm that but I'm still not sure it is worth backporting
> something as intrusive into stable 4.4.

No, it is not.  If you worry about those things, you should not be
running a 4.4 kernel, use 4.14 or newer please.

thanks,

greg k-h

Re: [PATCH stable 4.4 0/9] fix SegmentSmack in stable branch (CVE-2018-5390)

[Greg KH]

On Thu, Aug 16, 2018 at 02:33:56PM +0200, Michal Kubecek wrote:
> On Thu, Aug 16, 2018 at 08:05:50PM +0800, maowenan wrote:
> > On 2018/8/16 19:39, Michal Kubecek wrote:
> > > 
> > > I suspect you may be doing something wrong with your tests. I checked
> > > the segmentsmack testcase and the CPU utilization on receiving side
> > > (with sending 10 times as many packets as default) went down from ~100%
> > > to ~3% even when comparing what is in stable 4.4 now against older 4.4
> > > kernel.
> > 
> > There seems no obvious problem when you send packets with default
> > parameter in Segmentsmack POC, Which is also very related with your
> > server's hardware configuration. Please try with below parameter to
> > form OFO packets
> 
> I did and even with these (questionable, see below) changes, I did not
> get more than 10% (of one core) by receiving ksoftirqd.
> 
> >   for (i = 0; i < 1024; i++)  // 128->1024
> ...
> >   usleep(10*1000); // Adjust this and packet count to match the 
> > target!, sleep 100ms->10ms
> 
> The comment in the testcase source suggests to do _one_ of these two
> changes so that you generate 10 times as many packets as the original
> testcase. You did both so that you end up sending 102400 packets per
> second. With 55 byte long packets, this kind of attack requires at least
> 5.5 MB/s (44 Mb/s) of throughput. This is no longer a "low packet rate
> DoS", I'm afraid.
> 
> Anyway, even at this rate, I only get ~10% of one core (Intel E5-2697).
> 
> What I can see, though, is that with current stable 4.4 code, modified
> testcase which sends something like
> 
>   2:3, 3:4, ..., 3001:3002, 3003:3004, 3004:3005, ... 6001:6002, ...
> 
> I quickly eat 6 MB of memory for receive queue of one socket while
> earlier 4.4 kernels only take 200-300 KB. I didn't test latest 4.4 with
> Takashi's follow-up yet but I'm pretty sure it will help while
> preserving nice performance when using the original segmentsmack
> testcase (with increased packet ratio).

Ok, for now I've applied Takashi's fix to the 4.4 stable queue and will
push out a new 4.4-rc later tonight.  Can everyone standardize on that
and test and let me know if it does, or does not, fix the reported
issues?

If not, we can go from there and evaluate this much larger patch series.
But let's try the simple thing first.

thanks,

greg k-h

Re: [PATCH stable 4.4 1/9] Revert "tcp: detect malicious patterns in tcp_collapse_ofo_queue()"

[Greg KH]

On Thu, Aug 16, 2018 at 09:55:42AM +0800, maowenan wrote:
> 
> 
> On 2018/8/15 21:18, Greg KH wrote:
> > On Wed, Aug 15, 2018 at 09:21:00PM +0800, Mao Wenan wrote:
> >> This reverts commit dc6ae4dffd656811dee7151b19545e4cd839d378.
> > 
> > I need a reason why, and a signed-off-by line :(
> 
> stable 4.4 only back port two patches to fix CVE-2018-5390, I have tested 
> they can't
> fix fully because of simple queue used in lower version, so we need change 
> simple queue
> to RB tree to finally resolve. But 9f5afeae have many conflicts with tcp: 
> detect malicious patterns in tcp_collapse_ofo_queue()
> and tcp: avoid collapses in tcp_prune_queue() if possible, and there are 
> patch series from Eric in mainline to fix CVE-2018-5390,
> so I need revert part of patches in stable 4.4 firstly, then apply 9f5afeae, 
> and reapply five patches from Eric.
> 9f5afeae tcp: use an RB tree for ooo receive queue

Then please put this information in the changelog text, that's what we
need to see here.

thanks,

greg k-h

Re: [PATCH stable 4.4 0/9] fix SegmentSmack (CVE-2018-5390)

[Greg KH]

On Wed, Aug 15, 2018 at 03:24:32PM +0200, Greg KH wrote:
> On Wed, Aug 15, 2018 at 09:20:59PM +0800, Mao Wenan wrote:
> > There are five patches to fix CVE-2018-5390 in latest mainline 
> > branch, but only two patches exist in stable 4.4 and 3.18: 
> > dc6ae4d tcp: detect malicious patterns in tcp_collapse_ofo_queue()
> > 5fbec48 tcp: avoid collapses in tcp_prune_queue() if possible
> > but I have tested with these patches, and found the cpu usage was very high.
> > test results:
> > with fix patch： 78.2%   ksoftirqd
> > no fix patch：   90% ksoftirqd
> > 
> > After analysing the codes of stable 4.4, and debuging the 
> > system, the search of ofo_queue(tcp ofo using a simple queue) cost more 
> > cycles.
> > So I think only two patches can't fix the CVE-2018-5390.
> > So I try to backport "tcp: use an RB tree for ooo receive queue" using RB 
> > tree 
> > instead of simple queue, then backport Eric Dumazet 5 fixed patches in 
> > mainline,
> > good news is that ksoftirqd is turn to about 20%, which is the same with 
> > mainline now.
> 
> Thanks for doing this work, I had some questions on the individual
> patches.  Can you address them and resend?

Also, always cc: the stable@vger list when sending stable patches so
that others can review and comment on them.

thanks,

greg k-h

Re: [PATCH stable 4.4 4/9] tcp: use an RB tree for ooo receive queue

[Greg KH]

On Wed, Aug 15, 2018 at 09:21:03PM +0800, Mao Wenan wrote:
> From: Yaogong Wang 
> 
> Over the years, TCP BDP has increased by several orders of magnitude,
> and some people are considering to reach the 2 Gbytes limit.
> 
> Even with current window scale limit of 14, ~1 Gbytes maps to ~740,000
> MSS.
> 
> In presence of packet losses (or reorders), TCP stores incoming packets
> into an out of order queue, and number of skbs sitting there waiting for
> the missing packets to be received can be in the 10^5 range.
> 
> Most packets are appended to the tail of this queue, and when
> packets can finally be transferred to receive queue, we scan the queue
> from its head.
> 
> However, in presence of heavy losses, we might have to find an arbitrary
> point in this queue, involving a linear scan for every incoming packet,
> throwing away cpu caches.
> 
> This patch converts it to a RB tree, to get bounded latencies.
> 
> Yaogong wrote a preliminary patch about 2 years ago.
> Eric did the rebase, added ofo_last_skb cache, polishing and tests.
> 
> Tested with network dropping between 1 and 10 % packets, with good
> success (about 30 % increase of throughput in stress tests)
> 
> Next step would be to also use an RB tree for the write queue at sender
> side ;)
> 
> Signed-off-by: Yaogong Wang 
> Signed-off-by: Eric Dumazet 
> Cc: Yuchung Cheng 
> Cc: Neal Cardwell 
> Cc: Ilpo Järvinen 
> Acked-By: Ilpo Järvinen 
> Signed-off-by: David S. Miller 
> Signed-off-by: root 

root and commit id?

Re: [PATCH stable 4.4 6/9] tcp: avoid collapses in tcp_prune_queue() if possible

[Greg KH]

On Wed, Aug 15, 2018 at 09:21:05PM +0800, Mao Wenan wrote:
> From: Eric Dumazet 
> 
> [ Upstream commit f4a3313d8e2ca9fd8d8f45e40a2903ba782607e7 ]
> 
> Right after a TCP flow is created, receiving tiny out of order
> packets allways hit the condition :
> 
> if (atomic_read(>sk_rmem_alloc) >= sk->sk_rcvbuf)
>   tcp_clamp_window(sk);
> 
> tcp_clamp_window() increases sk_rcvbuf to match sk_rmem_alloc
> (guarded by tcp_rmem[2])
> 
> Calling tcp_collapse_ofo_queue() in this case is not useful,
> and offers a O(N^2) surface attack to malicious peers.
> 
> Better not attempt anything before full queue capacity is reached,
> forcing attacker to spend lots of resource and allow us to more
> easily detect the abuse.
> 
> Signed-off-by: Eric Dumazet 
> Acked-by: Soheil Hassas Yeganeh 
> Acked-by: Yuchung Cheng 
> Signed-off-by: David S. Miller 
> Signed-off-by: Greg Kroah-Hartman 
> Signed-off-by: root 

root?

Re: [PATCH stable 4.4 5/9] tcp: free batches of packets in tcp_prune_ofo_queue()

[Greg KH]

On Wed, Aug 15, 2018 at 09:21:04PM +0800, Mao Wenan wrote:
> From: Eric Dumazet 
> 
> Juha-Matti Tilli reported that malicious peers could inject tiny
> packets in out_of_order_queue, forcing very expensive calls
> to tcp_collapse_ofo_queue() and tcp_prune_ofo_queue() for
> every incoming packet. out_of_order_queue rb-tree can contain
> thousands of nodes, iterating over all of them is not nice.
> 
> Before linux-4.9, we would have pruned all packets in ofo_queue
> in one go, every  packets.  depends on sk_rcvbuf and skbs
> truesize, but is about 7000 packets with tcp_rmem[2] default of 6 MB.
> 
> Since we plan to increase tcp_rmem[2] in the future to cope with
> modern BDP, can not revert to the old behavior, without great pain.
> 
> Strategy taken in this patch is to purge ~12.5 % of the queue capacity.
> 
> Fixes: 36a6503fedda ("tcp: refine tcp_prune_ofo_queue() to not drop all 
> packets")
> Signed-off-by: Eric Dumazet 
> Reported-by: Juha-Matti Tilli 
> Acked-by: Yuchung Cheng 
> Acked-by: Soheil Hassas Yeganeh 
> Signed-off-by: David S. Miller 
> Signed-off-by: root 

root?

And commit id?

thanks,

greg k-h

Re: [PATCH stable 4.4 9/9] tcp: add tcp_ooo_try_coalesce() helper

[Greg KH]

On Wed, Aug 15, 2018 at 09:21:08PM +0800, Mao Wenan wrote:
> From: Eric Dumazet 
> 
> In case skb in out_or_order_queue is the result of
> multiple skbs coalescing, we would like to get a proper gso_segs
> counter tracking, so that future tcp_drop() can report an accurate
> number.
> 
> I chose to not implement this tracking for skbs in receive queue,
> since they are not dropped, unless socket is disconnected.
> 
> Signed-off-by: Eric Dumazet 
> Acked-by: Soheil Hassas Yeganeh 
> Acked-by: Yuchung Cheng 
> Signed-off-by: David S. Miller 
> Signed-off-by: Mao Wenan 

Upstream commit id?

Re: [PATCH stable 4.4 0/9] fix SegmentSmack (CVE-2018-5390)

[Greg KH]

On Wed, Aug 15, 2018 at 09:20:59PM +0800, Mao Wenan wrote:
> There are five patches to fix CVE-2018-5390 in latest mainline 
> branch, but only two patches exist in stable 4.4 and 3.18: 
> dc6ae4d tcp: detect malicious patterns in tcp_collapse_ofo_queue()
> 5fbec48 tcp: avoid collapses in tcp_prune_queue() if possible
> but I have tested with these patches, and found the cpu usage was very high.
> test results:
> with fix patch： 78.2%   ksoftirqd
> no fix patch：   90% ksoftirqd
> 
> After analysing the codes of stable 4.4, and debuging the 
> system, the search of ofo_queue(tcp ofo using a simple queue) cost more 
> cycles.
> So I think only two patches can't fix the CVE-2018-5390.
> So I try to backport "tcp: use an RB tree for ooo receive queue" using RB 
> tree 
> instead of simple queue, then backport Eric Dumazet 5 fixed patches in 
> mainline,
> good news is that ksoftirqd is turn to about 20%, which is the same with 
> mainline now.

Thanks for doing this work, I had some questions on the individual
patches.  Can you address them and resend?

thanks,

greg k-h

Re: [PATCH stable 4.4 8/9] tcp: call tcp_drop() from tcp_data_queue_ofo()

[Greg KH]

On Wed, Aug 15, 2018 at 09:21:07PM +0800, Mao Wenan wrote:
> From: Eric Dumazet 
> 
> In order to be able to give better diagnostics and detect
> malicious traffic, we need to have better sk->sk_drops tracking.
> 
> Fixes: 9f5afeae5152 ("tcp: use an RB tree for ooo receive queue")
> Signed-off-by: Eric Dumazet 
> Acked-by: Soheil Hassas Yeganeh 
> Acked-by: Yuchung Cheng 
> Signed-off-by: David S. Miller 
> Signed-off-by: Mao Wenan 
> ---
>  net/ipv4/tcp_input.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Upstream commit id?

Re: [PATCH stable 4.4 3/9] tcp: increment sk_drops for dropped rx packets

[Greg KH]

On Wed, Aug 15, 2018 at 09:21:02PM +0800, Mao Wenan wrote:
> From: Eric Dumazet 
> 
> Now ss can report sk_drops, we can instruct TCP to increment
> this per socket counter when it drops an incoming frame, to refine
> monitoring and debugging.
> 
> Following patch takes care of listeners drops.
> 
> Signed-off-by: Eric Dumazet 
> Signed-off-by: David S. Miller 
> Signed-off-by: Mao Wenan 

What is the upstream git commit id for this?

thanks,

greg k-h

Re: [PATCH stable 4.4 7/9] tcp: detect malicious patterns in tcp_collapse_ofo_queue()

[Greg KH]

On Wed, Aug 15, 2018 at 09:21:06PM +0800, Mao Wenan wrote:
> From: Eric Dumazet 
> 
> [ Upstream commit 3d4bf93ac12003f9b8e1e2de37fe27983deebdcf ]
> 
> In case an attacker feeds tiny packets completely out of order,
> tcp_collapse_ofo_queue() might scan the whole rb-tree, performing
> expensive copies, but not changing socket memory usage at all.
> 
> 1) Do not attempt to collapse tiny skbs.
> 2) Add logic to exit early when too many tiny skbs are detected.
> 
> We prefer not doing aggressive collapsing (which copies packets)
> for pathological flows, and revert to tcp_prune_ofo_queue() which
> will be less expensive.
> 
> In the future, we might add the possibility of terminating flows
> that are proven to be malicious.
> 
> Signed-off-by: Eric Dumazet 
> Acked-by: Soheil Hassas Yeganeh 
> Signed-off-by: David S. Miller 
> Signed-off-by: Greg Kroah-Hartman 
> Signed-off-by: root 

signed off by from "root"?  :)

And why are you adding the patch back after removing it?

thanks,

greg k-h

Re: [PATCH stable 4.4 2/9] Revert "tcp: avoid collapses in tcp_prune_queue() if possible"

[Greg KH]

On Wed, Aug 15, 2018 at 09:21:01PM +0800, Mao Wenan wrote:
> This reverts commit 5fbec4801264cb3279ef6ac9c70bcbe2aaef89d5.
> ---

Same here for description and signed off by.

thanks,

greg k-h

Re: [PATCH stable 4.4 1/9] Revert "tcp: detect malicious patterns in tcp_collapse_ofo_queue()"

[Greg KH]

On Wed, Aug 15, 2018 at 09:21:00PM +0800, Mao Wenan wrote:
> This reverts commit dc6ae4dffd656811dee7151b19545e4cd839d378.

I need a reason why, and a signed-off-by line :(

thanks,

greg k-h

Re: [PATCH 4.9-stable] tcp: add tcp_ooo_try_coalesce() helper

[Greg KH]

On Thu, Aug 09, 2018 at 08:37:13PM +0800, maowenan wrote:
> 
> 
> On 2018/8/7 21:22, Greg KH wrote:
> > On Sat, Aug 04, 2018 at 10:10:00AM +0100, David Woodhouse wrote:
> >> From: Eric Dumazet 
> >>
> >> commit 58152ecbbcc6a0ce7fddd5bf5f6ee535834ece0c upstream.
> >>
> >> In case skb in out_or_order_queue is the result of
> >> multiple skbs coalescing, we would like to get a proper gso_segs
> >> counter tracking, so that future tcp_drop() can report an accurate
> >> number.
> >>
> >> I chose to not implement this tracking for skbs in receive queue,
> >> since they are not dropped, unless socket is disconnected.
> >>
> >> Signed-off-by: Eric Dumazet 
> >> Acked-by: Soheil Hassas Yeganeh 
> >> Acked-by: Yuchung Cheng 
> >> Signed-off-by: David S. Miller 
> >> Signed-off-by: David Woodhouse 
> >> ---
> >>  net/ipv4/tcp_input.c | 23 +--
> >>  1 file changed, 21 insertions(+), 2 deletions(-)
> > 
> > Now applied, thanks,
> > 
> > greg k-h
> > 
> > .
> > 
> 
> Hello,
> 
> There are two patches in stable branch linux-4.4, but I have tested with 
> below patches, and found that the cpu usage was very high.
> dc6ae4d tcp: detect malicious patterns in tcp_collapse_ofo_queue()
> 5fbec48 tcp: avoid collapses in tcp_prune_queue() if possible
> 
> test results:
> with fix patch： 78.2%   ksoftirqd
> no fix patch：   90% ksoftirqd
> 
> there is %0 when no attack packets.
> 
> so please help verify that fixed patches are enough in linux-stable 4.4.
> 

I do not know, I am not a network developer.  Please try to reproduce
the same thing on a newer kernel release and see if the result is the
same or not.  If you can find a change that I missed, please let me know
and I will be glad to apply it.

thnaks,

greg k-h

Re: [PATCH 4.9-stable] tcp: add tcp_ooo_try_coalesce() helper

[Greg KH]

On Sat, Aug 04, 2018 at 10:10:00AM +0100, David Woodhouse wrote:
> From: Eric Dumazet 
> 
> commit 58152ecbbcc6a0ce7fddd5bf5f6ee535834ece0c upstream.
> 
> In case skb in out_or_order_queue is the result of
> multiple skbs coalescing, we would like to get a proper gso_segs
> counter tracking, so that future tcp_drop() can report an accurate
> number.
> 
> I chose to not implement this tracking for skbs in receive queue,
> since they are not dropped, unless socket is disconnected.
> 
> Signed-off-by: Eric Dumazet 
> Acked-by: Soheil Hassas Yeganeh 
> Acked-by: Yuchung Cheng 
> Signed-off-by: David S. Miller 
> Signed-off-by: David Woodhouse 
> ---
>  net/ipv4/tcp_input.c | 23 +--
>  1 file changed, 21 insertions(+), 2 deletions(-)

Now applied, thanks,

greg k-h

Re: [PATCH 3.18.y] Fix compilation error backporting upstream commit 9fc12023d6f5

[Greg KH]

On Sat, Aug 04, 2018 at 02:27:41PM +0200, Lorenzo Bianconi wrote:
> Fix following compilation error backporting upstream commit
> 9fc12023d6f5 ("ipv4: remove BUG_ON() from fib_compute_spec_dst)
> 
> net/ipv4/fib_frontend.c: In function 'fib_compute_spec_dst':
> net/ipv4/fib_frontend.c:225:3: error: expected expression before 'if'
>if (!fib_lookup(net, , ))
> 
> net/ipv4/fib_frontend.c:200:20: warning: unused variable 'res'
>   struct fib_result res;
> 
> Fixes: 0e46da6c6fac ("ipv4: remove BUG_ON() from fib_compute_spec_dst")
> Signed-off-by: Lorenzo Bianconi 
> ---
>  net/ipv4/fib_frontend.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c
> index 924db4bedd88..0b29649627a7 100644
> --- a/net/ipv4/fib_frontend.c
> +++ b/net/ipv4/fib_frontend.c
> @@ -221,7 +221,7 @@ __be32 fib_compute_spec_dst(struct sk_buff *skb)
>   fl4.saddr = 0;
>   fl4.flowi4_tos = RT_TOS(ip_hdr(skb)->tos);
>   fl4.flowi4_scope = scope;
> - fl4.flowi4_mark = vmark ? skb->mark : 0,
> + fl4.flowi4_mark = vmark ? skb->mark : 0;
>   if (!fib_lookup(net, , ))
>   return FIB_RES_PREFSRC(net, res);
>   } else {

Thanks, but I fixed this about 6 hours ago, right?

greg k-h

Re: [PATCH net 0/5] tcp: more robust ooo handling

[Greg KH]

On Fri, Aug 03, 2018 at 04:53:27PM -0700, David Miller wrote:
> From: David Woodhouse 
> Date: Fri, 03 Aug 2018 11:55:37 +0100
> 
> > I see the first four in 4.9.116 but not the fifth (adding
> > tcp_ooo_try_coalesce()).
> > 
> > Is that intentional? 
> 
> I don't work on the 4.9 -stable backports, so I personally have
> no idea.
> 
> I submitted for 4.17 and 4.14

Ok, then it's my fault :)

Odds are it did not apply and so I didn't backport it.  If you think it
should be there, please provide a working backport.

thanks,

greg k-h

Re: 862591bf4("xfrm: skip policies marked as dead while rehashing")

[Greg KH]

On Wed, Jun 20, 2018 at 05:42:51PM -0400, Zubin Mithra wrote:
> Hello,
> 
> Syzkaller has reported a crash here[1] for a slab OOB read in
> xfrm_hash_rebuild.
> 
> Could the following 2 patches be applied in order to on 4.4.y?
> 
> 6916fb3b10("xfrm: Ignore socket policies when rebuilding hash tables")
> 862591bf4f("xfrm: skip policies marked as dead while rehashing")
> 
> [1] 
> https://syzkaller.appspot.com/bug?id=1c11a638b7d27e871aa297f3b4d5fd5bc90f0cb4

Both now queued up, thanks.

greg k-h

Re: Incomplete fix for be9c798 / 7b2ee50 hv_netvsc: common detach logic?

[Greg KH]

On Tue, Jun 19, 2018 at 03:19:41PM -0400, Thomas Walker wrote:
> Upon updating some internal kernels from 4.14.18 to 4.14.49, our hyper-v 
> guests failed to bring up network interfaces on boot, logging "A link change 
> request failed with some changes committed already. Interface eth0 may have 
> been left with an inconsistent configuration, please check."  Running 
> 'ifconfig eth0 up' appears to fix the problem temporarily so I went about 
> bisecting which landed on:
> 
> commit be9c798d0d13ae609a91177323ac816545c39d28
> Author: Stephen Hemminger 
> Date:   Mon May 14 15:32:18 2018 -0700
> 
> hv_netvsc: common detach logic
> 
> [ Commit 7b2ee50c0cd513a176a26a71f2989facdd75bfea upstream. ]
> 
> Make common function for detaching internals of device
> during changes to MTU and RSS. Make sure no more packets
> are transmitted and all packets have been received before
> doing device teardown.
> 
> Change the wait logic to be common and use usleep_range().
> 
> Changes transmit enabling logic so that transmit queues are disabled
> during the period when lower device is being changed. And enabled
> only after sub channels are setup. This avoids issue where it could
> be that a packet was being sent while subchannel was not initialized.
> 
> Fixes: 8195b1396ec8 ("hv_netvsc: fix deadlock on hotplug")
> Signed-off-by: Stephen Hemminger 
> Signed-off-by: David S. Miller 
> Signed-off-by: Greg Kroah-Hartman 
> 
> 
> The removal of which does indeed fix the problem.  That led me to:
> 
> commit 52acf73b6e9a6962045feb2ba5a8921da2201915
> Author: Dexuan Cui 
> Date:   Wed Jun 6 21:32:51 2018 +
> 
> hv_netvsc: Fix a network regression after ifdown/ifup
> 
> Recently people reported the NIC stops working after
> "ifdown eth0; ifup eth0". It turns out in this case the TX queues are not
> enabled, after the refactoring of the common detach logic: when the NIC
> has sub-channels, usually we enable all the TX queues after all
> sub-channels are set up: see rndis_set_subchannel() ->
> netif_device_attach(), but in the case of "ifdown eth0; ifup eth0" where
> the number of channels doesn't change, we also must make sure the TX 
> queues
> are enabled. The patch fixes the regression.
> 
> Fixes: 7b2ee50c0cd5 ("hv_netvsc: common detach logic")
> Signed-off-by: Dexuan Cui 
> Cc: Stephen Hemminger 
> Cc: K. Y. Srinivasan 
> Cc: Haiyang Zhang 
> Signed-off-by: David S. Miller 
> 
> Which sounded very promising, but does not seem to fully fix the issue.  
> Doing some more digging, I was able to determine that the message coincides 
> with 'ip link set dev eth0 mtu 1300 up' very shortly (>~1 second) after the 
> hv_netvsc driver loads.  If I delay the mtu change until well after the 
> driver loads, everything works fine.  If I unload hv_netvsc and then reload 
> it and apply the mtu change immediately, the failure re-occurs.  So something 
> is racy here, and the above doesn't entirely address it.
> 
> I'm happy to test out any suggested patches and/or do additional debugging if 
> anyone has any suggestions.
> 
> (oh, and I did also try 4.18-rc1 and the problem still persists)

Always cc: the authors of the patch you are having problems with, along
with the mailing list for the networking subsystem, so that the right
people know to look at this.

Now fixed...

thanks,

greg k-h

Re: [Patch net-next] netdev-FAQ: clarify DaveM's position for stable backports

[Greg KH]

On Mon, Jun 04, 2018 at 11:07:19AM -0700, Cong Wang wrote:
> Per discussion with David at netconf 2018, let's clarify
> DaveM's position of handling stable backports in netdev-FAQ.
> 
> This is important for people relying on upstream -stable
> releases.
> 
> Cc: sta...@vger.kernel.org
> Cc: Greg Kroah-Hartman 
> Signed-off-by: Cong Wang 
> ---
>  Documentation/networking/netdev-FAQ.txt | 9 +
>  1 file changed, 9 insertions(+)
> 
> diff --git a/Documentation/networking/netdev-FAQ.txt 
> b/Documentation/networking/netdev-FAQ.txt
> index 2a3278d5cf35..6dde6686c870 100644
> --- a/Documentation/networking/netdev-FAQ.txt
> +++ b/Documentation/networking/netdev-FAQ.txt
> @@ -179,6 +179,15 @@ A: No.  See above answer.  In short, if you think it 
> really belongs in
> dash marker line as described in 
> Documentation/process/submitting-patches.rst to
> temporarily embed that information into the patch that you send.
>  
> +Q: Are all networking bug fixes backported to all stable releases?
> +
> +A: Due to capacity, Dave could only take care of the backports for the last
> +   3 stable releases. For earlier stable releases, each stable branch 
> maintainer

s/3/2/

There's no need for Dave to do more work than he currently does :)

thanks,

greg k-h

Re: 4.16 issue with mbim modem and ping with size > 14552 bytes

[Greg KH]

On Mon, May 28, 2018 at 09:58:01AM +0200, Daniele Palmas wrote:
> 2018-05-25 0:54 GMT+02:00 Daniele Palmas <dnl...@gmail.com>:
> > Hi Greg,
> >
> > 2018-05-24 17:53 GMT+02:00 Greg KH <gre...@linuxfoundation.org>:
> >> On Thu, May 24, 2018 at 05:04:49PM +0200, Daniele Palmas wrote:
> >>> Hello,
> >>>
> >>> I have an issue with an USB mbim modem when trying to send with ping
> >>> more than 14552 bytes: it looks like to me a kernel issue, but not at
> >>> the cdc_mbim or cdc_ncm level, anyway not sure, so I'm reporting the
> >>> issue.
> >>>
> >>> My kernel is 4.16. The device is the following:
> >>
> >> Does older kernels work, or is this something that has always been
> >> there?
> >>
> >
> > Not tested yet, I'm going to do.
> >
> 
> So, ping with more than 14552 was working properly until v4.5-rc7:
> starting from v4.5 I'm not even able to make a data connection with
> mbim since things fail badly with the following:
> 
> [  259.551836] [ cut here ]
> [  259.551848] WARNING: CPU: 2 PID: 0 at
> /home/kernel/COD/linux/net/sched/sch_generic.c:303
> dev_watchdog+0x237/0x240()
> [  259.551860] NETDEV WATCHDOG: wwp0s20u7i2 (cdc_mbim): transmit queue
> 0 timed out
> [  259.551861] Modules linked in: cdc_mbim cdc_wdm cdc_ncm usbnet mii
> intel_rapl x86_pkg_temp_thermal intel_powerclamp coretemp kvm
> snd_hda_codec_realtek snd_hda_codec_hdmi snd_hda_codec_generic
> irqbypass crct10dif_pclmul snd_hda_intel snd_hda_codec snd_hda_core
> crc32_pclmul snd_hwdep ghash_clmulni_intel aesni_intel aes_x86_64
> snd_pcm snd_seq_midi lrw snd_seq_midi_event gf128mul input_leds
> snd_rawmidi snd_seq glue_helper snd_seq_device ablk_helper snd_timer
> cryptd snd soundcore shpchp serio_raw mac_hid lpc_ich 8250_fintek
> mei_me mei parport_pc ppdev lp parport autofs4 hid_generic usbhid hid
> i915 i2c_algo_bit drm_kms_helper syscopyarea sysfillrect psmouse
> sysimgblt ahci fb_sys_fops e1000e libahci drm ptp pps_core wmi fjes
> video
> [  259.551889] CPU: 2 PID: 0 Comm: swapper/2 Not tainted
> 4.5.0-040500-generic #201603140130
> [  259.551890] Hardware name: LENOVO 10A6A0J5IX/SHARKBAY, BIOS
> FBKT79AUS 04/17/2014
> [  259.551891]  0286 108c91d75cf5b65f 88021eb03d98
> 813e0233
> [  259.551893]  88021eb03de0 81d816a0 88021eb03dd0
> 81080e72
> [  259.551894]   8800cee81880 0002
> 8800a19f8000
> [  259.551895] Call Trace:
> [  259.551896][] dump_stack+0x63/0x90
> [  259.551903]  [] warn_slowpath_common+0x82/0xc0
> [  259.551904]  [] warn_slowpath_fmt+0x5c/0x80
> [  259.551907]  [] dev_watchdog+0x237/0x240
> [  259.551909]  [] ? qdisc_rcu_free+0x40/0x40
> [  259.551910]  [] call_timer_fn+0x35/0x120
> [  259.551911]  [] ? qdisc_rcu_free+0x40/0x40
> [  259.551912]  [] run_timer_softirq+0x246/0x2f0
> [  259.551914]  [] __do_softirq+0xf6/0x280
> [  259.551916]  [] irq_exit+0xa3/0xb0
> [  259.551919]  [] smp_apic_timer_interrupt+0x42/0x50
> [  259.551920]  [] apic_timer_interrupt+0x82/0x90
> [  259.551922][] ? cpuidle_enter_state+0x11d/0x2c0
> [  259.551925]  [] cpuidle_enter+0x17/0x20
> [  259.551928]  [] call_cpuidle+0x2a/0x40
> [  259.551929]  [] cpu_startup_entry+0x295/0x350
> [  259.551931]  [] start_secondary+0x15e/0x190
> [  259.551933] ---[ end trace 6f8d3c1a1b02644d ]---
> 
> but this is probably something different, cause with v4.16 data
> connection works fine.
> 
> If this is not helpful I guess I need to bisect.

Bisection would be best.  It looks like you narrowed things down really
well already, bisection should go very quickly.

thanks,

greg k-h

Re: 4.16 issue with mbim modem and ping with size > 14552 bytes

[Greg KH]

On Thu, May 24, 2018 at 05:04:49PM +0200, Daniele Palmas wrote:
> Hello,
> 
> I have an issue with an USB mbim modem when trying to send with ping
> more than 14552 bytes: it looks like to me a kernel issue, but not at
> the cdc_mbim or cdc_ncm level, anyway not sure, so I'm reporting the
> issue.
> 
> My kernel is 4.16. The device is the following:

Does older kernels work, or is this something that has always been
there?

I ask, as my mobile provider does horrible things to large packet sizes.
So much so that I have to set the mtu to 1280 just to get things to work
properly when tethering my phone through to my laptop.  So this might be
a network provider issue :)

thanks,

greg k-h

Re: [PATCH v3 net-next 0/2] bpfilter

[Greg KH]

On Wed, May 23, 2018 at 01:26:48PM -0400, David Miller wrote:
> From: Alexei Starovoitov 
> Date: Mon, 21 May 2018 19:22:28 -0700
> 
> > v2->v3:
> > - followed Luis's suggestion and significantly simplied first patch
> >   with shmem_kernel_file_setup+kernel_write. Added kdoc for new helper
> > - fixed typos and race to access pipes with mutex
> > - tested with bpfilter being 'builtin'. CONFIG_BPFILTER_UMH=y|m both work.
> >   Interesting to see a usermode executable being embedded inside vmlinux.
> > - it doesn't hurt to enable bpfilter in .config.
> >   ip_setsockopt commands sent to usermode via pipes and -ENOPROTOOPT is
> >   returned from userspace, so kernel falls back to original iptables code
> > 
> > v1->v2:
> > this patch set is almost a full rewrite of the earlier umh modules approach
> > The v1 of patches and follow up discussion was covered by LWN:
> > https://lwn.net/Articles/749108/
> > 
> > I believe the v2 addresses all issues brought up by Andy and others.
> > Mainly there are zero changes to kernel/module.c
> > Instead of teaching module loading logic to recognize special
> > umh module, let normal kernel modules execute part of its own
> > .init.rodata as a new user space process (Andy's idea)
> > Patch 1 introduces this new helper:
> > int fork_usermode_blob(void *data, size_t len, struct umh_info *info);
> > Input:
> >   data + len == executable file
> > Output:
> >   struct umh_info {
> >struct file *pipe_to_umh;
> >struct file *pipe_from_umh;
> >pid_t pid;
> >   };
> 
> Series applied, let the madness begin... :-)

Yeah, this is going to be fun :)

Re: Request for -stable inclusion: time stamping fix for nfp

[Greg KH]

On Thu, May 17, 2018 at 08:32:02PM +0200, Willy Tarreau wrote:
> Adding Greg here.
> 
> Greg, apparently a backport of 46f1c52e66db is needed in 4.9 according
> to the thread below. It was merged in 4.13 so 4.14 already has it.

Thanks for the notice, now queued up.

greg k-h

Re: net: ieee802154: 6lowpan: fix frag reassembly

[Greg KH]

On Thu, May 17, 2018 at 04:16:20PM +0200, Stefan Schmidt wrote:
> Hello Greg.
> 
> On 17.05.2018 10:59, Greg KH wrote:
> > On Mon, May 14, 2018 at 05:22:18PM +0200, Stefan Schmidt wrote:
> >> Hello.
> >>
> >>
> >> Please apply f18fa5de5ba7f1d6650951502bb96a6e4715a948
> >>
> >> (net: ieee802154: 6lowpan: fix frag reassembly) to the 4.16.x stable tree.
> >>
> >>
> >> Earlier trees are not needed as the problem was introduced in 4.16.
> > 
> > Really?  Commit f18fa5de5ba7 ("net: ieee802154: 6lowpan: fix frag
> > reassembly") says it fixes commit 648700f76b03 ("inet: frags: use
> > rhashtables for reassembly units") which did not show up until 4.17-rc1:
> > $ git describe --contains 648700f76b03
> > v4.17-rc1~148^2~20^2~11
> > 
> > Also, it did not get backported to 4.16.y, so I don't see how it is
> > needed in 4.16-stable.
> 
> I guess its time to blush on my side. During the bisection for the
> commit that introduced the problem I came to the point where it was
> clear to me that it was already in 4.16. This was a while back I have
> have honestly no idea how I did this mistake.
> 
> I tested again now with plain 4.16 and it works fine.
> The fix is also in 4.17-rcX where it actually is needed. In the end I am
> glad that it was not introduced and slipped me in an earlier release.
> 
> > To verify this, I tried applying the patch, and it totally fails to
> > apply to the 4.16.y tree.
> > 
> > So are you _sure_ you want/need this in 4.16?  If so, can you provide a
> > working backport that you have verified works?
> 
> No backport needed. I simply screwed up when verifying this for 4.16.
> I put on the hat of shame for today and will try harder the next time.

Hey, not a problem, thanks for verifying, 'git describe --contains' is
your friend :)

thanks,

greg k-h

Re: net: ieee802154: 6lowpan: fix frag reassembly

[Greg KH]

On Mon, May 14, 2018 at 05:22:18PM +0200, Stefan Schmidt wrote:
> Hello.
> 
> 
> Please apply f18fa5de5ba7f1d6650951502bb96a6e4715a948
> 
> (net: ieee802154: 6lowpan: fix frag reassembly) to the 4.16.x stable tree.
> 
> 
> Earlier trees are not needed as the problem was introduced in 4.16.

Really?  Commit f18fa5de5ba7 ("net: ieee802154: 6lowpan: fix frag
reassembly") says it fixes commit 648700f76b03 ("inet: frags: use
rhashtables for reassembly units") which did not show up until 4.17-rc1:
$ git describe --contains 648700f76b03
v4.17-rc1~148^2~20^2~11

Also, it did not get backported to 4.16.y, so I don't see how it is
needed in 4.16-stable.

To verify this, I tried applying the patch, and it totally fails to
apply to the 4.16.y tree.

So are you _sure_ you want/need this in 4.16?  If so, can you provide a
working backport that you have verified works?

thanks,

greg k-h

Re: [PATCH v4 1/1] drivers core: multi-threading device shutdown

[Greg KH]

On Mon, May 14, 2018 at 03:42:54PM -0400, Pavel Tatashin wrote:
> When system is rebooted, halted or kexeced device_shutdown() is
> called.
> 
> This function shuts down every single device by calling either:
> 
>   dev->bus->shutdown(dev)
>   dev->driver->shutdown(dev)
> 
> Even on a machine with just a moderate amount of devices, device_shutdown()
> may take multiple seconds to complete. This is because many devices require
> a specific delays to perform this operation.
> 
> Here is a sample analysis of time it takes to call device_shutdown() on a
> two socket Intel(R) Xeon(R) CPU E5-2630 v4 @ 2.20GHz machine.
> 
> device_shutdown   2.95s
> -
>  mlx4_shutdown1.14s
>  megasas_shutdown 0.24s
>  ixgbe_shutdown   0.37s x 4 (four ixgbe devices on this machine).
>  the rest 0.09s
> 
> In mlx4 we spent the most time, but that is because there is a 1 second
> sleep, which is defined by hardware specifications:
> mlx4_shutdown
>  mlx4_unload_one
>   mlx4_free_ownership
>msleep(1000)
> 
> With megasas we spend a quarter of a second, but sometimes longer (up-to
> 0.5s) in this path:
> 
> megasas_shutdown
>   megasas_flush_cache
> megasas_issue_blocked_cmd
>   wait_event_timeout
> 
> Finally, with ixgbe_shutdown() it takes 0.37 for each device, but that time
> is spread all over the place, with bigger offenders:
> 
> ixgbe_shutdown
>   __ixgbe_shutdown
> ixgbe_close_suspend
>   ixgbe_down
> ixgbe_init_hw_generic
>   ixgbe_reset_hw_X540
> msleep(100);0.104483472
> ixgbe_get_san_mac_addr_generic  0.048414851
> ixgbe_get_wwn_prefix_generic0.048409893
>   ixgbe_start_hw_X540
> ixgbe_start_hw_generic
>   ixgbe_clear_hw_cntrs_generic  0.048581502
>   ixgbe_setup_fc_generic0.024225800
> 
> All the ixgbe_*generic functions end-up calling:
> ixgbe_read_eerd_X540()
>   ixgbe_acquire_swfw_sync_X540
> usleep_range(5000, 6000);
>   ixgbe_release_swfw_sync_X540
> usleep_range(5000, 6000);
> 
> While these are short sleeps, they end-up calling them over 24 times!
> 24 * 0.0055s = 0.132s. Adding-up to 0.528s for four devices. Also we have
> four msleep(100). Totaling to:  0.928s
> 
> While we should keep optimizing the individual device drivers, in some
> cases this is simply a hardware property that forces a specific delay, and
> we must wait.
> 
> So, the solution for this problem is to shutdown devices in parallel.
> However, we must shutdown children before shutting down parents, so parent
> device must wait for its children to finish.
> 
> With this patch, on the same machine devices_shutdown() takes 1.142s, and
> without mlx4 one second delay only 0.38s
> 
> This feature can be optionally disabled via kernel parameter:
> device_shutdown_serial. When booted with this parameter, device_shutdown()
> will shutdown devices one by one.
> 
> Signed-off-by: Pavel Tatashin 
> ---
>  drivers/base/core.c | 292 
>  1 file changed, 242 insertions(+), 50 deletions(-)

Can you refactor this to be at least 2 patches?  One that moves code
around to comon functions to make the second patch, that adds the new
functionality, easier to review and understand?

And I echo the "don't use kerneldoc for static functions" review
comment, that's not needed at all.

thanks,

greg k-h

Re: net: ieee802154: 6lowpan: fix frag reassembly

[Greg KH]

On Mon, May 14, 2018 at 05:22:18PM +0200, Stefan Schmidt wrote:
> Hello.
> 
> 
> Please apply f18fa5de5ba7f1d6650951502bb96a6e4715a948
> 
> (net: ieee802154: 6lowpan: fix frag reassembly) to the 4.16.x stable tree.
> 
> 
> Earlier trees are not needed as the problem was introduced in 4.16.
> 
> 
> Normally net/ patches would come through DaveM, but he asked me for this one 
> to submit it directly when i sent him the pull request.
> 
> 
> First time stable request on my side here, let me know if I got something 
> wrong.

Looks fine, I'll queue it up later this week in the next release after
these go out on Wednesday.

thanks,

greg k-h

Re: [PATCH v3 0/1] multi-threading device shutdown

[Greg KH]

On Mon, May 07, 2018 at 11:54:01AM -0400, Pavel Tatashin wrote:
> Changelog
> v2 - v3
>   - Fixed warning from kbuild test.
>   - Moved device_lock/device_unlock inside device_shutdown_tree().
> 
> v1 - v2
>   - It turns out we cannot lock more than MAX_LOCK_DEPTH by a single
> thread. (By default this value is 48), and is used to detect
> deadlocks. So, I re-wrote the code to only lock one devices per
> thread instead of pre-locking all devices by the main thread.
>   - Addressed comments from Tobin C. Harding.
>   - As suggested by Alexander Duyck removed ixgbe changes. It can be
> done as a separate work scaling RTNL mutex.
> 
> Do a faster shutdown by calling dev->*->shutdown(dev) in parallel.
> device_shutdown() calls these functions for every single device but
> only using one thread.
> 
> Since, nothing else is running on the machine by the device_shutdown()
> s called, there is no reason not to utilize all the available CPU
> resources.

Ah, we can hope so.  I bet this is going to break something, so can we
have some way of turning it on/off dynamically for when it does?

thanks,

greg k-h

Re: WARNING in kernfs_add_one

[Greg KH]

On Sat, May 05, 2018 at 10:43:45AM -0700, Eric Dumazet wrote:
> 
> 
> On 05/05/2018 09:40 AM, Greg KH wrote:
> > On Sat, May 05, 2018 at 08:47:02AM -0700, syzbot wrote:
> >> Hello,
> >>
> >> syzbot found the following crash on:
> >>
> >> HEAD commit:8fb11a9a8d51 net/ipv6: rename rt6_next to fib6_next
> >> git tree:   net-next
> >> console output: https://syzkaller.appspot.com/x/log.txt?x=14b2723780
> >> kernel config:  https://syzkaller.appspot.com/x/.config?x=c416c61f3cd96be
> >> dashboard link: 
> >> https://syzkaller.appspot.com/bug?extid=df47f81c226b31d89fb1
> >> compiler:   gcc (GCC) 8.0.1 20180413 (experimental)
> >> syzkaller repro:https://syzkaller.appspot.com/x/repro.syz?x=172fb3e780
> >> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=16552e5780
> >>
> >> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> >> Reported-by: syzbot+df47f81c226b31d89...@syzkaller.appspotmail.com
> >>
> >> RBP: 7fff808f3e10 R08: 0002 R09: 7fff80003534
> >> R10:  R11: 0246 R12: 
> >> R13: 0006 R14:  R15: 
> >> [ cut here ]
> >> kernfs: ns required in 'ieee80211' for 'phy3'
> > 
> > That's interesting, this looks like a netfilter bug (adding netdev to
> > the report here.)
> 
> 
> I do not see anything netfilter related here.
> 
> More likely wireless territory

Ugh, that's what I get for writing emails before coffee in the
morning...

Yes, you are right, this looks like a wireless issue.

Now cc: linux-wireless.

> > Yes, we can "tone down" the kernfs warning to just be an error message
> > in the log, but there might be something worse going on here.
> > 
> > Network developers, any idea?  Rest of the callback chain is here:
> > 
> > 
> >> WARNING: CPU: 0 PID: 4538 at fs/kernfs/dir.c:759 kernfs_add_one+0x406/0x4d0
> >> fs/kernfs/dir.c:758
> >> Kernel panic - not syncing: panic_on_warn set ...
> >>
> >> CPU: 0 PID: 4538 Comm: syz-executor486 Not tainted 4.17.0-rc3+ #33
> >> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> >> Google 01/01/2011
> >> Call Trace:
> >>  __dump_stack lib/dump_stack.c:77 [inline]
> >>  dump_stack+0x1b9/0x294 lib/dump_stack.c:113
> >>  panic+0x22f/0x4de kernel/panic.c:184
> >>  __warn.cold.8+0x163/0x1b3 kernel/panic.c:536
> >>  report_bug+0x252/0x2d0 lib/bug.c:186
> >>  fixup_bug arch/x86/kernel/traps.c:178 [inline]
> >>  do_error_trap+0x1de/0x490 arch/x86/kernel/traps.c:296
> >>  do_invalid_op+0x1b/0x20 arch/x86/kernel/traps.c:315
> >>  invalid_op+0x14/0x20 arch/x86/entry/entry_64.S:992
> >> RIP: 0010:kernfs_add_one+0x406/0x4d0 fs/kernfs/dir.c:758
> >> RSP: 0018:8801ca9eece0 EFLAGS: 00010286
> >> RAX: 002d RBX: 87d5cee0 RCX: 8160ba7d
> >> RDX:  RSI: 81610731 RDI: 8801ca9ee840
> >> RBP: 8801ca9eed20 R08: 8801d9538500 R09: 0006
> >> R10: 8801d9538500 R11:  R12: 8801ad1cb6c0
> >> R13: 885da640 R14: 0020 R15: 
> >>  kernfs_create_link+0x112/0x180 fs/kernfs/symlink.c:41
> >>  sysfs_do_create_link_sd.isra.2+0x90/0x130 fs/sysfs/symlink.c:43
> >>  sysfs_do_create_link fs/sysfs/symlink.c:79 [inline]
> >>  sysfs_create_link+0x65/0xc0 fs/sysfs/symlink.c:91
> >>  device_add_class_symlinks drivers/base/core.c:1612 [inline]
> >>  device_add+0x7a0/0x16d0 drivers/base/core.c:1810
> >>  wiphy_register+0x178a/0x2430 net/wireless/core.c:806
> >>  ieee80211_register_hw+0x13cd/0x35d0 net/mac80211/main.c:1047
> >>  mac80211_hwsim_new_radio+0x1d9b/0x3410
> >> drivers/net/wireless/mac80211_hwsim.c:2772
> >>  hwsim_new_radio_nl+0x7a7/0xa60 drivers/net/wireless/mac80211_hwsim.c:3246
> >>  genl_family_rcv_msg+0x889/0x1120 net/netlink/genetlink.c:599
> >>  genl_rcv_msg+0xc6/0x170 net/netlink/genetlink.c:624
> >>  netlink_rcv_skb+0x172/0x440 net/netlink/af_netlink.c:2448
> >>  genl_rcv+0x28/0x40 net/netlink/genetlink.c:635
> >>  netlink_unicast_kernel net/netlink/af_netlink.c:1310 [inline]
> >>  netlink_unicast+0x58b/0x740 net/netlink/af_netlink.c:1336
> >>  netlink_sendmsg+0x9f0/0xfa0 net/netlink/af_netlink.c:1901
> >>  sock_sendmsg_nosec net/socket.c:629 [inline]
> >>  sock_sendmsg+0xd5/0x120 n

Re: WARNING in kernfs_add_one

[Greg KH]

On Sat, May 05, 2018 at 08:47:02AM -0700, syzbot wrote:
> Hello,
> 
> syzbot found the following crash on:
> 
> HEAD commit:8fb11a9a8d51 net/ipv6: rename rt6_next to fib6_next
> git tree:   net-next
> console output: https://syzkaller.appspot.com/x/log.txt?x=14b2723780
> kernel config:  https://syzkaller.appspot.com/x/.config?x=c416c61f3cd96be
> dashboard link: https://syzkaller.appspot.com/bug?extid=df47f81c226b31d89fb1
> compiler:   gcc (GCC) 8.0.1 20180413 (experimental)
> syzkaller repro:https://syzkaller.appspot.com/x/repro.syz?x=172fb3e780
> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=16552e5780
> 
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+df47f81c226b31d89...@syzkaller.appspotmail.com
> 
> RBP: 7fff808f3e10 R08: 0002 R09: 7fff80003534
> R10:  R11: 0246 R12: 
> R13: 0006 R14:  R15: 
> [ cut here ]
> kernfs: ns required in 'ieee80211' for 'phy3'

That's interesting, this looks like a netfilter bug (adding netdev to
the report here.)

Yes, we can "tone down" the kernfs warning to just be an error message
in the log, but there might be something worse going on here.

Network developers, any idea?  Rest of the callback chain is here:


> WARNING: CPU: 0 PID: 4538 at fs/kernfs/dir.c:759 kernfs_add_one+0x406/0x4d0
> fs/kernfs/dir.c:758
> Kernel panic - not syncing: panic_on_warn set ...
> 
> CPU: 0 PID: 4538 Comm: syz-executor486 Not tainted 4.17.0-rc3+ #33
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> Google 01/01/2011
> Call Trace:
>  __dump_stack lib/dump_stack.c:77 [inline]
>  dump_stack+0x1b9/0x294 lib/dump_stack.c:113
>  panic+0x22f/0x4de kernel/panic.c:184
>  __warn.cold.8+0x163/0x1b3 kernel/panic.c:536
>  report_bug+0x252/0x2d0 lib/bug.c:186
>  fixup_bug arch/x86/kernel/traps.c:178 [inline]
>  do_error_trap+0x1de/0x490 arch/x86/kernel/traps.c:296
>  do_invalid_op+0x1b/0x20 arch/x86/kernel/traps.c:315
>  invalid_op+0x14/0x20 arch/x86/entry/entry_64.S:992
> RIP: 0010:kernfs_add_one+0x406/0x4d0 fs/kernfs/dir.c:758
> RSP: 0018:8801ca9eece0 EFLAGS: 00010286
> RAX: 002d RBX: 87d5cee0 RCX: 8160ba7d
> RDX:  RSI: 81610731 RDI: 8801ca9ee840
> RBP: 8801ca9eed20 R08: 8801d9538500 R09: 0006
> R10: 8801d9538500 R11:  R12: 8801ad1cb6c0
> R13: 885da640 R14: 0020 R15: 
>  kernfs_create_link+0x112/0x180 fs/kernfs/symlink.c:41
>  sysfs_do_create_link_sd.isra.2+0x90/0x130 fs/sysfs/symlink.c:43
>  sysfs_do_create_link fs/sysfs/symlink.c:79 [inline]
>  sysfs_create_link+0x65/0xc0 fs/sysfs/symlink.c:91
>  device_add_class_symlinks drivers/base/core.c:1612 [inline]
>  device_add+0x7a0/0x16d0 drivers/base/core.c:1810
>  wiphy_register+0x178a/0x2430 net/wireless/core.c:806
>  ieee80211_register_hw+0x13cd/0x35d0 net/mac80211/main.c:1047
>  mac80211_hwsim_new_radio+0x1d9b/0x3410
> drivers/net/wireless/mac80211_hwsim.c:2772
>  hwsim_new_radio_nl+0x7a7/0xa60 drivers/net/wireless/mac80211_hwsim.c:3246
>  genl_family_rcv_msg+0x889/0x1120 net/netlink/genetlink.c:599
>  genl_rcv_msg+0xc6/0x170 net/netlink/genetlink.c:624
>  netlink_rcv_skb+0x172/0x440 net/netlink/af_netlink.c:2448
>  genl_rcv+0x28/0x40 net/netlink/genetlink.c:635
>  netlink_unicast_kernel net/netlink/af_netlink.c:1310 [inline]
>  netlink_unicast+0x58b/0x740 net/netlink/af_netlink.c:1336
>  netlink_sendmsg+0x9f0/0xfa0 net/netlink/af_netlink.c:1901
>  sock_sendmsg_nosec net/socket.c:629 [inline]
>  sock_sendmsg+0xd5/0x120 net/socket.c:639
>  ___sys_sendmsg+0x805/0x940 net/socket.c:2117
>  __sys_sendmsg+0x115/0x270 net/socket.c:2155
>  __do_sys_sendmsg net/socket.c:2164 [inline]
>  __se_sys_sendmsg net/socket.c:2162 [inline]
>  __x64_sys_sendmsg+0x78/0xb0 net/socket.c:2162
>  do_syscall_64+0x1b1/0x800 arch/x86/entry/common.c:287
>  entry_SYSCALL_64_after_hwframe+0x49/0xbe
> RIP: 0033:0x4404c9
> RSP: 002b:7fff808f3e08 EFLAGS: 0246 ORIG_RAX: 002e
> RAX: ffda RBX:  RCX: 004404c9
> RDX:  RSI: 20b3dfc8 RDI: 0005
> RBP: 7fff808f3e10 R08: 0002 R09: 7fff80003534
> R10:  R11: 0246 R12: 
> R13: 0006 R14:  R15: 
> Dumping ftrace buffer:
>(ftrace buffer empty)
> Kernel Offset: disabled
> Rebooting in 86400 seconds..


Any ideas?

thanks,

greg k-h

Re: Request for stable 4.14.x inclusion: net: don't call update_pmtu unconditionally

[Greg KH]

On Tue, May 01, 2018 at 12:15:37AM +0200, Thomas Deutschmann wrote:
> Hi,
> 
> On 2018-04-30 20:22, Greg KH wrote:
> > The geneve hunk doesn't apply at all to the 4.14.y tree, so I think
> > someone has a messed up tree somewhere...
> > 
> > I'll go look into this now.
> 
> Mh?
> 
> > $ git clone 
> > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git
> > $ cd linux-stable
> > $ git checkout v4.14.38
> > $ git cherry-pick 52a589d51f1008f62569bf89e95b26221ee76690
> 
> Works for me... then I cherry-pick
> f15ca723c1ebe6c1a06bc95fda6b62cd87b44559 on top, adjust
> "net/ipv6/ip6_tunnel.c" like shown in my previous mail and everything is
> fine for me...

Ah crap, I missed the dependancy here as well, it was a long day
yesterday...

I'll drop this and try it again for the next release.

greg k-h

Re: Request for stable 4.14.x inclusion: net: don't call update_pmtu unconditionally

[Greg KH]

On Fri, Apr 27, 2018 at 07:43:52PM +0100, Eddie Chapman wrote:
> On 27/04/18 19:07, Thomas Deutschmann wrote:
> > Hi Greg,
> > 
> > first, we need to cherry-pick another patch first:
> > >  From 52a589d51f1008f62569bf89e95b26221ee76690 Mon Sep 17 00:00:00 2001
> > > From: Xin Long 
> > > Date: Mon, 25 Dec 2017 14:43:58 +0800
> > > Subject: [PATCH] geneve: update skb dst pmtu on tx path
> > > 
> > > Commit a93bf0ff4490 ("vxlan: update skb dst pmtu on tx path") has fixed
> > > a performance issue caused by the change of lower dev's mtu for vxlan.
> > > 
> > > The same thing needs to be done for geneve as well.
> > > 
> > > Note that geneve cannot adjust it's mtu according to lower dev's mtu
> > > when creating it. The performance is very low later when netperfing
> > > over it without fixing the mtu manually. This patch could also avoid
> > > this issue.
> > > 
> > > Signed-off-by: Xin Long 
> > > Signed-off-by: David S. Miller 
> 
> Oops, I completely missed that the coreos patch doesn't have the geneve hunk
> that is in the original 4.15 patch. I don't load the geneve module on my box
> hence why no problems surfaced on my machine.

The geneve hunk doesn't apply at all to the 4.14.y tree, so I think
someone has a messed up tree somewhere...

I'll go look into this now.

greg k-h

Re: Request for stable 4.14.x inclusion: net: don't call update_pmtu unconditionally

[Greg KH]

On Fri, Apr 27, 2018 at 07:43:52PM +0100, Eddie Chapman wrote:
> On 27/04/18 19:07, Thomas Deutschmann wrote:
> > Hi Greg,
> > 
> > first, we need to cherry-pick another patch first:
> > >  From 52a589d51f1008f62569bf89e95b26221ee76690 Mon Sep 17 00:00:00 2001
> > > From: Xin Long 
> > > Date: Mon, 25 Dec 2017 14:43:58 +0800
> > > Subject: [PATCH] geneve: update skb dst pmtu on tx path
> > > 
> > > Commit a93bf0ff4490 ("vxlan: update skb dst pmtu on tx path") has fixed
> > > a performance issue caused by the change of lower dev's mtu for vxlan.
> > > 
> > > The same thing needs to be done for geneve as well.
> > > 
> > > Note that geneve cannot adjust it's mtu according to lower dev's mtu
> > > when creating it. The performance is very low later when netperfing
> > > over it without fixing the mtu manually. This patch could also avoid
> > > this issue.
> > > 
> > > Signed-off-by: Xin Long 
> > > Signed-off-by: David S. Miller 
> 
> Oops, I completely missed that the coreos patch doesn't have the geneve hunk
> that is in the original 4.15 patch. I don't load the geneve module on my box
> hence why no problems surfaced on my machine.
> 
> Thanks Thomas for the correct instructions. Ignore my message Greg, I'll
> drop back into the shadows where I belong, sorry for the noise!

Talking about patches and pointing me at them is not noise at all, don't
be sorry! :)

I'll work on this after these next kernels are released, thanks all for
the details on what needs to be done.

greg k-h

Re: Request for stable 4.14.x inclusion: net: don't call update_pmtu unconditionally

[Greg KH]

On Fri, Apr 27, 2018 at 03:51:25PM +0200, Greg KH wrote:
> On Fri, Apr 27, 2018 at 02:20:07PM +0200, Thomas Deutschmann wrote:
> > On 2018-04-22 23:50, Thomas Deutschmann wrote:
> > > Hi,
> > > 
> > > please add
> > > 
> > >> From f15ca723c1ebe6c1a06bc95fda6b62cd87b44559 Mon Sep 17 00:00:00 2001
> > >> From: Nicolas Dichtel <nicolas.dich...@6wind.com>
> > >> Date: Thu, 25 Jan 2018 19:03:03 +0100
> > >> Subject: net: don't call update_pmtu unconditionally
> > >>
> > >> Some dst_ops (e.g. md_dst_ops)) doesn't set this handler. It may result 
> > >> to:
> > >> "BUG: unable to handle kernel NULL pointer dereference at   
> > >> (null)"
> > >>
> > >> Let's add a helper to check if update_pmtu is available before calling 
> > >> it.
> > >>
> > >> Fixes: 52a589d51f10 ("geneve: update skb dst pmtu on tx path")
> > >> Fixes: a93bf0ff4490 ("vxlan: update skb dst pmtu on tx path")
> > >> CC: Roman Kapl <c...@rkapl.cz>
> > >> CC: Xin Long <lucien@gmail.com>
> > >> Signed-off-by: Nicolas Dichtel <nicolas.dich...@6wind.com>
> > >> Signed-off-by: David S. Miller <da...@davemloft.net>
> > > 
> > > to 4.14.x.
> > > 
> > > This fixes NULL derefs caused by a93bf0ff4490 ("vxlan: update
> > > skb dst pmtu on tx path"), which was backported to 4.14.24.
> > 
> > *ping* - Not yet applied and not yet queued. Is there a problem with the
> > patch which prevents a cherry-pick for 4.14.x?
> 
> This looks like an "obvious" fix for me to pick up.

Well, it would be "obvious" if it actually applied to the 4.14.y tree :(

Thomas, did you try this patch out?  I can't apply it as-is, it will
need a backport.  Please work on that, and test it out, as I don't get
the impression that you did that here.

Then post the working backport and I'll be glad to consider it for
future 4.14.y releases.

thanks,

greg k-h

Re: Request for stable 4.14.x inclusion: net: don't call update_pmtu unconditionally

[Greg KH]

On Fri, Apr 27, 2018 at 02:20:07PM +0200, Thomas Deutschmann wrote:
> On 2018-04-22 23:50, Thomas Deutschmann wrote:
> > Hi,
> > 
> > please add
> > 
> >> From f15ca723c1ebe6c1a06bc95fda6b62cd87b44559 Mon Sep 17 00:00:00 2001
> >> From: Nicolas Dichtel 
> >> Date: Thu, 25 Jan 2018 19:03:03 +0100
> >> Subject: net: don't call update_pmtu unconditionally
> >>
> >> Some dst_ops (e.g. md_dst_ops)) doesn't set this handler. It may result to:
> >> "BUG: unable to handle kernel NULL pointer dereference at   (null)"
> >>
> >> Let's add a helper to check if update_pmtu is available before calling it.
> >>
> >> Fixes: 52a589d51f10 ("geneve: update skb dst pmtu on tx path")
> >> Fixes: a93bf0ff4490 ("vxlan: update skb dst pmtu on tx path")
> >> CC: Roman Kapl 
> >> CC: Xin Long 
> >> Signed-off-by: Nicolas Dichtel 
> >> Signed-off-by: David S. Miller 
> > 
> > to 4.14.x.
> > 
> > This fixes NULL derefs caused by a93bf0ff4490 ("vxlan: update
> > skb dst pmtu on tx path"), which was backported to 4.14.24.
> 
> *ping* - Not yet applied and not yet queued. Is there a problem with the
> patch which prevents a cherry-pick for 4.14.x?

This looks like an "obvious" fix for me to pick up.

Dave, any objections for me just grabbing it as-is?

thanks,

greg k-h

Re: [PATCH stable v4.4+] r8152: add Linksys USB3GIGV1 id

[Greg KH]

On Wed, Apr 25, 2018 at 11:54:37AM +0200, Krzysztof Kozlowski wrote:
> commit 90841047a01b452cc8c3f9b990698b264143334a upstream
> 
> This linksys dongle by default comes up in cdc_ether mode.
> This patch allows r8152 to claim the device:
>Bus 002 Device 002: ID 13b1:0041 Linksys
> 
> Signed-off-by: Grant Grundler 
> Reviewed-by: Douglas Anderson 
> Signed-off-by: David S. Miller 
> [krzk: Rebase on v4.4]

Also added to 4.9.y, thanks.

greg k-h

Re: [PATCH net-next v4 1/3] vmcore: add API to collect hardware dump in second kernel

[Greg KH]

On Tue, Apr 17, 2018 at 01:14:17PM +0530, Rahul Lakkireddy wrote:
> +config PROC_VMCORE_DEVICE_DUMP
> + bool "Device Hardware/Firmware Log Collection"
> + depends on PROC_VMCORE
> + default y

Only things that require the machine to keep working should be 'default
y', please remove this, it's an option.

> + help
> +   Device drivers can collect the device specific snapshot of
> +   their hardware or firmware before they are initialized in
> +   crash recovery kernel. If you say Y here, the device dumps
> +   will be added as ELF notes to /proc/vmcore

Which exact "device drivers" are you referring to here?

thanks,

greg k-h

Re: [PATCH 1/2] staging: irda: Replace mdelay with usleep_range in stir421x_fw_upload

[Greg KH]

On Wed, Apr 11, 2018 at 04:11:00PM +0800, Jia-Ju Bai wrote:
> 
> 
> On 2018/4/11 16:03, Greg KH wrote:
> > On Wed, Apr 11, 2018 at 03:17:10PM +0800, Jia-Ju Bai wrote:
> > > 
> > > On 2018/4/11 14:41, Greg KH wrote:
> > > > On Wed, Apr 11, 2018 at 09:29:34AM +0800, Jia-Ju Bai wrote:
> > > > > stir421x_fw_upload() is never called in atomic context.
> > > > > 
> > > > > The call chain ending up at stir421x_fw_upload() is:
> > > > > [1] stir421x_fw_upload() <- stir421x_patch_device() <- 
> > > > > irda_usb_probe()
> > > > > 
> > > > > irda_usb_probe() is set as ".probe" in struct usb_driver.
> > > > > This function is not called in atomic context.
> > > > > 
> > > > > Despite never getting called from atomic context, stir421x_fw_upload()
> > > > > calls mdelay() to busily wait.
> > > > > This is not necessary and can be replaced with usleep_range() to
> > > > > avoid busy waiting.
> > > > > 
> > > > > This is found by a static analysis tool named DCNS written by myself.
> > > > > And I also manually check it.
> > > > > 
> > > > > Signed-off-by: Jia-Ju Bai <baijiaju1...@gmail.com>
> > > > > ---
> > > > >drivers/staging/irda/drivers/irda-usb.c | 2 +-
> > > > >1 file changed, 1 insertion(+), 1 deletion(-)
> > > > Please, at the very least, work off of Linus's tree.  There is no
> > > > drivers/staging/irda/ anymore :)
> > > > 
> > > Okay, sorry.
> > > Could you please recommend me a right tree or its git address?
> > Have you looked in the MAINTAINERS file?  Worst case, always use
> > linux-next.
> > 
> > greg k-h
> 
> Oh, sorry, I did notice the git tree in the MAINTAINERS file.
> I always used linux-stable.

linux-stable is almost never the tree to use as it is almost always
12000 patches behind what is in Linus's tree and about 2 changes
behind linux-next.

thanks,

greg k-h

Re: [PATCH 1/2] staging: irda: Replace mdelay with usleep_range in stir421x_fw_upload

[Greg KH]

On Wed, Apr 11, 2018 at 03:17:10PM +0800, Jia-Ju Bai wrote:
> 
> 
> On 2018/4/11 14:41, Greg KH wrote:
> > On Wed, Apr 11, 2018 at 09:29:34AM +0800, Jia-Ju Bai wrote:
> > > stir421x_fw_upload() is never called in atomic context.
> > > 
> > > The call chain ending up at stir421x_fw_upload() is:
> > > [1] stir421x_fw_upload() <- stir421x_patch_device() <- irda_usb_probe()
> > > 
> > > irda_usb_probe() is set as ".probe" in struct usb_driver.
> > > This function is not called in atomic context.
> > > 
> > > Despite never getting called from atomic context, stir421x_fw_upload()
> > > calls mdelay() to busily wait.
> > > This is not necessary and can be replaced with usleep_range() to
> > > avoid busy waiting.
> > > 
> > > This is found by a static analysis tool named DCNS written by myself.
> > > And I also manually check it.
> > > 
> > > Signed-off-by: Jia-Ju Bai <baijiaju1...@gmail.com>
> > > ---
> > >   drivers/staging/irda/drivers/irda-usb.c | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > Please, at the very least, work off of Linus's tree.  There is no
> > drivers/staging/irda/ anymore :)
> > 
> 
> Okay, sorry.
> Could you please recommend me a right tree or its git address?

Have you looked in the MAINTAINERS file?  Worst case, always use
linux-next.

greg k-h

Re: [PATCH 1/2] staging: irda: Replace mdelay with usleep_range in stir421x_fw_upload

[Greg KH]

On Wed, Apr 11, 2018 at 09:29:34AM +0800, Jia-Ju Bai wrote:
> stir421x_fw_upload() is never called in atomic context.
> 
> The call chain ending up at stir421x_fw_upload() is:
> [1] stir421x_fw_upload() <- stir421x_patch_device() <- irda_usb_probe()
> 
> irda_usb_probe() is set as ".probe" in struct usb_driver.
> This function is not called in atomic context.
> 
> Despite never getting called from atomic context, stir421x_fw_upload()
> calls mdelay() to busily wait.
> This is not necessary and can be replaced with usleep_range() to
> avoid busy waiting.
> 
> This is found by a static analysis tool named DCNS written by myself.
> And I also manually check it.
> 
> Signed-off-by: Jia-Ju Bai 
> ---
>  drivers/staging/irda/drivers/irda-usb.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Please, at the very least, work off of Linus's tree.  There is no
drivers/staging/irda/ anymore :)

thanks,

greg k-h

Re: [PATCH 12/28] net: convert datagram_poll users tp ->poll_mask

[Greg KH]

On Wed, Mar 21, 2018 at 08:40:16AM +0100, Christoph Hellwig wrote:
> Signed-off-by: Christoph Hellwig 
> ---
>  drivers/isdn/mISDN/socket.c|  2 +-
>  drivers/net/ppp/pppoe.c|  2 +-
>  drivers/staging/ipx/af_ipx.c   |  2 +-
>  drivers/staging/irda/net/af_irda.c |  6 +++---

irda is now gone in linux-next, but that's an easy thing to handle when
merging.

Reviewed-by: Greg Kroah-Hartman 

Re: [PATCH 28/28] random: convert to ->poll_mask

[Greg KH]

On Wed, Mar 21, 2018 at 08:40:32AM +0100, Christoph Hellwig wrote:
> The big change is that random_read_wait and random_write_wait are merged
> into a single waitqueue that uses keyed wakeups.  Because wait_event_*
> doesn't know about that this will lead to occassional spurious wakeups
> in _random_read and add_hwgenerator_randomness, but wait_event_* is
> designed to handle these and were are not in a a hot path there.
> 
> Signed-off-by: Christoph Hellwig 
> ---
>  drivers/char/random.c | 27 +++
>  1 file changed, 15 insertions(+), 12 deletions(-)

Reviewed-by: Greg Kroah-Hartman 

Re: [PATCH 06/28] aio: implement IOCB_CMD_POLL

[Greg KH]

On Wed, Mar 21, 2018 at 08:40:10AM +0100, Christoph Hellwig wrote:
> Simple one-shot poll through the io_submit() interface.  To poll for
> a file descriptor the application should submit an iocb of type
> IOCB_CMD_POLL.  It will poll the fd for the events specified in the
> the first 32 bits of the aio_buf field of the iocb.
> 
> Unlike poll or epoll without EPOLLONESHOT this interface always works
> in one shot mode, that is once the iocb is completed, it will have to be
> resubmitted.
> 
> Signed-off-by: Christoph Hellwig 
> Acked-by: Jeff Moyer 

Reviewed-by: Greg Kroah-Hartman 

Re: [PATCH 05/28] fs: introduce new ->get_poll_head and ->poll_mask methods

[Greg KH]

On Wed, Mar 21, 2018 at 08:40:09AM +0100, Christoph Hellwig wrote:
> ->get_poll_head returns the waitqueue that the poll operation is going
> to sleep on.  Note that this means we can only use a single waitqueue
> for the poll, unlike some current drivers that use two waitqueues for
> different events.  But now that we have keyed wakeups and heavily use
> those for poll there aren't that many good reason left to keep the
> multiple waitqueues, and if there are any ->poll is still around, the
> driver just won't support aio poll.
> 
> Signed-off-by: Christoph Hellwig 

Reviewed-by: Greg Kroah-Hartman 

Re: [PATCH 03/28] fs: update documentation to mention __poll_t

[Greg KH]

On Wed, Mar 21, 2018 at 08:40:07AM +0100, Christoph Hellwig wrote:
> Signed-off-by: Christoph Hellwig 

Reviewed-by: Greg Kroah-Hartman 

Re: [PATCH 04/28] fs: add new vfs_poll and file_can_poll helpers

[Greg KH]

On Wed, Mar 21, 2018 at 08:40:08AM +0100, Christoph Hellwig wrote:
> These abstract out calls to the poll method in preparation for changes
> in how we poll.

Yeah!!!

> 
> Signed-off-by: Christoph Hellwig 
> Reviewed-by: Darrick J. Wong 

Reviewed-by: Greg Kroah-Hartman 

Re: [PATCH 02/28] fs: cleanup do_pollfd

[Greg KH]

On Wed, Mar 21, 2018 at 08:40:06AM +0100, Christoph Hellwig wrote:
> Use straigline code with failure handling gotos instead of a lot

"straightline"?  :)

> of nested conditionals.
> 
> Signed-off-by: Christoph Hellwig 
> Reviewed-by: Darrick J. Wong 

Reviewed-by: Greg Kroah-Hartman 

Re: [PATCH 01/28] fs: unexport poll_schedule_timeout

[Greg KH]

On Wed, Mar 21, 2018 at 08:40:05AM +0100, Christoph Hellwig wrote:
> No users outside of select.c.
> 
> Signed-off-by: Christoph Hellwig 
> Reviewed-by: Darrick J. Wong 

Reviewed-by: Greg Kroah-Hartman 

Re: [PATCH v5 1/6] staging: fsl-dpaa2/ethsw: Add APIs for DPSW object

[Greg KH]

On Tue, Mar 13, 2018 at 08:51:51AM -0500, Razvan Stefanescu wrote:
> Add the command build/parse APIs for operating on DPSW objects through
> the DPAA2 Management Complex.
> 
> Signed-off-by: Razvan Stefanescu 
> ---
> Changelog:
>  v2:
> - use u8 for en parameter of dpsw_if_set_flooding/broadcast()
>  v3:
> - no changes
>  v4:
> - adjust to moving MC-bus out of staging
> - fix sparse warnings
>  v5:
>- no changes
> 
>  drivers/staging/fsl-dpaa2/Kconfig  |8 +
>  drivers/staging/fsl-dpaa2/Makefile |1 +
>  drivers/staging/fsl-dpaa2/ethsw/Makefile   |7 +
>  drivers/staging/fsl-dpaa2/ethsw/dpsw-cmd.h |  358 +
>  drivers/staging/fsl-dpaa2/ethsw/dpsw.c | 1116 
> 
>  drivers/staging/fsl-dpaa2/ethsw/dpsw.h |  579 +++
>  6 files changed, 2069 insertions(+)
>  create mode 100644 drivers/staging/fsl-dpaa2/ethsw/Makefile
>  create mode 100644 drivers/staging/fsl-dpaa2/ethsw/dpsw-cmd.h
>  create mode 100644 drivers/staging/fsl-dpaa2/ethsw/dpsw.c
>  create mode 100644 drivers/staging/fsl-dpaa2/ethsw/dpsw.h
> 
> diff --git a/drivers/staging/fsl-dpaa2/Kconfig 
> b/drivers/staging/fsl-dpaa2/Kconfig
> index dfff675..8a508ef 100644
> --- a/drivers/staging/fsl-dpaa2/Kconfig
> +++ b/drivers/staging/fsl-dpaa2/Kconfig
> @@ -16,3 +16,11 @@ config FSL_DPAA2_ETH
>   ---help---
> Ethernet driver for Freescale DPAA2 SoCs, using the
> Freescale MC bus driver
> +
> +config FSL_DPAA2_ETHSW
> + tristate "Freescale DPAA2 Ethernet Switch"
> + depends on FSL_DPAA2
> + depends on NET_SWITCHDEV
> + ---help---
> + Driver for Freescale DPAA2 Ethernet Switch. Select
> + BRIDGE to have support for bridge tools.
> diff --git a/drivers/staging/fsl-dpaa2/Makefile 
> b/drivers/staging/fsl-dpaa2/Makefile
> index 0836ba8..6cfd76b 100644
> --- a/drivers/staging/fsl-dpaa2/Makefile
> +++ b/drivers/staging/fsl-dpaa2/Makefile
> @@ -3,3 +3,4 @@
>  #
>  
>  obj-$(CONFIG_FSL_DPAA2_ETH)  += ethernet/
> +obj-$(CONFIG_FSL_DPAA2_ETHSW)+= ethsw/
> diff --git a/drivers/staging/fsl-dpaa2/ethsw/Makefile 
> b/drivers/staging/fsl-dpaa2/ethsw/Makefile
> new file mode 100644
> index 000..db137f7
> --- /dev/null
> +++ b/drivers/staging/fsl-dpaa2/ethsw/Makefile
> @@ -0,0 +1,7 @@
> +#
> +# Makefile for the Freescale DPAA2 Ethernet Switch
> +#
> +
> +obj-$(CONFIG_FSL_DPAA2_ETHSW) += dpaa2-ethsw.o
> +
> +dpaa2-ethsw-objs := dpsw.o
> diff --git a/drivers/staging/fsl-dpaa2/ethsw/dpsw-cmd.h 
> b/drivers/staging/fsl-dpaa2/ethsw/dpsw-cmd.h
> new file mode 100644
> index 000..36edef6
> --- /dev/null
> +++ b/drivers/staging/fsl-dpaa2/ethsw/dpsw-cmd.h
> @@ -0,0 +1,358 @@
> +/* Copyright 2013-2016 Freescale Semiconductor Inc.
> + * Copyright 2017-2018 NXP
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions are 
> met:
> + * * Redistributions of source code must retain the above copyright
> + *notice, this list of conditions and the following disclaimer.
> + * * Redistributions in binary form must reproduce the above copyright
> + *notice, this list of conditions and the following disclaimer in the
> + *documentation and/or other materials provided with the distribution.
> + * * Neither the name of the above-listed copyright holders nor the
> + *names of any contributors may be used to endorse or promote products
> + *derived from this software without specific prior written permission.
> + *
> + *
> + * ALTERNATIVELY, this software may be distributed under the terms of the
> + * GNU General Public License ("GPL") as published by the Free Software
> + * Foundation, either version 2 of that License or (at your option) any
> + * later version.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS 
> IS"
> + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDERS OR CONTRIBUTORS BE
> + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
> + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
> + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
> + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
> + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
> + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
> + * POSSIBILITY OF SUCH DAMAGE.
> + */

Can you resend this series and just use the correct SPDX identifiers for
all of the new files, instead of all of this horrid boiler-plate code?

That will save me time when I have to go delete all of this in the near
future :)

Also, why dual license it?  Are you _SURE_ you want to do that, and are
totally 

Re: [PATCH net-next] modules: allow modprobe load regular elf binaries

[Greg KH]

On Fri, Mar 09, 2018 at 10:23:27AM -0800, Andy Lutomirski wrote:
> 
> 
> > On Mar 9, 2018, at 10:15 AM, Greg KH <gre...@linuxfoundation.org> wrote:
> > 
> 
> > 
> > Oh, and for the record, I like Andy's proposal as well as dumping this
> > into a kernel module "blob" with the exception that this now would take
> > up unswapable memory, which isn't the nicest and is one big reason we
> > removed the in-kernel-memory firmware blobs many years ago.
> > 
> 
> It might not be totally crazy to back it by tmpfs.

Ah, yeah, tricky, but yes, I'd be fine with that.

Micro-kernel here we come!  :)

Re: [PATCH net-next] modules: allow modprobe load regular elf binaries

[Greg KH]

On Fri, Mar 09, 2018 at 09:32:36AM -0800, Alexei Starovoitov wrote:
> On 3/9/18 8:24 AM, Andy Lutomirski wrote:
> > On Fri, Mar 9, 2018 at 3:39 PM, Alexei Starovoitov  wrote:
> > > On 3/9/18 7:16 AM, Andy Lutomirski wrote:
> > > > > > 
> > > > > > On Mar 8, 2018, at 9:08 PM, Alexei Starovoitov  wrote:
> > > > > > 
> > > > > > On 3/8/18 7:54 PM, Andy Lutomirski wrote:
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > > On Mar 8, 2018, at 7:06 PM, Linus Torvalds
> > > > > > >  wrote:
> > > > > > > 
> > > > > > > 
> > > > > > > Honestly, that "read twice" thing may be what scuttles this.
> > > > > > > Initially, I thought it was a non-issue, because anybody who 
> > > > > > > controls
> > > > > > > the module subdirectory enough to rewrite files would be in a 
> > > > > > > position
> > > > > > > to just execute the file itself directly instead.
> > > > > > 
> > > > > > 
> > > > > > On further consideration, I think there’s another showstopper. This
> > > > > > patch is a potentially severe ABI break. Right now, loading a module
> > > > > > *copies* it into memory and does not hold a reference to the 
> > > > > > underlying fs.
> > > > > > With the patch applied, all kinds of use cases can break in gnarly 
> > > > > > ways.
> > > > > > Initramfs is maybe okay, but initrd may be screwed. If you load an 
> > > > > > ET_EXEC
> > > > > > module from initrd, then umount it, then clear the ramdisk, 
> > > > > > something will
> > > > > > go horribly wrong. Exactly what goes wrong depends on whether 
> > > > > > userspace
> > > > > > notices that umount() failed. Similarly, if you load one of these 
> > > > > > modules
> > > > > > over a network and then lose your connection, you have a problem.
> > > > > 
> > > > > 
> > > > > there is not abi breakage and file cannot disappear from running task.
> > > > > One cannot umount fs while file is still being used.
> > > > 
> > > > 
> > > > Sure it is. Without your patch, init_module doesn’t keep using the
> > > > file, so it’s common practice to load a module and then delete or
> > > > unmount it. With your patch, the unmount case breaks. This is likely
> > > > to break existing userspace, so, in Linux speak it’s an ABI break.
> > > 
> > > 
> > > please read the patch again.
> > > file is only used in case of umh modules.
> > > There is zero difference in default case.
> > 
> > Say I'm running some distro or other working Linux setup.  I upgrade
> > my kernel to a kernel that uses umh modules.  The user tooling
> > generates some kind of boot entry that references the new kernel
> > image, and it also generates a list of modules to be loaded at various
> > times in the boot process.  This list might, and probably should,
> > include one or more umh modules.  (You are being very careful to make
> > sure that depmod keeps working, so umh modules are clearly intended to
> > work with existing tooling.)  So now I have a kernel image and some
> > modules to be loaded from various places.  And I have an init script
> > (initramfs's '/init' or similar) that will call init_module() on that
> > .ko file.  That script was certainly written under the assumption
> > that, once init_module() returns, the kernel is done with the .ko
> > file.  With your patch applied, that assumption is no longer true.
> 
> There is no intent to use umh modules during boot process.

For _your_ use case, yes.  For mine and Andy's and someone else's in the
future, it might be :)

You are creating a very generic, new, user/kernel api that a whole bunch
of people are going to want to use.  Let's not hamper the ability for us
all to use this right from the beginning please.

> This is not a replacement for drivers and kernel modules.
> From your earlier comments regarding usb driver as umh module
> I suspect you're assuming that everything will sooner or later
> will convert to umh model.

We have userspace drivers for USB today, being able to drag that
out-of-tree codebase into the kernel is a _HUGE_ bonus, and something
that I would love to do for a lot of reasons.  I also can see moving
some of our existing in-kernel drivers out of the kernel in a way that
provides "it just works" functionality by using this type of feature.

So again, please don't prevent us from using this, otherwise you should
be just calling this "bpf_usermode_helper" :)

Oh, and for the record, I like Andy's proposal as well as dumping this
into a kernel module "blob" with the exception that this now would take
up unswapable memory, which isn't the nicest and is one big reason we
removed the in-kernel-memory firmware blobs many years ago.

thanks,

greg k-h

Re: [PATCH net-next 2/2] net: cdc_eem: clean up bind error path

[Greg KH]

On Wed, Mar 07, 2018 at 10:46:58AM +0100, Johan Hovold wrote:
> Drop bogus call to usb_driver_release_interface() from an error path in
> the usbnet bind() callback, which is called during interface probe. At
> this point the interface is not bound and usb_driver_release_interface()
> returns early.
> 
> Also remove the bogus call to clear the interface data, which is owned
> by the usbnet driver and would not even have been set by the time bind()
> is called.
> 
> Signed-off-by: Johan Hovold 

Reviewed-by: Greg Kroah-Hartman 

Re: [PATCH net-next 1/2] net: kalmia: clean up bind error path

[Greg KH]

On Wed, Mar 07, 2018 at 10:46:57AM +0100, Johan Hovold wrote:
> Drop bogus call to usb_driver_release_interface() from an error path in
> the usbnet bind() callback, which is called during interface probe. At
> this point the interface is not bound and usb_driver_release_interface()
> returns early.
> 
> Also remove the bogus call to clear the interface data, which is owned
> by the usbnet driver and would not even have been set by the time bind()
> is called.
> 
> Signed-off-by: Johan Hovold 

Reviewed-by: Greg Kroah-Hartman 

Re: [PATCH net-next] modules: allow modprobe load regular elf binaries

[Greg KH]

On Tue, Mar 06, 2018 at 05:07:45PM -0800, Alexei Starovoitov wrote:
> combining multiple answers...
> 
> On 3/6/18 3:05 AM, Greg KH wrote:
> > 
> > Any chance you can add a field to your "umh module" type such that a
> > normal 'modinfo' program will be able to notice it is different easily?
> 
> ok. handling of modinfo turned out to be straightforward.
> kmod tooling worked fine with simple addition of .modinfo section.
> 
> $ modinfo bpfilter
> filename:
> /lib/modules/4.16.0-rc4-00799-g1716f0aa3039-dirty/net/bpfilter/bpfilter.ko
> umh:Y

Nice.  But perhaps spell it out, "user_mode_helper"?  Anyway,
bikesheding now, sorry, whatever you want to call it is fine with me.

> license:GPL
> 
> I will require umh=Y and license to be present.
> umh has to be set to Y for this 'umh modules'
> and taint of kernel will happen if license is not gpl.

Interesting, I like it :)


> Other modinfo like vermagic are not applicable here, since
> umh modules interact with kernel via normal kernel/user abi.

Very true.

> > > Since umh can crash, can be oom-ed by the kernel, killed by admin,
> > > the subsystem that uses them (like bpfilter) need to manage life
> > > time of umh on its own, so module infra doesn't do any accounting
> > > of them. They don't appear in "lsmod" and cannot be "rmmod".
> > > Multiple request_module("umh") will load multiple umh.ko processes.
> > > 
> > > Similar to kernel modules the kernel will be tainted if "umh module"
> > > has invalid signature.
> > 
> > Shouldn't we fail to load the "module" if the signature is not valid if
> > CONFIG_MODULE_SIG_FORCE=y is enabled, like we do for modules?  I run my
> > systems like that, and just "warning" isn't probably a good idea for
> > systems that want to enforce that everything is signed properly?
> 
> CONFIG_MODULE_SIG_FORCE=y is already handled by this patch.
> It's checked first for either .ko or umh.ko (before any elf parsing)
> and returns -ENOKEY to user space without any dmesg message.
> I think it's best to keep it as-is.
> The taint and warning is for 'undef SIG_FORCE' and when module
> is signed, but incorrectly.

Ah, sorry, I missed that, thanks for clearing it up.

> > Other than that, one minor question:
> > 
> > > @@ -1745,7 +1745,9 @@ static int do_execveat_common(int fd, struct 
> > > filename *filename,
> > >   sched_exec();
> > > 
> > >   bprm->file = file;
> > > - if (fd == AT_FDCWD || filename->name[0] == '/') {
> > > + if (!filename) {
> > > + bprm->filename = "/dev/null";
> > 
> > Why the use of "/dev/null" for the filename here, and elsewhere in the
> > code?  While I'm "sure" that everyone really does have /dev/null/
> > mounted in the root namespace, what is the use of it here?
> 
> filename is assumed to be non-null in several places further
> down and instead of hacking it everywhere it's cleaner to assign
> some string to it.
> I'll change it to filename = "none"
> Same in umh part.

Thanks, that makes sense.

greg k-h

Re: [PATCH v2] staging: Replace printk() with appropriate net_*macro_ratelimited()

[Greg KH]

On Mon, Mar 05, 2018 at 02:19:11AM +0530, Arushi Singhal wrote:
> Replace printk having a log level with the appropriate
> net_*macro_ratelimited.
> It's better to use actual device name as a prefix in error messages.
> Indentation is also changed, to fix the  checkpatch issue.
> 
> Signed-off-by: Arushi Singhal 
> ---
> changes in v2
> *In previous version printk was changed to pr_*macro(), which is used
> in kernel instead of calling printk() directly. And for drivers,
> dev_*macro() or net_*macro_ratelimited() should be used for calling
> printk() directly.
> 
>  drivers/staging/ipx/af_ipx.c | 14 +++---

Please read drivers/staging/ipx/TODO

thanks,

greg k-h

Re: [PATCH net-next] modules: allow modprobe load regular elf binaries

[Greg KH]

On Mon, Mar 05, 2018 at 05:34:57PM -0800, Alexei Starovoitov wrote:
> As the first step in development of bpfilter project [1] the request_module()
> code is extended to allow user mode helpers to be invoked. Idea is that
> user mode helpers are built as part of the kernel build and installed as
> traditional kernel modules with .ko file extension into distro specified
> location, such that from a distribution point of view, they are no different
> than regular kernel modules. Thus, allow request_module() logic to load such
> user mode helper (umh) modules via:
> 
>   request_module("foo") ->
> call_umh("modprobe foo") ->
>   sys_finit_module(FD of /lib/modules/.../foo.ko) ->
> call_umh(struct file)
> 
> Such approach enables kernel to delegate functionality traditionally done
> by kernel modules into user space processes (either root or !root) and
> reduces security attack surface of such new code, meaning in case of
> potential bugs only the umh would crash but not the kernel. Another
> advantage coming with that would be that bpfilter.ko can be debugged and
> tested out of user space as well (e.g. opening the possibility to run
> all clang sanitizers, fuzzers or test suites for checking translation).
> Also, such architecture makes the kernel/user boundary very precise:
> control plane is done by the user space while data plane stays in the kernel.
> 
> It's easy to distinguish "umh module" from traditional kernel module:
> 
> $ readelf -h bld/net/bpfilter/bpfilter.ko|grep Type
>   Type:  EXEC (Executable file)
> $ readelf -h bld/net/ipv4/netfilter/iptable_filter.ko |grep Type
>   Type:  REL (Relocatable file)

Any chance you can add a field to your "umh module" type such that a
normal 'modinfo' program will be able to notice it is different easily?

> Since umh can crash, can be oom-ed by the kernel, killed by admin,
> the subsystem that uses them (like bpfilter) need to manage life
> time of umh on its own, so module infra doesn't do any accounting
> of them. They don't appear in "lsmod" and cannot be "rmmod".
> Multiple request_module("umh") will load multiple umh.ko processes.
> 
> Similar to kernel modules the kernel will be tainted if "umh module"
> has invalid signature.

Shouldn't we fail to load the "module" if the signature is not valid if
CONFIG_MODULE_SIG_FORCE=y is enabled, like we do for modules?  I run my
systems like that, and just "warning" isn't probably a good idea for
systems that want to enforce that everything is signed properly?

Other than that, one minor question:

> @@ -1745,7 +1745,9 @@ static int do_execveat_common(int fd, struct filename 
> *filename,
>   sched_exec();
>  
>   bprm->file = file;
> - if (fd == AT_FDCWD || filename->name[0] == '/') {
> + if (!filename) {
> + bprm->filename = "/dev/null";

Why the use of "/dev/null" for the filename here, and elsewhere in the
code?  While I'm "sure" that everyone really does have /dev/null/
mounted in the root namespace, what is the use of it here?

Also, what "namespace" does this usermode helper run in?  I'm guessing
the "root" one, which is fine with me, but note that people have
complained in the past about other UMH running in that namespace and not
in the specific namespace of the "container" that they wanted it to run
in.

Anyway, this is crazy stuff, but I like the idea and have no objection
to it overall :)

thanks,

greg k-h

Re: [PATCH v3] staging: ipx: Replace printk() with appropriate net_*macro_ratelimited()

[Greg KH]

On Mon, Mar 05, 2018 at 09:47:40AM +0530, Arushi Singhal wrote:
> Replace printk having a log level with the appropriate
> net_*macro_ratelimited.
> It's better to use actual device name as a prefix in error messages.
> 
> Signed-off-by: Arushi Singhal 
> ---
> changes in v2
> *In v1 printk was changed to pr_*macro(), which is used
> in kernel instead of calling printk() directly. And for drivers,
> dev_*macro() or net_*macro_ratelimited() should be used for calling
> printk() directly.
> 
> changes in v3
> *Indentation is not changed, as line is exceeding 80 characters limit.
> 
>  drivers/staging/ipx/af_ipx.c | 2 +-

Did you read drivers/staging/ipx/TODO?

Please go do so.

sorry,

greg k-h

Re: [PATCH] staging: ipx: Replace printk() with appropriate pr_*() macro

[Greg KH]

On Sun, Mar 04, 2018 at 02:29:35PM +0530, Arushi Singhal wrote:
> Using pr_() is more concise than printk(KERN_).
> Replace printks having a log level with the appropriate pr_*() macros.
> 
> Signed-off-by: Arushi Singhal 
> ---
>  drivers/staging/ipx/af_ipx.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/ipx/af_ipx.c b/drivers/staging/ipx/af_ipx.c
> index d21a9d1..27f4461 100644
> --- a/drivers/staging/ipx/af_ipx.c
> +++ b/drivers/staging/ipx/af_ipx.c
> @@ -744,7 +744,7 @@ static void ipxitf_discover_netnum(struct ipx_interface 
> *intrfc,
>   intrfc->if_netnum = cb->ipx_source_net;
>   ipxitf_add_local_route(intrfc);
>   } else {
> - printk(KERN_WARNING "IPX: Network number collision "
> + pr_warn("IPX: Network number collision "

It is a driver, so it would be best to use dev_warn() or even better
yet, net_warn().  Please try to make that change instead.

thanks,

greg k-h

Re: [Patch 4.14 0/4] net_sched: backport tc filter fixes to 4.14

[Greg KH]

On Thu, Mar 01, 2018 at 08:56:17PM -0500, David Miller wrote:
> From: Cong Wang 
> Date: Thu,  1 Mar 2018 13:46:35 -0800
> 
> > This patchset backports 4 important bug fixes for tc filter to
> > 4.14 stable branch. Due to some big changes between 4.14 and 4.15,
> > the backports are not trivial, I have to adjust and fix the conflicts
> > manually.
> > 
> > Thanks to Roland for reporting the kernel warning and testing
> > the patches.
> > 
> > Reported-by: Roland Franke 
> > Tested-by: Roland Franke 
> > Cc: Jiri Pirko 
> > Cc: Roman Kapl 
> > Cc: David S. Miller 
> > Signed-off-by: Cong Wang 
> 
> Greg, please queue up this series for -stable.

All now applied, thanks.

greg k-h

Re: USB rndis_host - slow download transfers, RX errors

[Greg KH]

On Thu, Feb 08, 2018 at 10:53:20AM -0500, Tomasz Janowski wrote:
> On Thursday, February 8, 2018 3:43:05 PM EST Greg KH wrote:
> > On Thu, Feb 08, 2018 at 02:16:08PM +, Tomasz Janowski, Ph.D. wrote:
> > > Dear USB developers,
> > > 
> > > Based on my google research, the problem I experience seems to happen
> > > with some newer smartphones. My test case is Samsung Galaxy S8 (SM-950U1).
> > > I am trying to use USB tethering and everything seems to work as expected
> > > (modules are loaded, Ethernet devices are up and running, dhcp works
> > > fine). I can connect to the external world using both LTE or wireless
> > > network on the phone.
> > > 
> > > Now, the problem is that the download speeds are terrible, around 64 KB/s,
> > > while uploads are fast, the order of 15 MB/s. These speeds do not depend
> > > on the wireless service provider: the results are similar when I tether
> > > wi-fi. The USB Ethernet interface on the Linux host reports a lot of
> > > receive errors (attached: device_state.txt), while kernel reports bad
> > > rndis messages (attached: kernel.log.txt).
> > > 
> > > Windows 10 works great with the same hardware (same PC and same phone),
> > > with uploads and downloads in the order of 150 Mbit/s, which is probably
> > > as fast as my wireless network can do. But some people reported issues
> > > with older Windows drivers too. Is possible that some newer version of
> > > RNDIS protocol is around and Linux hasn't updated its RNDIS module yet?
> > 
> > Hey, I was _just_ talking to someone at Google about this same issue
> > yesterday, you beat him sending this same type of report to the mailing
> > list, nice job :)
> > 
> > Yes, this is not good, and we should work to resolve this, but first,
> > what kernel version are you using?  I think some fixes for the rndis
> > driver went in recently to 4.15, but it would be good to verify that
> > this isn't already resolved.
> 
> The error messages which I have attached were produced by a precompiled 
> Debian 
> kernel: "Linux version 4.14.0-0.bpo.3-amd64 (debian-ker...@lists.debian.org) 
> (gcc version 6.3.0 20170516 (Debian 6.3.0-18)) #1 SMP Debian 4.14.13-1~bpo9+1 
> (2018-01-14)".
> 
> But I have downloaded the most recent version of the kernel from the official 
> git repository (last commit: Jan 31, 2018) and it had exactly the same 
> problem. Unless a patch was submitted within the last week, the issue is 
> still 
> there.
> 
> Should I get the version as of today and test it again?

If you find a 4.15 tree, that would be great to test, but odds are, the
issues are still there.

I'll try to carve out some time to look at this tomorrow, as I have a
bunch of Android devices to test with, and there's no good reason why
Windows should be slower than Linux for stuff like this.  We should be
able to go as fast as the device lets us.  Most likely we are doing
something "stupid" in the rndis driver somewhere :)

thanks,

greg k-h

Re: USB rndis_host - slow download transfers, RX errors

[Greg KH]

On Thu, Feb 08, 2018 at 02:16:08PM +, Tomasz Janowski, Ph.D. wrote:
> Dear USB developers,
> 
> Based on my google research, the problem I experience seems to happen
> with some newer smartphones. My test case is Samsung Galaxy S8 (SM-950U1). I 
> am
> trying to use USB tethering and everything seems to work as expected (modules
> are loaded, Ethernet devices are up and running, dhcp works fine). I can 
> connect to
> the external world using both LTE or wireless network on the phone.
> 
> Now, the problem is that the download speeds are terrible, around 64 KB/s,
> while uploads are fast, the order of 15 MB/s. These speeds do not depend
> on the wireless service provider: the results are similar when I tether wi-fi.
> The USB Ethernet interface on the Linux host reports a lot of receive errors 
> (attached:
> device_state.txt), while kernel reports bad rndis messages (attached: 
> kernel.log.txt).
> 
> Windows 10 works great with the same hardware (same PC and same phone), with
> uploads and downloads in the order of 150 Mbit/s, which is probably as fast 
> as my
> wireless network can do. But some people reported issues with older Windows 
> drivers too.
> Is possible that some newer version of RNDIS protocol is around and Linux 
> hasn't updated
> its RNDIS module yet?

Hey, I was _just_ talking to someone at Google about this same issue
yesterday, you beat him sending this same type of report to the mailing
list, nice job :)

Yes, this is not good, and we should work to resolve this, but first,
what kernel version are you using?  I think some fixes for the rndis
driver went in recently to 4.15, but it would be good to verify that
this isn't already resolved.

thanks,

greg k-h

Re: Backport Mellanox mlx5 patches to stable 4.9.y

[Greg KH]

On Tue, Jan 30, 2018 at 10:12:51AM +0100, Marta Rybczynska wrote:
> Hello Mellanox maintainers,
> I'd like to ask you to OK backporting two patches in mlx5 driver to 4.9 stable
> tree (they're in master for some time already).
> 
> We have multiple deployment in 4.9 that are running into the bug fixed by 
> those
> patches. We're deploying patched kernels and the issue disappears.
> 
> The patches are:
> 1410a90ae449061b7e1ae19d275148f36948801b net/mlx5: Define interface bits for 
> fencing UMR wqe
> 6e8484c5cf07c7ee632587e98c1a12d319dacb7c RDMA/mlx5: set UMR wqe fence 
> according to HCA cap

Looks good, now queued up, thanks.

greg k-h

Re: [PATCH v3 bpf] bpf: introduce BPF_JIT_ALWAYS_ON config

[Greg KH]

On Mon, Jan 29, 2018 at 12:40:47AM +0100, Daniel Borkmann wrote:
> On 01/28/2018 03:45 PM, Greg KH wrote:
> > On Wed, Jan 24, 2018 at 11:10:50AM +0100, Daniel Borkmann wrote:
> >> On 01/24/2018 11:07 AM, David Woodhouse wrote:
> >>> On Tue, 2018-01-09 at 22:39 +0100, Daniel Borkmann wrote:
> >>>> On 01/09/2018 07:04 PM, Alexei Starovoitov wrote:
> >>>>>
> >>>>> The BPF interpreter has been used as part of the spectre 2 attack 
> >>>>> CVE-2017-5715.
> >>>>>
> >>>>> A quote from goolge project zero blog:
> >>>>> "At this point, it would normally be necessary to locate gadgets in
> >>>>> the host kernel code that can be used to actually leak data by reading
> >>>>> from an attacker-controlled location, shifting and masking the result
> >>>>> appropriately and then using the result of that as offset to an
> >>>>> attacker-controlled address for a load. But piecing gadgets together
> >>>>> and figuring out which ones work in a speculation context seems 
> >>>>> annoying.
> >>>>> So instead, we decided to use the eBPF interpreter, which is built into
> >>>>> the host kernel - while there is no legitimate way to invoke it from 
> >>>>> inside
> >>>>> a VM, the presence of the code in the host kernel's text section is 
> >>>>> sufficient
> >>>>> to make it usable for the attack, just like with ordinary ROP gadgets."
> >>>>>
> >>>>> To make attacker job harder introduce BPF_JIT_ALWAYS_ON config
> >>>>> option that removes interpreter from the kernel in favor of JIT-only 
> >>>>> mode.
> >>>>> So far eBPF JIT is supported by:
> >>>>> x64, arm64, arm32, sparc64, s390, powerpc64, mips64
> >>>>>
> >>>>> The start of JITed program is randomized and code page is marked as 
> >>>>> read-only.
> >>>>> In addition "constant blinding" can be turned on with 
> >>>>> net.core.bpf_jit_harden
> >>>>>
> >>>>> v2->v3:
> >>>>> - move __bpf_prog_ret0 under ifdef (Daniel)
> >>>>>
> >>>>> v1->v2:
> >>>>> - fix init order, test_bpf and cBPF (Daniel's feedback)
> >>>>> - fix offloaded bpf (Jakub's feedback)
> >>>>> - add 'return 0' dummy in case something can invoke prog->bpf_func
> >>>>> - retarget bpf tree. For bpf-next the patch would need one extra hunk.
> >>>>>   It will be sent when the trees are merged back to net-next
> >>>>>
> >>>>> Considered doing:
> >>>>>   int bpf_jit_enable __read_mostly = BPF_EBPF_JIT_DEFAULT;
> >>>>> but it seems better to land the patch as-is and in bpf-next remove
> >>>>> bpf_jit_enable global variable from all JITs, consolidate in one place
> >>>>> and remove this jit_init() function.
> >>>>>
> >>>>> Signed-off-by: Alexei Starovoitov <a...@kernel.org>
> >>>>
> >>>> Applied to bpf tree, thanks Alexei!
> >>>
> >>> For stable too?
> >>
> >> Yes, this will go into stable as well; batch of backports will come 
> >> Thurs/Fri.
> > 
> > Any word on these?  Worse case, a simple list of git commit ids to
> > backport is all I need.
> 
> Sorry for the delay! There are various conflicts all over the place, so I had
> to backport manually. I just flushed out tested 4.14 batch, I'll see to get 
> 4.9
> out hopefully tonight as well, and the rest for 4.4 on Mon.

Not a problem at all, wanted to make sure I didn't miss them having be
posted somewhere I missed :)

If you need/want help for the 4.4 stuff, just let me know, and I'll be
glad to work on it.

thanks,

greg k-h

Re: [PATCH v3 bpf] bpf: introduce BPF_JIT_ALWAYS_ON config

[Greg KH]

On Mon, Jan 29, 2018 at 04:36:35PM +0100, Daniel Borkmann wrote:
> On 01/29/2018 12:40 AM, Daniel Borkmann wrote:
> > On 01/28/2018 03:45 PM, Greg KH wrote:
> >> On Wed, Jan 24, 2018 at 11:10:50AM +0100, Daniel Borkmann wrote:
> >>> On 01/24/2018 11:07 AM, David Woodhouse wrote:
> >>>> On Tue, 2018-01-09 at 22:39 +0100, Daniel Borkmann wrote:
> >>>>> On 01/09/2018 07:04 PM, Alexei Starovoitov wrote:
> [...]
> >>>>> Applied to bpf tree, thanks Alexei!
> >>>>
> >>>> For stable too?
> >>>
> >>> Yes, this will go into stable as well; batch of backports will come 
> >>> Thurs/Fri.
> >>
> >> Any word on these?  Worse case, a simple list of git commit ids to
> >> backport is all I need.
> > 
> > Sorry for the delay! There are various conflicts all over the place, so I 
> > had
> > to backport manually. I just flushed out tested 4.14 batch, I'll see to get 
> > 4.9
> > out hopefully tonight as well, and the rest for 4.4 on Mon.
> 
> While 4.14 and 4.9 BPF backports are tested and out since yesterday, and I
> saw Greg queued them up (thanks!), it looks like plain 4.4.113 doesn't even
> boot on my machine. While I can shortly see the kernel log, my screen turns
> black shortly thereafter and nothing reacts anymore. No such problems with
> 4.9 and 4.14 stables seen. (using x86_64, i7-6600U) Is this a known issue?

Not that I know of, sorry.  Odd graphics issue perhaps?

If you have some test programs I can run, I can look into doing the
backports, I still have a laptop around here that runs 4.4 :)

There's always a virtual machine as well, have you tried that?

thanks,

greg k-h

Re: [PATCH v3 bpf] bpf: introduce BPF_JIT_ALWAYS_ON config

[Greg KH]

On Wed, Jan 24, 2018 at 11:10:50AM +0100, Daniel Borkmann wrote:
> On 01/24/2018 11:07 AM, David Woodhouse wrote:
> > On Tue, 2018-01-09 at 22:39 +0100, Daniel Borkmann wrote:
> >> On 01/09/2018 07:04 PM, Alexei Starovoitov wrote:
> >>>
> >>> The BPF interpreter has been used as part of the spectre 2 attack 
> >>> CVE-2017-5715.
> >>>
> >>> A quote from goolge project zero blog:
> >>> "At this point, it would normally be necessary to locate gadgets in
> >>> the host kernel code that can be used to actually leak data by reading
> >>> from an attacker-controlled location, shifting and masking the result
> >>> appropriately and then using the result of that as offset to an
> >>> attacker-controlled address for a load. But piecing gadgets together
> >>> and figuring out which ones work in a speculation context seems annoying.
> >>> So instead, we decided to use the eBPF interpreter, which is built into
> >>> the host kernel - while there is no legitimate way to invoke it from 
> >>> inside
> >>> a VM, the presence of the code in the host kernel's text section is 
> >>> sufficient
> >>> to make it usable for the attack, just like with ordinary ROP gadgets."
> >>>
> >>> To make attacker job harder introduce BPF_JIT_ALWAYS_ON config
> >>> option that removes interpreter from the kernel in favor of JIT-only mode.
> >>> So far eBPF JIT is supported by:
> >>> x64, arm64, arm32, sparc64, s390, powerpc64, mips64
> >>>
> >>> The start of JITed program is randomized and code page is marked as 
> >>> read-only.
> >>> In addition "constant blinding" can be turned on with 
> >>> net.core.bpf_jit_harden
> >>>
> >>> v2->v3:
> >>> - move __bpf_prog_ret0 under ifdef (Daniel)
> >>>
> >>> v1->v2:
> >>> - fix init order, test_bpf and cBPF (Daniel's feedback)
> >>> - fix offloaded bpf (Jakub's feedback)
> >>> - add 'return 0' dummy in case something can invoke prog->bpf_func
> >>> - retarget bpf tree. For bpf-next the patch would need one extra hunk.
> >>>   It will be sent when the trees are merged back to net-next
> >>>
> >>> Considered doing:
> >>>   int bpf_jit_enable __read_mostly = BPF_EBPF_JIT_DEFAULT;
> >>> but it seems better to land the patch as-is and in bpf-next remove
> >>> bpf_jit_enable global variable from all JITs, consolidate in one place
> >>> and remove this jit_init() function.
> >>>
> >>> Signed-off-by: Alexei Starovoitov 
> >>
> >> Applied to bpf tree, thanks Alexei!
> > 
> > For stable too?
> 
> Yes, this will go into stable as well; batch of backports will come Thurs/Fri.

Any word on these?  Worse case, a simple list of git commit ids to
backport is all I need.

thanks,

greg k-h

Re: [PATCH 4.14-stable] xfrm: Fix a race in the xdst pcpu cache.

[Greg KH]

On Wed, Jan 24, 2018 at 09:57:06AM -0500, David Miller wrote:
> From: Steffen Klassert 
> Date: Wed, 24 Jan 2018 10:13:25 +0100
> 
> > commit 76a4201191814a0061cb5c861fafb9ecaa764846 upstream.
> > 
> > We need to run xfrm_resolve_and_create_bundle() with
> > bottom halves off. Otherwise we may reuse an already
> > released dst_enty when the xfrm lookup functions are
> > called from process context.
> > 
> > Fixes: c30d78c14a813db39a647b6a348b428 ("xfrm: add xdst pcpu cache")
> > Reported-by: Darius Ski 
> > Signed-off-by: Steffen Klassert 
> 
> ACK

Thanks, now queued up.

greg k-h

[PATCH] sysfs: turn WARN() into pr_warn()

[Greg KH]

From: Greg Kroah-Hartman 

It's not good to crash the machine if panic_on_warn() is set just
because someone made a stupid mistake of trying to create a sysfs file
with the same name of an existing one.  This makes the automated testing
tools a lot harder to find the real bugs in the kernel.

So just print a warning out and dump the stack to get the attention of
the developer that they did something foolish.  Then keep on trucking,
as this should not be a fatal error at all.

Reported-by: Dmitry Vyukov 
Signed-off-by: Greg Kroah-Hartman 
---

Dmitry, does this look good to you?  If so, I'll queue it up for
4.16-rc1.


diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c
index 2b67bda2021b..3a36a48a4b3f 100644
--- a/fs/sysfs/dir.c
+++ b/fs/sysfs/dir.c
@@ -10,6 +10,7 @@
  * Please see Documentation/filesystems/sysfs.txt for more information.
  */
 
+#define pr_fmt(fmt)"sysfs: " fmt
 #undef DEBUG
 
 #include 
@@ -27,8 +28,8 @@ void sysfs_warn_dup(struct kernfs_node *parent, const char 
*name)
if (buf)
kernfs_path(parent, buf, PATH_MAX);
 
-   WARN(1, KERN_WARNING "sysfs: cannot create duplicate filename 
'%s/%s'\n",
-buf, name);
+   pr_warn("cannot create duplicate filename '%s/%s'\n", buf, name);
+   dump_stack();
 
kfree(buf);
 }

Re: WARNING in sysfs_warn_dup

[Greg KH]

On Mon, Jan 22, 2018 at 03:30:12PM +0100, Dmitry Vyukov wrote:
> On Mon, Jan 22, 2018 at 3:00 PM, Greg KH <gre...@linuxfoundation.org> wrote:
> > On Mon, Jan 22, 2018 at 02:47:33PM +0100, Dmitry Vyukov wrote:
> >> On Tue, Dec 19, 2017 at 10:06 AM, Dmitry Vyukov <dvyu...@google.com> wrote:
> >> > On Tue, Dec 19, 2017 at 10:03 AM, Dmitry Vyukov <dvyu...@google.com> 
> >> > wrote:
> >> >>
> >> >> On Tue, Dec 19, 2017 at 10:01 AM, Greg KH <gre...@linuxfoundation.org> 
> >> >> wrote:
> >> >>>
> >> >>> On Mon, Dec 18, 2017 at 08:57:01AM -0800, syzbot wrote:
> >> >>> > Hello,
> >> >>> >
> >> >>> > syzkaller hit the following crash on
> >> >>> > 6084b576dca2e898f5c101baef151f7bfdbb606d
> >> >>> > git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/master
> >> >>> > compiler: gcc (GCC) 7.1.1 20170620
> >> >>> > .config is attached
> >> >>> > Raw console output is attached.
> >> >>> >
> >> >>> > Unfortunately, I don't have any reproducer for this bug yet.
> >> >>> >
> >> >>> >
> >> >>> > netlink: 9 bytes leftover after parsing attributes in process
> >> >>> > `syz-executor3'.
> >> >>> > sg_write: data in/out 822404280/197 bytes for SCSI command 0x12-- 
> >> >>> > guessing
> >> >>> > data in;
> >> >>> >program syz-executor0 not setting count and/or reply_len properly
> >> >>> > sg_write: data in/out 262364/161 bytes for SCSI command 0xff-- 
> >> >>> > guessing data
> >> >>> > in;
> >> >>> >program syz-executor0 not setting count and/or reply_len properly
> >> >>> > WARNING: CPU: 1 PID: 22282 at fs/sysfs/dir.c:31 
> >> >>> > sysfs_warn_dup+0x60/0x80
> >> >>> > fs/sysfs/dir.c:30
> >> >>> > Kernel panic - not syncing: panic_on_warn set ...
> >> >>>
> >> >>> Looks like a networking issue, it tried to create two sysfs directories
> >> >>> with the same name, which isn't a sysfs bug :)
> >> >
> >> >
> >> > Now as plain text:
> >> >
> >> > +net/core/dev.c maintainers
> >>
> >>
> >> Also happens for wiphy_register (on upstream
> >> a8750ddca918032d6349adbf9a4b6555e7db20da):
> >>
> >> [ cut here ]
> >> sysfs: cannot create duplicate filename
> >> '/class/ieee80211/š§"­ût{§Ô­ðô Š!× ž 7… Š†õiùS6 È< »þ {_CK5äá   ×ÝÊmô Be'
> >
> > That's a wonderful filename :)
> >
> >> WARNING: CPU: 1 PID: 8233 at fs/sysfs/dir.c:31
> >> sysfs_warn_dup+0x7e/0xa0 fs/sysfs/dir.c:30
> >
> > As this is just sysfs saying "Hey dummy, you are trying to do something
> > foolish here", what would be the better thing for it to do?
> >
> > Just printk(KERN_WARNING...) and then dump the stack?
> >
> > It seems the WARN_ON() that is currently being used is being treated as
> > an "error" by your testing, when really it isn't, unless the caller can
> > not handle the error being passed back up to it by the sysfs core.
> > Which it should, but I don't think you are even giving it the chance as
> > you are:
> >
> >> Kernel panic - not syncing: panic_on_warn set ...
> >
> > Yup, panic_on_warn :(
> >
> > ideas to make this easier for you?
> 
> 
> pr_warn or pr_warn_once (optionally followed by dump_stack) would work
> for syzbot.

This shouldn't be a _once() call, as it is called by things all over the
kernel, all with unique paths.

I'll go make up a patch for this, thanks.

greg k-h

Re: WARNING in sysfs_warn_dup

[Greg KH]

On Mon, Jan 22, 2018 at 02:47:33PM +0100, Dmitry Vyukov wrote:
> On Tue, Dec 19, 2017 at 10:06 AM, Dmitry Vyukov <dvyu...@google.com> wrote:
> > On Tue, Dec 19, 2017 at 10:03 AM, Dmitry Vyukov <dvyu...@google.com> wrote:
> >>
> >> On Tue, Dec 19, 2017 at 10:01 AM, Greg KH <gre...@linuxfoundation.org> 
> >> wrote:
> >>>
> >>> On Mon, Dec 18, 2017 at 08:57:01AM -0800, syzbot wrote:
> >>> > Hello,
> >>> >
> >>> > syzkaller hit the following crash on
> >>> > 6084b576dca2e898f5c101baef151f7bfdbb606d
> >>> > git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/master
> >>> > compiler: gcc (GCC) 7.1.1 20170620
> >>> > .config is attached
> >>> > Raw console output is attached.
> >>> >
> >>> > Unfortunately, I don't have any reproducer for this bug yet.
> >>> >
> >>> >
> >>> > netlink: 9 bytes leftover after parsing attributes in process
> >>> > `syz-executor3'.
> >>> > sg_write: data in/out 822404280/197 bytes for SCSI command 0x12-- 
> >>> > guessing
> >>> > data in;
> >>> >program syz-executor0 not setting count and/or reply_len properly
> >>> > sg_write: data in/out 262364/161 bytes for SCSI command 0xff-- guessing 
> >>> > data
> >>> > in;
> >>> >program syz-executor0 not setting count and/or reply_len properly
> >>> > WARNING: CPU: 1 PID: 22282 at fs/sysfs/dir.c:31 sysfs_warn_dup+0x60/0x80
> >>> > fs/sysfs/dir.c:30
> >>> > Kernel panic - not syncing: panic_on_warn set ...
> >>>
> >>> Looks like a networking issue, it tried to create two sysfs directories
> >>> with the same name, which isn't a sysfs bug :)
> >
> >
> > Now as plain text:
> >
> > +net/core/dev.c maintainers
> 
> 
> Also happens for wiphy_register (on upstream
> a8750ddca918032d6349adbf9a4b6555e7db20da):
> 
> [ cut here ]
> sysfs: cannot create duplicate filename
> '/class/ieee80211/š§"­ût{§Ô­ðô Š!× ž 7… Š†õiùS6 È< »þ {_CK5äá   ×ÝÊmô Be'

That's a wonderful filename :)

> WARNING: CPU: 1 PID: 8233 at fs/sysfs/dir.c:31
> sysfs_warn_dup+0x7e/0xa0 fs/sysfs/dir.c:30

As this is just sysfs saying "Hey dummy, you are trying to do something
foolish here", what would be the better thing for it to do?

Just printk(KERN_WARNING...) and then dump the stack?

It seems the WARN_ON() that is currently being used is being treated as
an "error" by your testing, when really it isn't, unless the caller can
not handle the error being passed back up to it by the sysfs core.
Which it should, but I don't think you are even giving it the chance as
you are:

> Kernel panic - not syncing: panic_on_warn set ...

Yup, panic_on_warn :(

ideas to make this easier for you?

thanks,

greg k-h

Re: [PATCH] e1000e: Fix e1000_check_for_copper_link_ich8lan return value.

[Greg KH]

On Wed, Jan 10, 2018 at 10:34:43AM +0900, Benjamin Poirier wrote:
> On 2018/01/10 01:29, rwar...@gmx.de wrote:
> > hallo
> > 
> > any chance to get this patch into stable and 4.15 ?
> > 
> > https://marc.info/?l=linux-kernel=151297726823919=2
> > 
> 
> It was part of the last network pull request and should be included in
> the next mainline release as
> 4110e02eb45e e1000e: Fix e1000_check_for_copper_link_ich8lan return value.
> 
> It's needed in stable branches that include commit 19110cfbb34d
> ("e1000e: Separate signaling for link check/link up"):
>   linux-4.14.y
>   linux-4.9.y
>   linux-4.4.y
>   linux-4.1.y
>   linux-3.18.y

Now queued up, thanks!

greg k-h

Re: [PATCH 4.4-stable 0/6] bpf: prevent out-of-bounds speculation

[Greg KH]

On Fri, Jan 12, 2018 at 05:17:15PM +0100, Jiri Slaby wrote:
> Hi,
> 
> this is a backport of these patches which I did for our kernels:
> c237ee5eb33b bpf: add bpf_patch_insn_single helper
> 3df126f35f88 bpf: don't (ab)use instructions to store state
> e245c5c6a565 bpf: move fixup_bpf_calls() function
> 79741b3bdec0 bpf: refactor fixup_bpf_calls()
> 8041902dae52 bpf: adjust insn_aux_data when patching insns
> b2157399cc98 bpf: prevent out-of-bounds speculation
> 
> I offer it here for use in stable 4.4, if there is no better/simpler
> backport available yet.

Thanks for all of these, now queued up.

I've also attempted a backport for 4.9, and also for 4.14 and queued
them up.  I'll go run the selftests to see how well that actually worked
or not...

greg k-h

Re: [PATCH v2 10/19] ipv4: prevent bounds-check bypass via speculative execution

[Greg KH]

On Fri, Jan 12, 2018 at 10:47:44AM -0800, Dan Williams wrote:
> On Thu, Jan 11, 2018 at 11:59 PM, Greg KH <gre...@linuxfoundation.org> wrote:
> >> Cc: "David S. Miller" <da...@davemloft.net>
> >> Cc: Alexey Kuznetsov <kuz...@ms2.inr.ac.ru>
> >> Cc: Hideaki YOSHIFUJI <yoshf...@linux-ipv6.org>
> >> Cc: netdev@vger.kernel.org
> >> Signed-off-by: Elena Reshetova <elena.reshet...@intel.com>
> >> Signed-off-by: Dan Williams <dan.j.willi...@intel.com>
> >> ---
> >>  net/ipv4/raw.c |   10 ++
> >>  1 file changed, 6 insertions(+), 4 deletions(-)
> >
> > Ugh, what is this, the 4th time I've said "I don't think this is an
> > issue, so why are you changing this code." to this patch.  To be
> > followed by a "oh yeah, you are right, I'll drop it", only to see it
> > show back up in the next time this patch series is sent out?
> >
> > Same for the other patches in this series that I have reviewed 4, maybe
> > 5, times already.  The "v2" is not very true here...
> 
> The theme of the review feedback on v1 was 'don't put ifence in any
> net/ code', and that was addressed.
> 
> I honestly thought the new definition of array_ptr() changed the
> calculus on this patch. Given the same pattern appears in the ipv6
> case, and I have yet to hear that we should drop the ipv6 patch, make
> the code symmetric just for readability purposes. Otherwise we need a
> comment saying why this is safe for ipv4, but maybe not safe for ipv6,
> I think 'array_ptr' is effectively that comment. I.e. 'array_ptr()' is
> designed to be low impact for instrumenting false positives. If that
> new argument does not hold water I will definitely drop this patch.

I also argued, again in an older review of this same patch series, that
the ipv6 patch should be dropped as well for this same exact reason.

I didn't think you wanted to hear me rant about the same thing on both
patches :)

greg k-h

Re: [PATCH v2 10/19] ipv4: prevent bounds-check bypass via speculative execution

[Greg KH]

On Thu, Jan 11, 2018 at 04:47:18PM -0800, Dan Williams wrote:
> Static analysis reports that 'offset' may be a user controlled value
> that is used as a data dependency reading from a raw_frag_vec buffer.
> In order to avoid potential leaks of kernel memory values, block
> speculative execution of the instruction stream that could issue further
> reads based on an invalid '*(rfv->c + offset)' value.
> 
> Based on an original patch by Elena Reshetova.

There is the "Co-Developed-by:" tag now, if you want to use it...

> Cc: "David S. Miller" 
> Cc: Alexey Kuznetsov 
> Cc: Hideaki YOSHIFUJI 
> Cc: netdev@vger.kernel.org
> Signed-off-by: Elena Reshetova 
> Signed-off-by: Dan Williams 
> ---
>  net/ipv4/raw.c |   10 ++
>  1 file changed, 6 insertions(+), 4 deletions(-)

Ugh, what is this, the 4th time I've said "I don't think this is an
issue, so why are you changing this code." to this patch.  To be
followed by a "oh yeah, you are right, I'll drop it", only to see it
show back up in the next time this patch series is sent out?

Same for the other patches in this series that I have reviewed 4, maybe
5, times already.  The "v2" is not very true here...

thanks,

greg k-h

Re: [PATCH 10/18] qla2xxx: prevent bounds-check bypass via speculative execution

[Greg KH]

On Thu, Jan 11, 2018 at 02:15:12PM -0800, Dan Williams wrote:
> On Sat, Jan 6, 2018 at 1:03 AM, Greg KH <gre...@linuxfoundation.org> wrote:
> > On Fri, Jan 05, 2018 at 05:10:48PM -0800, Dan Williams wrote:
> >> Static analysis reports that 'handle' may be a user controlled value
> >> that is used as a data dependency to read 'sp' from the
> >> 'req->outstanding_cmds' array.  In order to avoid potential leaks of
> >> kernel memory values, block speculative execution of the instruction
> >> stream that could issue reads based on an invalid value of 'sp'. In this
> >> case 'sp' is directly dereferenced later in the function.
> >
> > I'm pretty sure that 'handle' comes from the hardware, not from
> > userspace, from what I can tell here.  If we want to start auditing
> > __iomem data sources, great!  But that's a bigger task, and one I don't
> > think we are ready to tackle...
> 
> I think it falls in the hygiene bucket of shutting off an array index
> from a source that could be under attacker control. Should we leave
> this one un-patched while we decide if we generally have a problem
> with trusting completion 'tags' from hardware? My vote is patch it for
> now.

Hah, if you are worried about "tags" from hardware, we have a lot more
auditing to do, right?  I don't think anyone has looked into just basic
"bounds checking" for that type of information.  For USB devices we have
_just_ started doing that over the past year, the odds of anyone looking
at PCI devices for this same problem is slim-to-none.

Again, here are my questions/objections right now to this series:
- How can we audit this stuff?
- How did you audit this stuff to find these usages?
- How do you know that this series fixes all of the issues?
- What exact tree/date did you run your audit against?
- How do you know that linux-next does not contain a boatload
  more problems that we need to go back and fix after 4.16-rc1
  is out?
- How can we prevent this type of pattern showing up again?
- How can we audit the data coming from hardware correctly?

I'm all for merging this series, but if anyone things that somehow the
whole problem is now "solved" in this area, they are sorely mistaken.

thanks,

greg k-h

Re: [PATCH] MAINTAINERS: Mark some staging directories as "Obsolete"

[Greg KH]

On Tue, Jan 09, 2018 at 11:33:56AM -0800, Joe Perches wrote:
> Several staging directories have TODO files that indicate a
> subsystem will be removed in the future.
> 
> Using a status entry of "S:   Obsolete" helps indicate the
> subsystem files should not be modified unnecessarily.
> 
> checkpatch also tests this setting and emits a warning that
> the matching subsystem files should not be modified.
> 
> This might help avoid receiving patches that will be dropped.
> 
> Signed-off-by: Joe Perches 

Ah nice, I didn't know about this, I'll go queue this up, thanks.

greg k-h

Re: [PATCH 07/18] [media] uvcvideo: prevent bounds-check bypass via speculative execution

[Greg KH]

On Tue, Jan 09, 2018 at 04:26:28PM +0200, Laurent Pinchart wrote:
> Hi Greg,
> 
> On Tuesday, 9 January 2018 12:04:10 EET Greg KH wrote:
> > On Tue, Jan 09, 2018 at 10:40:21AM +0200, Laurent Pinchart wrote:
> > > On Saturday, 6 January 2018 11:40:26 EET Greg KH wrote:
> > >> On Sat, Jan 06, 2018 at 10:09:07AM +0100, Greg KH wrote:
> > >> 
> > >> While I'm all for fixing this type of thing, I feel like we need to do
> > >> something "else" for this as playing whack-a-mole for this pattern is
> > >> going to be a never-ending battle for all drivers for forever.
> > > 
> > > That's my concern too, as even if we managed to find and fix all the
> > > occurrences of the problematic patterns (and we won't), new ones will keep
> > > being merged all the time.
> > 
> > And what about the millions of lines of out-of-tree drivers that we all
> > rely on every day in our devices?  What about the distro kernels that
> > add random new drivers?
> 
> Of course, even though the out-of-tree drivers probably come with lots of 
> security issues worse than this one.

Sure, but I have worked with some teams that have used coverity to find
and fix all of the reported bugs it founds.  So some companies are
trying to fix their problems here, let's not make it impossible for them :)

> > We need some sort of automated way to scan for this.
> 
> Is there any initiative to implement such a scan in an open-source tool ?

Sure, if you want to, but I have no such initiative...

> We also need to educate developers. An automatic scanner could help there, 
> but 
> in the end the information has to spread to all our brains. It won't be easy, 
> and is likely not fully feasible, but it's no different than how developers 
> have to be educated about race conditions and locking for instance. It's a 
> mind set.

Agreed.

> > Intel, any chance we can get your coverity rules?  Given that the date
> > of this original patchset was from last August, has anyone looked at
> > what is now in Linus's tree?  What about linux-next?  I just added 3
> > brand-new driver subsystems to the kernel tree there, how do we know
> > there isn't problems in them?
> > 
> > And what about all of the other ways user-data can be affected?  Again,
> > as Peter pointed out, USB devices.  I want some chance to be able to at
> > least audit the codebase we have to see if that path is an issue.
> > Without any hint of how to do this in an automated manner, we are all
> > in deep shit for forever.
> 
> Or at least until the hardware architecture evolves. Let's drop the x86 
> instruction set, expose the µops, and have gcc handle the scheduling. Sure, 
> it 
> will mean recompiling everything for every x86 CPU model out there, but we 
> have source-based distros to the rescue :-D

Then we are back at the itanium mess, where all of the hardware issues
were supposed be fixed by the compiler writers.  We all remember how
well that worked out...

> > >> Either we need some way to mark this data path to make it easy for tools
> > >> like sparse to flag easily, or we need to catch the issue in the driver
> > >> subsystems, which unfortunatly, would harm the drivers that don't have
> > >> this type of issue (like here.)
> > > 
> > > But how would you do so ?
> > 
> > I do not know, it all depends on the access pattern, right?
> 
> Any data coming from userspace could trigger such accesses. If we want 
> complete coverage the only way I can think of is starting from syscalls and 
> tainting data down the call stacks (__user could help to some extend), but 
> we'll likely be drowned in false positives. I don't see how we could mark 
> paths manually.

I agree, which is why I want to see how someone did this work
originally.  We have no idea as no one is telling us anything :(

How do we "know" that these are the only problem areas?  When was the
last scan run?  On what tree?  And so on...

thanks,

greg k-h

Re: [PATCH 07/18] [media] uvcvideo: prevent bounds-check bypass via speculative execution

[Greg KH]

On Tue, Jan 09, 2018 at 10:40:21AM +0200, Laurent Pinchart wrote:
> On Saturday, 6 January 2018 11:40:26 EET Greg KH wrote:
> > On Sat, Jan 06, 2018 at 10:09:07AM +0100, Greg KH wrote:
> > 
> > While I'm all for fixing this type of thing, I feel like we need to do
> > something "else" for this as playing whack-a-mole for this pattern is
> > going to be a never-ending battle for all drivers for forever.
> 
> That's my concern too, as even if we managed to find and fix all the 
> occurrences of the problematic patterns (and we won't), new ones will keep 
> being merged all the time.

And what about the millions of lines of out-of-tree drivers that we all
rely on every day in our devices?  What about the distro kernels that
add random new drivers?

We need some sort of automated way to scan for this.

Intel, any chance we can get your coverity rules?  Given that the date
of this original patchset was from last August, has anyone looked at
what is now in Linus's tree?  What about linux-next?  I just added 3
brand-new driver subsystems to the kernel tree there, how do we know
there isn't problems in them?

And what about all of the other ways user-data can be affected?  Again,
as Peter pointed out, USB devices.  I want some chance to be able to at
least audit the codebase we have to see if that path is an issue.
Without any hint of how to do this in an automated manner, we are all
in deep shit for forever.

> > Either we need some way to mark this data path to make it easy for tools
> > like sparse to flag easily, or we need to catch the issue in the driver
> > subsystems, which unfortunatly, would harm the drivers that don't have
> > this type of issue (like here.)
> 
> But how would you do so ?

I do not know, it all depends on the access pattern, right?

> > I'm guessing that other operating systems, which don't have the luxury
> > of auditing all of their drivers are going for the "big hammer in the
> > subsystem" type of fix, right?
> 
> Other operating systems that ship closed-source drivers authored by hardware 
> vendors and not reviewed by third parties will likely stay vulnerable 
> forever. 
> That's a small concern though as I expect those drivers to contain much large 
> security holes anyway.

Well yes, but odds are those operating systems are doing something to
mitigate this, right?  Slowing down all user/kernel data paths?
Targeted code analysis tools?  Something else?  I doubt they just don't
care at all about it.  At the least, I would think Coverity would be
trying to sell licenses for this :(

thanks,

greg k-h

Re: [PATCH v2] b43: Replace mdelay with usleep_range in b43_radio_2057_init_post

[Greg KH]

On Tue, Jan 09, 2018 at 09:40:06AM +0800, Jia-Ju Bai wrote:
> b43_radio_2057_init_post is not called in an interrupt handler
> nor holding a spinlock.
> The function mdelay in it can be replaced with usleep_range, 
> to reduce busy wait.
> 
> Signed-off-by: Jia-Ju Bai 
> ---
> v2:
> * Replace mdelay with usleep_range, instead of msleep in v1.
>   Thank Larry for good advice.
> ---
>  drivers/net/wireless/broadcom/b43/phy_n.c |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/wireless/broadcom/b43/phy_n.c 
> b/drivers/net/wireless/broadcom/b43/phy_n.c
> index a5557d7..f2a2f41 100644
> --- a/drivers/net/wireless/broadcom/b43/phy_n.c
> +++ b/drivers/net/wireless/broadcom/b43/phy_n.c
> @@ -1031,7 +1031,7 @@ static void b43_radio_2057_init_post(struct b43_wldev 
> *dev)
>  
>   b43_radio_set(dev, R2057_RFPLL_MISC_CAL_RESETN, 0x78);
>   b43_radio_set(dev, R2057_XTAL_CONFIG2, 0x80);
> - mdelay(2);
> + usleep_range(2000, 3000);

Where did 3000 come from?  Are you sure about that?

thanks,

greg k-h

Re: [PATCH 06/18] x86, barrier: stop speculation for failed access_ok

[Greg KH]

On Sun, Jan 07, 2018 at 09:23:47PM -0500, David Miller wrote:
> From: Thomas Gleixner 
> Date: Sun, 7 Jan 2018 21:56:39 +0100 (CET)
> 
> > I surely agree, but we have gone the way of PTI without the ability of
> > exempting individual processes exactly for one reason:
> > 
> >   Lack of time
> > 
> > It can be done on top of the PTI implementation and it won't take ages.
> > 
> > For spectre_v1/2 we face the same problem simply because we got informed so
> > much ahead of time and we were all twiddling thumbs, enjoying our christmas
> > vacation and having a good time.
> 
> I just want to point out that this should be noted in history as a
> case where all of this controlled disclosure stuff seems to have made
> things worse rather than better.

I will note that the "controlled disclosure" for this thing was a total
and complete mess, and unlike any that I have ever seen in the past.
The people involved in running it had no idea how to do it at all, and
because of that, it failed miserably, despite being warned about it
numerous times by numerous people.

> Why is there so much haste and paranoia if supposedly some group of
> people had all this extra time to think about and deal with this bug?

Because that group was so small and isolated that they did not actually
talk to anyone who could actually provide input to help deal with the
bug.

So we are stuck now with dealing with this "properly", which is fine,
but please don't think that this is an excuse to blame "controlled
disclosure".  We know how to do that correctly, it did not happen in
this case at all because of the people driving the problem refused to do
it.

> Think I'm nuts?  Ok, then how did we fare any better by keeping this
> junk under wraps for weeks if not months?  (seriously, did responsible
> people really know about this as far back as... June 2017?)

Some "people" did, just not some "responsible people" :)

Oh well, time for the kernel to fix hardware bugs again, that's what we
are here for, you would think we would be used to it by now...

greg k-h

Re: [PATCH 07/18] [media] uvcvideo: prevent bounds-check bypass via speculative execution

[Greg KH]

On Sat, Jan 06, 2018 at 09:41:17AM -0800, Dan Williams wrote:
> On Sat, Jan 6, 2018 at 1:40 AM, Greg KH <gre...@linuxfoundation.org> wrote:
> > On Sat, Jan 06, 2018 at 10:09:07AM +0100, Greg KH wrote:
> >> On Fri, Jan 05, 2018 at 05:10:32PM -0800, Dan Williams wrote:
> >> > Static analysis reports that 'index' may be a user controlled value that
> >> > is used as a data dependency to read 'pin' from the
> >> > 'selector->baSourceID' array. In order to avoid potential leaks of
> >> > kernel memory values, block speculative execution of the instruction
> >> > stream that could issue reads based on an invalid value of 'pin'.
> >> >
> >> > Based on an original patch by Elena Reshetova.
> >> >
> >> > Cc: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
> >> > Cc: Mauro Carvalho Chehab <mche...@kernel.org>
> >> > Cc: linux-me...@vger.kernel.org
> >> > Signed-off-by: Elena Reshetova <elena.reshet...@intel.com>
> >> > Signed-off-by: Dan Williams <dan.j.willi...@intel.com>
> >> > ---
> >> >  drivers/media/usb/uvc/uvc_v4l2.c |7 +--
> >> >  1 file changed, 5 insertions(+), 2 deletions(-)
> >> >
> >> > diff --git a/drivers/media/usb/uvc/uvc_v4l2.c 
> >> > b/drivers/media/usb/uvc/uvc_v4l2.c
> >> > index 3e7e283a44a8..7442626dc20e 100644
> >> > --- a/drivers/media/usb/uvc/uvc_v4l2.c
> >> > +++ b/drivers/media/usb/uvc/uvc_v4l2.c
> >> > @@ -22,6 +22,7 @@
> >> >  #include 
> >> >  #include 
> >> >  #include 
> >> > +#include 
> >> >
> >> >  #include 
> >> >  #include 
> >> > @@ -810,6 +811,7 @@ static int uvc_ioctl_enum_input(struct file *file, 
> >> > void *fh,
> >> > struct uvc_entity *iterm = NULL;
> >> > u32 index = input->index;
> >> > int pin = 0;
> >> > +   __u8 *elem;
> >> >
> >> > if (selector == NULL ||
> >> > (chain->dev->quirks & UVC_QUIRK_IGNORE_SELECTOR_UNIT)) {
> >> > @@ -820,8 +822,9 @@ static int uvc_ioctl_enum_input(struct file *file, 
> >> > void *fh,
> >> > break;
> >> > }
> >> > pin = iterm->id;
> >> > -   } else if (index < selector->bNrInPins) {
> >> > -   pin = selector->baSourceID[index];
> >> > +   } else if ((elem = nospec_array_ptr(selector->baSourceID, index,
> >> > +   selector->bNrInPins))) {
> >> > +   pin = *elem;
> >>
> >> I dug through this before, and I couldn't find where index came from
> >> userspace, I think seeing the coverity rule would be nice.
> >
> > Ok, I take it back, this looks correct.  Ugh, the v4l ioctl api is
> > crazy complex (rightfully so), it's amazing that coverity could navigate
> > that whole thing :)
> >
> > While I'm all for fixing this type of thing, I feel like we need to do
> > something "else" for this as playing whack-a-mole for this pattern is
> > going to be a never-ending battle for all drivers for forever.  Either
> > we need some way to mark this data path to make it easy for tools like
> > sparse to flag easily, or we need to catch the issue in the driver
> > subsystems, which unfortunatly, would harm the drivers that don't have
> > this type of issue (like here.)
> >
> > I'm guessing that other operating systems, which don't have the luxury
> > of auditing all of their drivers are going for the "big hammer in the
> > subsystem" type of fix, right?
> >
> > I don't have a good answer for this, but if there was some better way to
> > rewrite these types of patterns to just prevent the need for the
> > nospec_array_ptr() type thing, that might be the best overall for
> > everyone.  Much like ebpf did with their changes.  That way a simple
> > coccinelle rule would be able to catch the pattern and rewrite it.
> >
> > Or am I just dreaming?
> 
> At least on the coccinelle front you're dreaming. Julia already took a
> look and said:
> 
> "I don't think Coccinelle would be good for doing this (ie
> implementing taint analysis) because the dataflow is too complicated."

Sorry for the confusion, no, I don't mean the "taint tracking", I mean
the generic pattern of "speculative out of bounds access" that we are
fixing here.

Yes, a

Re: [PATCH 14/18] ipv4: prevent bounds-check bypass via speculative execution

[Greg KH]

On Sat, Jan 06, 2018 at 12:23:47PM +, Alan Cox wrote:
> On Sat, 6 Jan 2018 10:01:54 +0100
> Greg KH <gre...@linuxfoundation.org> wrote:
> 
> > On Fri, Jan 05, 2018 at 05:11:10PM -0800, Dan Williams wrote:
> > > Static analysis reports that 'offset' may be a user controlled value  
> > 
> > Can I see the rule that determined that?  It does not feel like that is
> > correct, given the 3+ levels deep that this function gets this value
> > from...
> 
> On a current x86 you can execute something upwards of 150 instructions in
> a speculation window.

Yeah, I agree, it's deep :(

But for this patch, I thought the prior review determined that it was
not a problem.  Was that somehow proven incorrect?

thanks,

greg k-h

Re: [PATCH 10/18] qla2xxx: prevent bounds-check bypass via speculative execution

[Greg KH]

On Sat, Jan 06, 2018 at 10:03:22AM +0100, Greg KH wrote:
> On Fri, Jan 05, 2018 at 05:10:48PM -0800, Dan Williams wrote:
> > Static analysis reports that 'handle' may be a user controlled value
> > that is used as a data dependency to read 'sp' from the
> > 'req->outstanding_cmds' array.  In order to avoid potential leaks of
> > kernel memory values, block speculative execution of the instruction
> > stream that could issue reads based on an invalid value of 'sp'. In this
> > case 'sp' is directly dereferenced later in the function.
> 
> I'm pretty sure that 'handle' comes from the hardware, not from
> userspace, from what I can tell here.  If we want to start auditing
> __iomem data sources, great!  But that's a bigger task, and one I don't
> think we are ready to tackle...

And as Peter Zijlstra has already mentioned, if we have to look at those
codepaths, USB drivers are the first up for that mess, so having access
to the coverity rules would be a great help in starting that effort.

thanks,

greg k-h

Re: [PATCH 07/18] [media] uvcvideo: prevent bounds-check bypass via speculative execution

[Greg KH]

On Sat, Jan 06, 2018 at 10:09:07AM +0100, Greg KH wrote:
> On Fri, Jan 05, 2018 at 05:10:32PM -0800, Dan Williams wrote:
> > Static analysis reports that 'index' may be a user controlled value that
> > is used as a data dependency to read 'pin' from the
> > 'selector->baSourceID' array. In order to avoid potential leaks of
> > kernel memory values, block speculative execution of the instruction
> > stream that could issue reads based on an invalid value of 'pin'.
> > 
> > Based on an original patch by Elena Reshetova.
> > 
> > Cc: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
> > Cc: Mauro Carvalho Chehab <mche...@kernel.org>
> > Cc: linux-me...@vger.kernel.org
> > Signed-off-by: Elena Reshetova <elena.reshet...@intel.com>
> > Signed-off-by: Dan Williams <dan.j.willi...@intel.com>
> > ---
> >  drivers/media/usb/uvc/uvc_v4l2.c |7 +--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/media/usb/uvc/uvc_v4l2.c 
> > b/drivers/media/usb/uvc/uvc_v4l2.c
> > index 3e7e283a44a8..7442626dc20e 100644
> > --- a/drivers/media/usb/uvc/uvc_v4l2.c
> > +++ b/drivers/media/usb/uvc/uvc_v4l2.c
> > @@ -22,6 +22,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >  
> >  #include 
> >  #include 
> > @@ -810,6 +811,7 @@ static int uvc_ioctl_enum_input(struct file *file, void 
> > *fh,
> > struct uvc_entity *iterm = NULL;
> > u32 index = input->index;
> > int pin = 0;
> > +   __u8 *elem;
> >  
> > if (selector == NULL ||
> > (chain->dev->quirks & UVC_QUIRK_IGNORE_SELECTOR_UNIT)) {
> > @@ -820,8 +822,9 @@ static int uvc_ioctl_enum_input(struct file *file, void 
> > *fh,
> > break;
> > }
> > pin = iterm->id;
> > -   } else if (index < selector->bNrInPins) {
> > -   pin = selector->baSourceID[index];
> > +   } else if ((elem = nospec_array_ptr(selector->baSourceID, index,
> > +   selector->bNrInPins))) {
> > +   pin = *elem;
> 
> I dug through this before, and I couldn't find where index came from
> userspace, I think seeing the coverity rule would be nice.

Ok, I take it back, this looks correct.  Ugh, the v4l ioctl api is
crazy complex (rightfully so), it's amazing that coverity could navigate
that whole thing :)

While I'm all for fixing this type of thing, I feel like we need to do
something "else" for this as playing whack-a-mole for this pattern is
going to be a never-ending battle for all drivers for forever.  Either
we need some way to mark this data path to make it easy for tools like
sparse to flag easily, or we need to catch the issue in the driver
subsystems, which unfortunatly, would harm the drivers that don't have
this type of issue (like here.)

I'm guessing that other operating systems, which don't have the luxury
of auditing all of their drivers are going for the "big hammer in the
subsystem" type of fix, right?

I don't have a good answer for this, but if there was some better way to
rewrite these types of patterns to just prevent the need for the
nospec_array_ptr() type thing, that might be the best overall for
everyone.  Much like ebpf did with their changes.  That way a simple
coccinelle rule would be able to catch the pattern and rewrite it.

Or am I just dreaming?

thanks,

greg k-h

Re: [PATCH 10/18] qla2xxx: prevent bounds-check bypass via speculative execution

[Greg KH]

On Fri, Jan 05, 2018 at 05:10:48PM -0800, Dan Williams wrote:
> Static analysis reports that 'handle' may be a user controlled value
> that is used as a data dependency to read 'sp' from the
> 'req->outstanding_cmds' array.  In order to avoid potential leaks of
> kernel memory values, block speculative execution of the instruction
> stream that could issue reads based on an invalid value of 'sp'. In this
> case 'sp' is directly dereferenced later in the function.

I'm pretty sure that 'handle' comes from the hardware, not from
userspace, from what I can tell here.  If we want to start auditing
__iomem data sources, great!  But that's a bigger task, and one I don't
think we are ready to tackle...

thanks,

greg k-h

Re: [PATCH 14/18] ipv4: prevent bounds-check bypass via speculative execution

[Greg KH]

On Fri, Jan 05, 2018 at 05:11:10PM -0800, Dan Williams wrote:
> Static analysis reports that 'offset' may be a user controlled value

Can I see the rule that determined that?  It does not feel like that is
correct, given the 3+ levels deep that this function gets this value
from...

Same for the ipv6 patch, it's the same code logic.

thanks,

greg k-h

Re: [PATCH 14/18] ipv4: prevent bounds-check bypass via speculative execution

[Greg KH]

On Fri, Jan 05, 2018 at 05:11:10PM -0800, Dan Williams wrote:
> Static analysis reports that 'offset' may be a user controlled value
> that is used as a data dependency reading from a raw_frag_vec buffer.
> In order to avoid potential leaks of kernel memory values, block
> speculative execution of the instruction stream that could issue further
> reads based on an invalid '*(rfv->c + offset)' value.
> 
> Based on an original patch by Elena Reshetova.

I thought we "proved" that this patch was not needed at all, based on
previous review.  It doesn't look like that review cycle got
incorporated into this patch series at all, I guess I have to go back
and do it all again :(

thanks,

greg k-h

Re: [PATCH] Staging: ipx: fixed several no space before tabs coding style issues

[Greg KH]

On Wed, Dec 27, 2017 at 09:25:44PM +, Jianshen Liu wrote:
> Fixed several coding style warnings of "please, no space before tabs".
> 
> Signed-off-by: Jianshen Liu 
> ---
>  drivers/staging/ipx/af_ipx.c| 56 
> -
>  drivers/staging/ipx/ipx_proc.c  |  2 +-
>  drivers/staging/ipx/ipx_route.c |  6 ++---
>  3 files changed, 32 insertions(+), 32 deletions(-)

Please read drivers/staging/ipx/TODO

Re: [PATCH 1/3] staging: irda: fix type from "unsigned" to "unsigned int"

[Greg KH]

On Tue, Dec 26, 2017 at 09:52:54PM -0800, JI-HUN KIM wrote:
> Clean up checkpatch warning:
> WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
> 
> Signed-off-by: JI-HUN KIM 
> ---
>  drivers/staging/irda/drivers/esi-sir.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Please read drivers/staging/irda/TODO

sorry.

greg k-h

Re: 4.4.103 linux kernel regression

[Greg KH]

adding stable@ and netdev@

On Sat, Dec 23, 2017 at 10:49:27AM +, Mathias Tillman wrote:
> Hi, I wanted to make you aware of a recent regression to the Linux kernel
> introduced with commit 2417da3f4d6bc4fc6c77f613f0e2264090892aa5:
> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/commit/net/ipv6?h=linux-4.4.y=2417da3f4d6bc4fc6c77f613f0e2264090892aa5

Is this issue also present in Linus's tree?

> I have reported it here:
> https://bugzilla.kernel.org/show_bug.cgi?id=198189

Bugzilla doesn't work for networking bugs, nor stable stuff, just for a
few subsystems, sorry.

> Basically, that commit causes an endless loop if, for some reason, not all
> devices are unregistered in the rollback_registered_many function in
> net/dev.c
> 
> Decided to contact you directly since I have yet to receive any reply on
> the bug report, and I wasn't entirely sure what the procedure was. Please
> do let me know if I have to change anything in the report.

I can revert it, but it would be good to verify if this is an issue in
the latest releases or not first.

thanks,

greg k-h

Re: [PATCH v3 00/27] kill devm_ioremap_nocache

[Greg KH]

On Sat, Dec 23, 2017 at 06:55:25PM +0800, Yisheng Xie wrote:
> Hi all,
> 
> When I tried to use devm_ioremap function and review related code, I found
> devm_ioremap and devm_ioremap_nocache is almost the same with each other,
> except one use ioremap while the other use ioremap_nocache.

For all arches?  Really?  Look at MIPS, and x86, they have different
functions.

> While ioremap's
> default function is ioremap_nocache, so devm_ioremap_nocache also have the
> same function with devm_ioremap, which can just be killed to reduce the size
> of devres.o(from 20304 bytes to 18992 bytes in my compile environment).
> 
> I have posted two versions, which use macro instead of function for
> devm_ioremap_nocache[1] or devm_ioremap[2]. And Greg suggest me to kill
> devm_ioremap_nocache for no need to keep a macro around for the duplicate
> thing. So here comes v3 and please help to review.

I don't think this can be done, what am I missing?  These functions are
not identical, sorry for missing that before.

thanks,

greg k-h

Re: [PATCH v3 27/27] devres: kill devm_ioremap_nocache

[Greg KH]

On Sat, Dec 23, 2017 at 07:02:59PM +0800, Yisheng Xie wrote:
> --- a/lib/devres.c
> +++ b/lib/devres.c
> @@ -44,35 +44,6 @@ void __iomem *devm_ioremap(struct device *dev, 
> resource_size_t offset,
>  EXPORT_SYMBOL(devm_ioremap);
>  
>  /**
> - * devm_ioremap_nocache - Managed ioremap_nocache()
> - * @dev: Generic device to remap IO address for
> - * @offset: Resource address to map
> - * @size: Size of map
> - *
> - * Managed ioremap_nocache().  Map is automatically unmapped on driver
> - * detach.
> - */
> -void __iomem *devm_ioremap_nocache(struct device *dev, resource_size_t 
> offset,
> -resource_size_t size)
> -{
> - void __iomem **ptr, *addr;
> -
> - ptr = devres_alloc(devm_ioremap_release, sizeof(*ptr), GFP_KERNEL);
> - if (!ptr)
> - return NULL;
> -
> - addr = ioremap_nocache(offset, size);

Wait, devm_ioremap() calls ioremap(), not ioremap_nocache(), are you
_SURE_ that these are all identical?  For all arches?  If so, then
ioremap_nocache() can also be removed, right?

In my quick glance, I don't think you can do this series at all :(

greg k-h

Re: [REGRESSION][4.13.y][4.14.y][v4.15.y] net: reduce skb_warn_bad_offload() noise

[Greg KH]

On Tue, Dec 12, 2017 at 09:10:11AM -0500, David Miller wrote:
> From: Willem de Bruijn 
> Date: Mon, 11 Dec 2017 16:56:56 -0500
> 
> > On Mon, Dec 11, 2017 at 4:44 PM, Greg Kroah-Hartman
> >  wrote:
> >> On Mon, Dec 11, 2017 at 04:25:26PM -0500, Willem de Bruijn wrote:
> >>> Note that UFO was removed in 4.14 and that skb_warn_bad_offload
> >>> can happen for various types of packets, so there may be multiple
> >>> independent bug reports. I'm investigating two other non-UFO reports
> >>> just now.
> >>
> >> Meta-comment, now that UFO is gone from mainline, I'm wondering if I
> >> should just delete it from 4.4 and 4.9 as well.  Any objections for
> >> that?  I'd like to make it easy to maintain these kernels for a while,
> >> and having them diverge like this, with all of the issues around UFO,
> >> seems like it will just make life harder for myself if I leave it in.
> >>
> >> Any opinions?
> > 
> > Some of that removal had to be reverted with commit 0c19f846d582
> > ("net: accept UFO datagrams from tuntap and packet") for VM live
> > migration between kernels.
> > 
> > Any backports probably should squash that in at the least. Just today
> > another thread discussed that that patch may not address all open
> > issues still, so it may be premature to backport at this point.
> > http://lkml.kernel.org/r/
> 
> I would probably discourage backporting the UFO removal, at least for
> now.

Ok, thanks for letting me know, I'll ask again in 6 months or so :)

greg k-h