WARNING in strp_data_ready

[syzbot]

Hello,

syzkaller hit the following crash on
73d3393ada4f70fa3df5639c8d438f2f034c0ecb
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/master
compiler: gcc (GCC) 7.1.1 20170620
.config is attached
Raw console output is attached.
C reproducer is attached
syzkaller reproducer is attached. See https://goo.gl/kgGztJ
for information about syzkaller reproducers


WARNING: CPU: 0 PID: 2996 at ./include/net/sock.h:1505 sock_owned_by_me
include/net/sock.h:1505 [inline]
WARNING: CPU: 0 PID: 2996 at ./include/net/sock.h:1505 sock_owned_by_user
include/net/sock.h:1511 [inline]
WARNING: CPU: 0 PID: 2996 at ./include/net/sock.h:1505
strp_data_ready+0x2b7/0x390 net/strparser/strparser.c:404
Kernel panic - not syncing: panic_on_warn set ...

CPU: 0 PID: 2996 Comm: syzkaller142210 Not tainted 4.14.0-rc5+ #138
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Call Trace:
<IRQ>
__dump_stack lib/dump_stack.c:16 [inline]
dump_stack+0x194/0x257 lib/dump_stack.c:52
panic+0x1e4/0x417 kernel/panic.c:181
__warn+0x1c4/0x1d9 kernel/panic.c:542
report_bug+0x211/0x2d0 lib/bug.c:183
fixup_bug+0x40/0x90 arch/x86/kernel/traps.c:178
do_trap_no_signal arch/x86/kernel/traps.c:212 [inline]
do_trap+0x260/0x390 arch/x86/kernel/traps.c:261
do_error_trap+0x120/0x390 arch/x86/kernel/traps.c:298
do_invalid_op+0x1b/0x20 arch/x86/kernel/traps.c:311
invalid_op+0x18/0x20 arch/x86/entry/entry_64.S:905
RIP: 0010:sock_owned_by_me include/net/sock.h:1505 [inline]
RIP: 0010:sock_owned_by_user include/net/sock.h:1511 [inline]
RIP: 0010:strp_data_ready+0x2b7/0x390 net/strparser/strparser.c:404
RSP: 0018:ffff8801db206b18 EFLAGS: 00010206
RAX: ffff8801d1e02080 RBX: ffff8801dad74c48 RCX: 0000000000000000
RDX: 0000000000000100 RSI: ffff8801d29fa0a0 RDI: ffffffff85cbede0
RBP: ffff8801db206b38 R08: 0000000000000005 R09: 1ffffffff0ce0bcd
R10: ffff8801db206a00 R11: dffffc0000000000 R12: ffff8801d29fa000
R13: ffff8801dad74c50 R14: ffff8801d4350a92 R15: 0000000000000001
psock_data_ready+0x56/0x70 net/kcm/kcmsock.c:353
tcp_child_process+0x559/0x990 net/ipv4/tcp_minisocks.c:818
tcp_v4_rcv+0x17e1/0x2f20 net/ipv4/tcp_ipv4.c:1682
ip_local_deliver_finish+0x2e2/0xba0 net/ipv4/ip_input.c:216
NF_HOOK include/linux/netfilter.h:249 [inline]
ip_local_deliver+0x1ce/0x6e0 net/ipv4/ip_input.c:257
dst_input include/net/dst.h:464 [inline]
ip_rcv_finish+0x887/0x19a0 net/ipv4/ip_input.c:397
NF_HOOK include/linux/netfilter.h:249 [inline]
ip_rcv+0xc3f/0x17d0 net/ipv4/ip_input.c:493
__netif_receive_skb_core+0x19af/0x33d0 net/core/dev.c:4428
__netif_receive_skb+0x2c/0x1b0 net/core/dev.c:4466
process_backlog+0x203/0x740 net/core/dev.c:5144
napi_poll net/core/dev.c:5542 [inline]
net_rx_action+0x792/0x1910 net/core/dev.c:5608
__do_softirq+0x2d7/0xb85 kernel/softirq.c:284
do_softirq_own_stack+0x2a/0x40 arch/x86/entry/entry_64.S:957
</IRQ>
do_softirq.part.22+0x14d/0x190 kernel/softirq.c:328
do_softirq kernel/softirq.c:176 [inline]
__local_bh_enable_ip+0x135/0x160 kernel/softirq.c:181
local_bh_enable include/linux/bottom_half.h:31 [inline]
rcu_read_unlock_bh include/linux/rcupdate.h:726 [inline]
ip_finish_output2+0x8ad/0x1460 net/ipv4/ip_output.c:231
ip_finish_output+0x85e/0xd10 net/ipv4/ip_output.c:317
NF_HOOK_COND include/linux/netfilter.h:238 [inline]
ip_output+0x1cc/0x860 net/ipv4/ip_output.c:405
dst_output include/net/dst.h:458 [inline]
ip_local_out+0x95/0x160 net/ipv4/ip_output.c:124
ip_queue_xmit+0x8c6/0x18e0 net/ipv4/ip_output.c:504
tcp_transmit_skb+0x19a1/0x3450 net/ipv4/tcp_output.c:1123
tcp_send_ack.part.34+0x386/0x610 net/ipv4/tcp_output.c:3557
tcp_send_ack+0x49/0x60 net/ipv4/tcp_output.c:3527
tcp_rcv_synsent_state_process net/ipv4/tcp_input.c:5756 [inline]
tcp_rcv_state_process+0x4a25/0x4d10 net/ipv4/tcp_input.c:5891
tcp_v4_do_rcv+0x55c/0x7d0 net/ipv4/tcp_ipv4.c:1482
sk_backlog_rcv include/net/sock.h:909 [inline]
__release_sock+0x124/0x360 net/core/sock.c:2264
release_sock+0xa4/0x2a0 net/core/sock.c:2776
inet_wait_for_connect net/ipv4/af_inet.c:557 [inline]
__inet_stream_connect+0x651/0xf00 net/ipv4/af_inet.c:643
inet_stream_connect+0x58/0xa0 net/ipv4/af_inet.c:682
SYSC_connect+0x204/0x470 net/socket.c:1642
SyS_connect+0x24/0x30 net/socket.c:1623
entry_SYSCALL_64_fastpath+0x1f/0xbe
RIP: 0033:0x43ff69
RSP: 002b:00007ffcf59753a8 EFLAGS: 00000217 ORIG_RAX: 000000000000002a
RAX: ffffffffffffffda RBX: ffffffffffffffff RCX: 000000000043ff69
RDX: 0000000000000010 RSI: 00000000200d1ff0 RDI: 0000000000000004
RBP: 0000000000000082 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000217 R12: 00000000004018d0
R13: 0000000000401960 R14: 0000000000000000 R15: 0000000000000000
Dumping ftrace buffer:
(ftrace buffer empty)
Kernel Offset: disabled
Rebooting in 86400 seconds..


---
This bug is generated by a dumb bot. It may contain errors.
See https://goo.gl/tpsmEJ for details.
Direct all questions to syzk...@googlegroups.com.

syzbot will keep track of this bug report.
Once a fix for this bug is committed, please reply to this email with:
#syz fix: exact-commit-title
To mark this as a duplicate of another syzbot report, please reply with:
#syz dup: exact-subject-of-another-report
If it's a one-off invalid bug report, please reply with:
#syz invalid
Note: if the crash happens again, it will cause creation of a new bug
report.

[John Fastabend]

Looks like KCM is calling sk_data_ready() without first taking the
sock lock.

/* Called with lower sock held */
static void kcm_rcv_strparser(struct strparser *strp, struct sk_buff *skb)
{
[...]
if (kcm_queue_rcv_skb(&kcm->sk, skb)) {

In this case kcm->sk is not the same lock the comment is referring to.
And kcm_queue_rcv_skb() will eventually call sk_data_ready().

@Tom, how about wrapping the sk_data_ready call in {lock|release}_sock?
I don't have anything better in mind immediately.

Thanks,
John

[Tom Herbert]

The sock locks are taken in reverse order in the send path so so
grabbing kcm sock lock with lower lock held to call sk_data_ready may
lead to deadlock like I think.

It might be possible to change the order in the send path to do this.
Something like:

trylock on lower socket lock
-if trylock fails
- release kcm sock lock
- lock lower sock
- lock kcm sock
- call sendpage locked function

I admit that dealing with two levels of socket locks in the data path
is quite a pain :-)

Tom

> Thanks,
> John

[Dmitry Vyukov]

up

still happening and we've lost 50K+ test VMs on this

[Dmitry Vyukov]

up

Still happens and number of crashes crossed 60K, can we do something
with this please?

[Tom Herbert]

Did you try the patch I posted?

[Dmitry Vyukov]

On Wed, Dec 27, 2017 at 8:09 PM, Tom Herbert <t...@herbertland.com> wrote:
> Did you try the patch I posted?

Hi Tom,

No. And I didn't know I need to. Why?
If you think the patch needs additional testing, you can ask syzbot to
test it. See https://github.com/google/syzkaller/blob/master/docs/syzbot.md#communication-with-syzbot
Otherwise proceed with committing it. Or what are we waiting for?

Thanks

[Ozgur]

27.12.2017, 22:21, "Dmitry Vyukov" <dvy...@google.com>:

> On Wed, Dec 27, 2017 at 8:09 PM, Tom Herbert <t...@herbertland.com> wrote:
>>  Did you try the patch I posted?
>
> Hi Tom,

Hello Dmitry,

> No. And I didn't know I need to. Why?
> If you think the patch needs additional testing, you can ask syzbot to
> test it. See https://github.com/google/syzkaller/blob/master/docs/syzbot.md#communication-with-syzbot
> Otherwise proceed with committing it. Or what are we waiting for?
>
> Thanks

I think we need to fixed patch for crash, in fact check to patch code and test solve the bug.
How do test it because there is no patch in the following bug?

The fix patch should be for this net/kcm/kcmsock.c file and lock functions must be added calling sk_data_ready ().

Regards

Ozgur

[Dmitry Vyukov]

On Wed, Dec 27, 2017 at 9:08 PM, Ozgur <oz...@goosey.org> wrote:
>
>
> 27.12.2017, 22:21, "Dmitry Vyukov" <dvy...@google.com>:
>> On Wed, Dec 27, 2017 at 8:09 PM, Tom Herbert <t...@herbertland.com> wrote:
>>> Did you try the patch I posted?
>>
>> Hi Tom,
>
> Hello Dmitry,
>
>> No. And I didn't know I need to. Why?
>> If you think the patch needs additional testing, you can ask syzbot to
>> test it. See https://github.com/google/syzkaller/blob/master/docs/syzbot.md#communication-with-syzbot
>> Otherwise proceed with committing it. Or what are we waiting for?
>>
>> Thanks
>
> I think we need to fixed patch for crash, in fact check to patch code and test solve the bug.
> How do test it because there is no patch in the following bug?

Hi Ozgur,

I am not sure I completely understand what you mean. But the
reproducer for this bug (which one can use for testing) is here:
https://groups.google.com/forum/#!topic/syzkaller-bugs/Kxs05ziCpgY
Tom also mentions there is some patch for this, but I don't know where
it is, it doesn't seem to be referenced from this thread.

[Ozgur]

27.12.2017, 23:14, "Dmitry Vyukov" <dvy...@google.com>:

> On Wed, Dec 27, 2017 at 9:08 PM, Ozgur <oz...@goosey.org> wrote:
>>  27.12.2017, 22:21, "Dmitry Vyukov" <dvy...@google.com>:
>>>  On Wed, Dec 27, 2017 at 8:09 PM, Tom Herbert <t...@herbertland.com> wrote:
>>>>   Did you try the patch I posted?
>>>
>>>  Hi Tom,
>>
>>  Hello Dmitry,
>>
>>>  No. And I didn't know I need to. Why?
>>>  If you think the patch needs additional testing, you can ask syzbot to
>>>  test it. See https://github.com/google/syzkaller/blob/master/docs/syzbot.md#communication-with-syzbot
>>>  Otherwise proceed with committing it. Or what are we waiting for?
>>>
>>>  Thanks
>>
>>  I think we need to fixed patch for crash, in fact check to patch code and test solve the bug.
>>  How do test it because there is no patch in the following bug?
>
> Hi Ozgur,
>
> I am not sure I completely understand what you mean. But the
> reproducer for this bug (which one can use for testing) is here:
> https://groups.google.com/forum/#!topic/syzkaller-bugs/Kxs05ziCpgY
> Tom also mentions there is some patch for this, but I don't know where
> it is, it doesn't seem to be referenced from this thread.

Hello Dmitry,

Ah, I'm sorry I don't seen Tom mail and I don't have a patch not tested :)
I think Tom send patch to only you and are you tested?

kcmsock.c will change and strp_data_ready I think locked.

Tom, please send a patch for me? I can test and inform you.

Regards

Ozgur

[Tom Herbert]

Hi Ozgur,

I reposted the patches as RFC "kcm: Fix lockdep issue". Please test if you can!

Thanks,
Tom

[Dmitry Vyukov]

OK, I will work as your typist this time:

#syz test: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
master

But I wonder what part of the following you don't understand? Do we
need to improve wording or something?

> If you think the patch needs additional testing, you can ask syzbot to test it.
> See https://github.com/google/syzkaller/blob/master/docs/syzbot.md#communication-with-syzbot

Also I don't know what git repo/branch you have in mind. Kernel
patches don't generally apply just to any tree. Fingers crossed that I
guessed correctly and it will apply.

[syzbot]

Hello,

syzbot has tested the proposed patch and the reproducer did not trigger
crash:

Reported-and-tested-by:
syzbot+c91c53af67f9ebe5...@syzkaller.appspotmail.com

Note: the tag will also help syzbot to understand when the bug is fixed.

Tested on commit 5f520fc318764df800789edd202b5e3b55130613

git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/master
compiler: gcc (GCC) 7.1.1 20170620

Patch is attached.
Kernel config is attached.


---
There is no WARRANTY for the result, to the extent permitted by applicable
law.
Except when otherwise stated in writing syzbot provides the result "AS IS"
without warranty of any kind, either expressed or implied, but not limited
to,
the implied warranties of merchantability and fittness for a particular
purpose.
The entire risk as to the quality of the result is with you. Should the
result
prove defective, you assume the cost of all necessary servicing, repair or
correction.

[Dmitry Vyukov]

On Thu, Dec 28, 2017 at 9:12 AM, syzbot
<syzbot+c91c53af67f9ebe5...@syzkaller.appspotmail.com>
wrote:

> Hello,
>
> syzbot has tested the proposed patch and the reproducer did not trigger
> crash:
>
> Reported-and-tested-by:
> syzbot+c91c53af67f9ebe5...@syzkaller.appspotmail.com

/\/\/\/\/\/\/\/\/\/\/\/\/\

Tom, please don't miss this part!

[Ozgur]

28.12.2017, 04:19, "Tom Herbert" <t...@herbertland.com>:

Hello Tom,

Which are you use the repos? I pulled but I don't seen this patches.

> git pull
Updating 464e1d5f23cc..5f520fc31876
Fast-forward
Documentation/devicetree/bindings/sound/da7218.txt | 2 +-
Documentation/devicetree/bindings/sound/da7219.txt | 2 +-
drivers/gpio/gpio-reg.c | 4 ++--
drivers/gpio/gpiolib-acpi.c | 2 +-
drivers/gpio/gpiolib-devprop.c | 17 +++++++----------
drivers/gpio/gpiolib-of.c | 3 ++-
drivers/gpio/gpiolib.h | 3 ++-
drivers/hwmon/hwmon.c | 21 +++++++++++++++++----
kernel/trace/ring_buffer.c | 12 +++++++++++-
kernel/trace/trace.c | 13 ++++---------
sound/hda/hdac_i915.c | 2 +-
sound/pci/hda/patch_conexant.c | 29 +++++++++++++++++++++++++++++
sound/pci/hda/patch_realtek.c | 14 +++++++++++---
sound/soc/amd/acp-pcm-dma.c | 7 +++++++
sound/soc/atmel/Kconfig | 2 +-
sound/soc/codecs/da7218.c | 2 +-
sound/soc/codecs/msm8916-wcd-analog.c | 2 +-
sound/soc/codecs/msm8916-wcd-digital.c | 4 ++--
sound/soc/codecs/nau8825.c | 1 +
sound/soc/codecs/rt5514-spi.c | 15 +++++++++------
sound/soc/codecs/rt5514.c | 2 +-
sound/soc/codecs/rt5645.c | 2 ++
sound/soc/codecs/rt5663.c | 4 ++++
sound/soc/codecs/rt5663.h | 4 ++++
sound/soc/codecs/tlv320aic31xx.h | 2 +-
sound/soc/codecs/twl4030.c | 4 +++-
sound/soc/codecs/wm_adsp.c | 12 ++++++------
sound/soc/fsl/fsl_asrc.h | 4 ++--
sound/soc/fsl/fsl_ssi.c | 44 ++++++++++++++++++++++++++++++++++----------
sound/soc/intel/boards/kbl_rt5663_max98927.c | 2 +-
sound/soc/intel/boards/kbl_rt5663_rt5514_max98927.c | 2 +-
sound/soc/intel/skylake/skl-nhlt.c | 15 ++++++++++-----
sound/soc/intel/skylake/skl-topology.c | 2 +-
sound/soc/rockchip/rockchip_spdif.c | 18 +++++++++++++-----
sound/soc/sh/rcar/adg.c | 6 +++---
sound/soc/sh/rcar/core.c | 4 ++--
sound/soc/sh/rcar/dma.c | 86 ++++++--------------------------------------------------------------------------------
sound/soc/sh/rcar/ssi.c | 16 ++++++++++------
sound/soc/sh/rcar/ssiu.c | 5 ++++-
39 files changed, 219 insertions(+), 172 deletions(-)

[Tom Herbert]

They are not in any public repo yet. I posted the patches to netdev
list so they can be reviewed and tested by third parties. Posting
patches to the list a normal path to get patches into the kernel
(http://nickdesaulniers.github.io/blog/2017/05/16/submitting-your-first-patch-to-the-linux-kernel-and-responding-to-feedback/).

These patches were applied to net-next but are simple enough that they
should apply to other branches. I will repost and target to net per
Dave's directive once they are verified to fix the issue.

Tom

[Dmitry Vyukov]

FWIW they are already verified to fix the issue, see few emails up, also here:
https://groups.google.com/d/msg/syzkaller-bugs/Kxs05ziCpgY/fPdZcO_GAwAJ
and don't forget this:
https://groups.google.com/d/msg/syzkaller-bugs/Kxs05ziCpgY/uGjsrA3HAwAJ

[Ozgur]

28.12.2017, 19:33, "Dmitry Vyukov" <dvy...@google.com>:

Hello,

thanks Tom and I have tested the fixed patch for linux-next builds and don't have to kernel panic. when nocheck funcs call sk_lock.owned and kernel doesn't give a panic. I have compiled and uploaded next-kernel.

Dmitry,
could you test it on linux-next?

Ozgur

[Dmitry Vyukov]

If you are trying to test how many times I can repeat this, I can
repeat this lots of times: