Re: WARNING in strp_data_ready

[Dmitry Vyukov]

On Thu, Dec 28, 2017 at 2:19 AM, Tom Herbert <tom@xxxxxxxxxxxxxxx> wrote:
> On Wed, Dec 27, 2017 at 12:20 PM, Ozgur <ozgur@xxxxxxxxxx> wrote:
>>
>>
>> 27.12.2017, 23:14, "Dmitry Vyukov" <dvyukov@xxxxxxxxxx>:
>>> On Wed, Dec 27, 2017 at 9:08 PM, Ozgur <ozgur@xxxxxxxxxx> wrote:
>>>>  27.12.2017, 22:21, "Dmitry Vyukov" <dvyukov@xxxxxxxxxx>:
>>>>>  On Wed, Dec 27, 2017 at 8:09 PM, Tom Herbert <tom@xxxxxxxxxxxxxxx> 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.
>>
> Hi Ozgur,
>
> I reposted the patches as RFC "kcm: Fix lockdep issue". Please test if you can!

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.
diff --git a/include/net/sock.h b/include/net/sock.h
index 9155da422692..7a7b14e9628a 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -1514,6 +1514,11 @@ static inline bool sock_owned_by_user(const struct sock *sk)
 	return sk->sk_lock.owned;
 }
 
+static inline bool sock_owned_by_user_nocheck(const struct sock *sk)
+{
+	return sk->sk_lock.owned;
+}
+
 /* no reclassification while locks are held */
 static inline bool sock_allow_reclassification(const struct sock *csk)
 {
diff --git a/net/strparser/strparser.c b/net/strparser/strparser.c
index c5fda15ba319..1fdab5c4eda8 100644
--- a/net/strparser/strparser.c
+++ b/net/strparser/strparser.c
@@ -401,7 +401,7 @@ void strp_data_ready(struct strparser *strp)
 	 * allows a thread in BH context to safely check if the process
 	 * lock is held. In this case, if the lock is held, queue work.
 	 */
-	if (sock_owned_by_user(strp->sk)) {
+	if (sock_owned_by_user_nocheck(strp->sk)) {
 		queue_work(strp_wq, &strp->work);
 		return;
 	}