From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id CD27FC3DA4A for ; Mon, 29 Jul 2024 14:50:03 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 04A8E6B008A; Mon, 29 Jul 2024 10:50:03 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id F3BD26B008C; Mon, 29 Jul 2024 10:50:02 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id E2A846B0092; Mon, 29 Jul 2024 10:50:02 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id C3BD96B008A for ; Mon, 29 Jul 2024 10:50:02 -0400 (EDT) Received: from smtpin05.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 6E68AA0443 for ; Mon, 29 Jul 2024 14:50:02 +0000 (UTC) X-FDA: 82393075044.05.E1172A6 Received: from mail-vk1-f175.google.com (mail-vk1-f175.google.com [209.85.221.175]) by imf05.hostedemail.com (Postfix) with ESMTP id 9759B100002 for ; Mon, 29 Jul 2024 14:50:00 +0000 (UTC) Authentication-Results: imf05.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=OYysIR6I; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf05.hostedemail.com: domain of elver@google.com designates 209.85.221.175 as permitted sender) smtp.mailfrom=elver@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1722264540; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=Dc5OnpQkO4GI5cUSMqWm8aEGW3XO5gfb3sMJB+82c8A=; b=Mw1QVE8L/RUUEhlKGRoEMghxVM7pP5hIO97yIpY7B6W8bsv5xEkoLUrMEGqz4rgVaZQF1W 9dlUFAxwaklD0VDMKB3+0iJf5XfP0RvqrAeFs/DcygsgKzH+DWwJb/nINNpSxlQtnGyPgN 13e8s1BJrmnPpf7qgNayrLpS3bu7jm4= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1722264540; a=rsa-sha256; cv=none; b=gY2xKdY9B5a7zSxBFX3+D0XV2ToeCLrRKjWQJ6TL4ctQp0PqEoTimZvkC8Owz8rhaymTOS gHP48B84ELl/I65D6lKnFOEbvG/17HLURZU5N4HGQLrpKQ9OMF69xgxUtIuw4poZLX+HKc IzOtpkNSDGl47Az3JYukNz1HadKdVqk= ARC-Authentication-Results: i=1; imf05.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=OYysIR6I; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf05.hostedemail.com: domain of elver@google.com designates 209.85.221.175 as permitted sender) smtp.mailfrom=elver@google.com Received: by mail-vk1-f175.google.com with SMTP id 71dfb90a1353d-4f51981b1beso1184193e0c.2 for ; Mon, 29 Jul 2024 07:50:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1722264599; x=1722869399; darn=kvack.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=Dc5OnpQkO4GI5cUSMqWm8aEGW3XO5gfb3sMJB+82c8A=; b=OYysIR6IUL23sIVQ5EyJsIwqorAxVtNySX9v6mwGQxH+pX4SJ1scGGnmV0rcccbiDU Pws3RKh3s2W7DsKolikqYMxnw6Zk+KSF7J/T/Fvx+tmD5FGTvMU8oRpgdHdaBSvmDRq9 KD87DMohAaP0wsKCCbNDmJlTzLViMUQBpb2amyv4f70yIi+lyanQmJ1yOrQHxDYObToF +MIixoufceOy6HlN1TUMS8LN13yG0tgjE+cpEPKbo1r8j+3v07anmhNS4ZvQKUOrhXew dbugskOJg9Cu99qjl/2ZpJJv2TgqmM8dAOcCmJXtqtgHOUzVrjtdXHuegXwM4gp5Y/0b sbqA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1722264599; x=1722869399; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=Dc5OnpQkO4GI5cUSMqWm8aEGW3XO5gfb3sMJB+82c8A=; b=uP5iA2++oOMZsp0c9HpIi8jQfRxOmTa5h6jlLbEVsr406rcEjR/5I2ZxYaMSm0vxSR Rq0g8MNP4XtzZpOUU1jJ6s04e+l7k0QIKJeoRC9Kxd8+tkfrPaVwYn71Dw6Hus2gRW/E Mdxjc4Apb6TpRlHRfJvwRYxTh2lqfHGoTh1+90Jp3PI6kvqwJP7RjwEewuj8p+JCAQ57 7q2dzV/IDSyGQ/W3W4YBH8FXsdOlDOGq/Df5fT/bKo+diHVtOTrO0UnEgOltwTwwSNas pEVqgBi3fnJxhkUeBRPDYi2hq7Ilo7JWM5iSihcw0/Pbn8d8xEUJoBZ5J8tsOLGMSeP0 Mb+A== X-Forwarded-Encrypted: i=1; AJvYcCXojrr4E+auJTKNLeJ5cBSeYwrNzw+S8nNq6Ja0oo4WunDsnWhvS/WVDVvNKMq49QHtTgUqKLE5oRe9TD3V9rE1VXc= X-Gm-Message-State: AOJu0YycvAorYmgVZ01N52yDrypGUsojA9yhSmWkwr9ZU0WmMb1X2+gE Zw/GEYuQvxZKv5br2zHrHkIEkcOhLSUQYW2O7hwpyjYJmnq3gyk8zWjwENhD36fzUClXq41GLRF zggpTeR3aK7pa32XMOzzaZ0nfsQDwbDOMm+9R X-Google-Smtp-Source: AGHT+IHFMADPLpMiCf0PtUHwwXP9ydcCss5DHpj4/ujsEVhBsMJ/f20LMGXqV2SAf4576Qb9p5SV9/SHngSzxzrhnsQ= X-Received: by 2002:a05:6122:310a:b0:4f6:a5ed:eb11 with SMTP id 71dfb90a1353d-4f6e68f714amr9340551e0c.8.1722264599389; Mon, 29 Jul 2024 07:49:59 -0700 (PDT) MIME-Version: 1.0 References: <20240729022158.92059-1-andrey.konovalov@linux.dev> In-Reply-To: From: Marco Elver Date: Mon, 29 Jul 2024 16:49:20 +0200 Message-ID: Subject: Re: [PATCH] kcov: properly check for softirq context To: andrey.konovalov@linux.dev Cc: Dmitry Vyukov , Andrew Morton , Andrey Konovalov , Aleksandr Nogikh , Alexander Potapenko , kasan-dev@googlegroups.com, linux-mm@kvack.org, Alan Stern , Greg Kroah-Hartman , Marcello Sylvester Bauer , linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, syzbot+2388cdaeb6b10f0c13ac@syzkaller.appspotmail.com, stable@vger.kernel.org, Sebastian Andrzej Siewior , Thomas Gleixner Content-Type: text/plain; charset="UTF-8" X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: 9759B100002 X-Stat-Signature: p5n34cbbi7xyyy46npn3dw5stokuh5pk X-Rspam-User: X-HE-Tag: 1722264600-995043 X-HE-Meta: U2FsdGVkX19kzdxmyvEMz8yCt+AjcBrtGfFw+q6tO1ULpabClEQzfZVQKaGxd67cCMLiL5S8Ae/LllOg6XMxYqakBJoejfZJzvJ42yPDqZKIlSH9n5tYWaEQsCI6TO82hO05jsMGclofGtn8BN1yBlANepqceTrFaeA7Xjpcpyu0gxaR0vf4C3KwkJgTp6PN7tIRGUIopRxatfKWBWtyZiiigESac3f082ekwgeNYotycALdol+uluYuGhHpeHlD8rgdhgTNmJUwObQ8+F9LeTRSoDTzCKwI90GFty1AD78g7UN2NjuaBjVdAe6K0NZBwIkGyGvusg5lblqu1i3d9oGTLUSGQGebgRe3X3ovDV9BeoMbzMB66HSHPkOmD3qyCpc19GX7dqjCAujEFBW/zxWWHuhNiRlzx9LMcHt0cL9pJi1eI0O7PwR8gYGeQVs+xhJEr1jbzDvwapaIO5ayn4+ZKDOJ/9mrX5QzKI83ooBt39k1QAtaYmaq8z4gOKsw4bbMulkgf1Y3Sz/MaRq+7Rp5um4cuuoSAmpeTSTJlq4lsZUWsDXjh1whIn7NKwW79lVFX1AKgUYofS+A2ODHvR2Jp8XF+7w+ITwungzMSUQg2UMChjreAXK73bJL77xe72x0DmtyJHRdMp88odhkzFkwU2JaLcoNQg6K8ZpZrMKi/HQueEKF/Pyk8Um2cWW8P97+hAHbfchoQDGzjZNcaeoa+j5m/P4tFCB0PQ9G7sZBeymXF5+CWr4kg4kocHwWv3hYlEzNsQP+QrL9T1EvxmBZcQWpvN36O/MgJnSfOUYRALClDEUQfGUVyoAg/IIJ8FFV6ElqT6AdE+MYL8tHPQ6m5buebBnBexcMMW54QumwJy6ZVmfuStAj+A2izdTQwhnJYH3qzC2BeAqQNwByl6wXmPlI1Pq2wqZFjXo8bwt6XOKnfL+lbaFtdu/niGp07LGDw+FKkzGQctyOgbG 6q76g+OY DYQppxyjlfXV1VJ0i8yvocCV88ZILdyPOwuj8m7U7OgMGP1JzmhVTgNjB9MadGBbf4JbHEtGwk5sLl5sO26bTqGkkCp2HRXFM/9S+BTS35Qyg3Pnmogs+UnF6hKGVgEw3Auw8RrmW4gZi1MNsK/yMGxalUxieE3awAiqU6qB8wwV2B31jpfd4b3ZEMNE0c9ugsAXRMc0JuIaLb3CXyvH7BCwT5szivLd7R7waM1aL3mUXs7qGlqj04gcC/0K8DHfPO2p7dhwp3i2nlKAyW2hTc5OIyXiaSJvF89eGsJ/mXAHH1KJpvvDoZ1o1Cj5BsbQ2kw5CAXbR61Ns/HvfwMX8LGk9OLx3fgvISSfNqoaos0fxxtKRfydmWKGqdnzm1so3ObXABDshyT5gS1ivadUuSnC37FooBU7wviJnYkJy/2Zd7E/IoJYpJWKgYFpe5LeRRY6zgheTco35Ucgq/tB0/xQx8LCzVvwy41bsRgfCkxgskB/PkjT+x6fP4A== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Mon, 29 Jul 2024 at 11:42, Marco Elver wrote: > > On Mon, 29 Jul 2024 at 04:22, wrote: > > > > From: Andrey Konovalov > > > > When collecting coverage from softirqs, KCOV uses in_serving_softirq() to > > check whether the code is running in the softirq context. Unfortunately, > > in_serving_softirq() is > 0 even when the code is running in the hardirq > > or NMI context for hardirqs and NMIs that happened during a softirq. > > > > As a result, if a softirq handler contains a remote coverage collection > > section and a hardirq with another remote coverage collection section > > happens during handling the softirq, KCOV incorrectly detects a nested > > softirq coverate collection section and prints a WARNING, as reported > > by syzbot. > > > > This issue was exposed by commit a7f3813e589f ("usb: gadget: dummy_hcd: > > Switch to hrtimer transfer scheduler"), which switched dummy_hcd to using > > hrtimer and made the timer's callback be executed in the hardirq context. > > > > Change the related checks in KCOV to account for this behavior of > > in_serving_softirq() and make KCOV ignore remote coverage collection > > sections in the hardirq and NMI contexts. > > > > This prevents the WARNING printed by syzbot but does not fix the inability > > of KCOV to collect coverage from the __usb_hcd_giveback_urb when dummy_hcd > > is in use (caused by a7f3813e589f); a separate patch is required for that. > > > > Reported-by: syzbot+2388cdaeb6b10f0c13ac@syzkaller.appspotmail.com > > Closes: https://syzkaller.appspot.com/bug?extid=2388cdaeb6b10f0c13ac > > Fixes: 5ff3b30ab57d ("kcov: collect coverage from interrupts") > > Cc: stable@vger.kernel.org > > Signed-off-by: Andrey Konovalov > > --- > > kernel/kcov.c | 15 ++++++++++++--- > > 1 file changed, 12 insertions(+), 3 deletions(-) > > > > diff --git a/kernel/kcov.c b/kernel/kcov.c > > index f0a69d402066e..274b6b7c718de 100644 > > --- a/kernel/kcov.c > > +++ b/kernel/kcov.c > > @@ -161,6 +161,15 @@ static void kcov_remote_area_put(struct kcov_remote_area *area, > > kmsan_unpoison_memory(&area->list, sizeof(area->list)); > > } > > > > +/* > > + * Unlike in_serving_softirq(), this function returns false when called during > > + * a hardirq or an NMI that happened in the softirq context. > > + */ > > +static inline bool in_softirq_really(void) > > +{ > > + return in_serving_softirq() && !in_hardirq() && !in_nmi(); > > +} > > Not sure you need this function. Check if just this will give you what you want: > > interrupt_context_level() == 1 > > I think the below condition could then also just become: > > if (interrupt_context_level() == 1 && t->kcov_softirq) > > Although the softirq_count() helper has a special PREEMPT_RT variant, > and interrupt_context_level() doesn't, so it's not immediately obvious > to me if that's also ok on PREEMPT_RT kernels. > > Maybe some RT folks can help confirm that using > interrupt_context_level()==1 does what your above function does also > on RT kernels. Hmm, so Thomas just told me that softirqs always run in threaded context on RT and because there's no nesting, interrupt_context_level() won't work for what I had imagined here. So your current solution is fine. Acked-by: Marco Elver > > static notrace bool check_kcov_mode(enum kcov_mode needed_mode, struct task_struct *t) > > { > > unsigned int mode; > > @@ -170,7 +179,7 @@ static notrace bool check_kcov_mode(enum kcov_mode needed_mode, struct task_stru > > * so we ignore code executed in interrupts, unless we are in a remote > > * coverage collection section in a softirq. > > */ > > - if (!in_task() && !(in_serving_softirq() && t->kcov_softirq)) > > + if (!in_task() && !(in_softirq_really() && t->kcov_softirq)) > > return false; > > mode = READ_ONCE(t->kcov_mode); > > /* > > @@ -849,7 +858,7 @@ void kcov_remote_start(u64 handle) > > > > if (WARN_ON(!kcov_check_handle(handle, true, true, true))) > > return; > > - if (!in_task() && !in_serving_softirq()) > > + if (!in_task() && !in_softirq_really()) > > return; > > > > local_lock_irqsave(&kcov_percpu_data.lock, flags); > > @@ -991,7 +1000,7 @@ void kcov_remote_stop(void) > > int sequence; > > unsigned long flags; > > > > - if (!in_task() && !in_serving_softirq()) > > + if (!in_task() && !in_softirq_really()) > > return; > > > > local_lock_irqsave(&kcov_percpu_data.lock, flags); > > -- > > 2.25.1 > >