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 D0DD2CD379F for ; Tue, 3 Sep 2024 17:35:27 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 1ED258D01B4; Tue, 3 Sep 2024 13:35:27 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 14DAA8D018A; Tue, 3 Sep 2024 13:35:27 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id E6E0E8D01B4; Tue, 3 Sep 2024 13:35:26 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id B50048D018A for ; Tue, 3 Sep 2024 13:35:26 -0400 (EDT) Received: from smtpin27.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 6007580633 for ; Tue, 3 Sep 2024 17:35:26 +0000 (UTC) X-FDA: 82524128652.27.B426BC6 Received: from mail-pj1-f53.google.com (mail-pj1-f53.google.com [209.85.216.53]) by imf24.hostedemail.com (Postfix) with ESMTP id 8FC91180007 for ; Tue, 3 Sep 2024 17:35:24 +0000 (UTC) Authentication-Results: imf24.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=m4ea14Hw; spf=pass (imf24.hostedemail.com: domain of andrii.nakryiko@gmail.com designates 209.85.216.53 as permitted sender) smtp.mailfrom=andrii.nakryiko@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1725384798; 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:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=l3hUXgLjalL7lAeUT4m6rvI6dnqff1plFSiWYGGCoE0=; b=1MHLJ6BDp3p9lkXQZ2MTUT8qqMBK4CdVQxVNI4ydbjrj2fAGU+JoBKNrjX+6omexShVU4K 81gi7waFCqk23773o3S3rvfJ0BqbTZRIktIwXixNpTw8SNKf6HQnOSJ0zW+frIFDxIv79N 9TWP777TsSfJnLavWzypcR9aqOkF+yY= ARC-Authentication-Results: i=1; imf24.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=m4ea14Hw; spf=pass (imf24.hostedemail.com: domain of andrii.nakryiko@gmail.com designates 209.85.216.53 as permitted sender) smtp.mailfrom=andrii.nakryiko@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1725384798; a=rsa-sha256; cv=none; b=tgfL/3EE5BVG/VV3bwrCa6YIMqfoSHttu6tywfwgtSkRo0S2VrObS+K3wRM2P8eglrKp9d C+nTjdR2banHjdWWAH+P+DM+ty5L8qZewwH1nbJTdkEM4zoY2b3VN9m664AqG9k+12eKqA sYacVlnevYZ+5ironRJzsn9mcHjWWlc= Received: by mail-pj1-f53.google.com with SMTP id 98e67ed59e1d1-2d8fa2ca5b1so1247585a91.3 for ; Tue, 03 Sep 2024 10:35:24 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1725384923; x=1725989723; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=l3hUXgLjalL7lAeUT4m6rvI6dnqff1plFSiWYGGCoE0=; b=m4ea14HwjlkTbf8MqSgoq6HEfLdfl5EEs+JvXww/YBrsO+pQlM8P4VwFWE8dPjF7jf NPsUD4l5z4BuB0rTFckMa7Dz08UaQv3C26iT8F3BzT1AhHZFM2x4r7Ior2ibk+hixuu1 k2sFpx5mdPu6zo+008X5c2BROVqYlFk4pcG7YjTIs84XjnRVgDQxjrOKi9BwCh+Y6IEx 9TU2TtnzLNWHHGhQG9+kMIy8y9vcgkbG/+YkNlPPApDUCy1G3CSK/s95O9J46Pheq5Iv 9YMWyTNyjveVgSPPq3lfxOgmP8oTOzeR7anoYhfAiSBjIieeMYtqUL2LoG8QKovXpi82 uGFg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1725384923; x=1725989723; h=content-transfer-encoding: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=l3hUXgLjalL7lAeUT4m6rvI6dnqff1plFSiWYGGCoE0=; b=OeWuHSh+rwNOQtHZTL0tUvGZE2P9Uk3l5PYvuIYpGdkKgvlwmKYQOXbFmqV+4pM//5 2lWStkWnTswWsvp6K5ug9uP26oaMrSqRqxpxRbZG0xt1cwa++R9L6bbk0n8rMrxG9gfa nTUC8413nG5gvALBv0bvDj7s4nYq6Hz5dpWkZ7ouIDPDlZg3LLPSIhCXE9Q2D1oJizt2 IjLqATx0V5t3GGRoRTiYHpY9jSXDWvaMH9zdgzRz9AoEsgIT1RIAl4ewgZZw21WcU5J6 luurLbuvVCo1sXiFK/LJWb40mLUjhqZsKbI0bkXtfLzyR51QpmLpWBvRyPgCzD+smWZS E7qg== X-Forwarded-Encrypted: i=1; AJvYcCXTshx+9Yb2OHuI0fW/5POqB0SBeh3QAXYE5e/hUx787lc83Z5DIqgdp4lJBjo1tJty3npgBpejow==@kvack.org X-Gm-Message-State: AOJu0Yxhf9TX99ERvlJCXvQ08RypjvQ6HtC+SRTdcfClN+dutgehAcvF Dxw5+OqeqEuYIDki75bGsdX2CQSbfvtdATnwuzqpYrVNlYH5mWlCYuppUxFCla6zzhyTeFTOO/b 1iqDb8aTuw55I5V5bR0Udmke8G+4= X-Google-Smtp-Source: AGHT+IE2Caw/xhU7eXyirtFUOj3URIayxgMSRZGkemLmVzQAwmUHrOMrd18peOQe/Wv611jgoKooKYSc1l00fOUfgnU= X-Received: by 2002:a17:90a:6fa2:b0:2d3:bfc3:3ef3 with SMTP id 98e67ed59e1d1-2d88d6af3dcmr11971100a91.12.1725384923145; Tue, 03 Sep 2024 10:35:23 -0700 (PDT) MIME-Version: 1.0 References: <20240829183741.3331213-1-andrii@kernel.org> <20240829183741.3331213-5-andrii@kernel.org> <20240830143151.GC20163@redhat.com> <20240830202050.GA7440@redhat.com> <20240831161914.GA9683@redhat.com> In-Reply-To: From: Andrii Nakryiko Date: Tue, 3 Sep 2024 10:35:10 -0700 Message-ID: Subject: Re: [PATCH v4 4/8] uprobes: travers uprobe's consumer list locklessly under SRCU protection To: Oleg Nesterov Cc: Jiri Olsa , Andrii Nakryiko , linux-trace-kernel@vger.kernel.org, peterz@infradead.org, rostedt@goodmis.org, mhiramat@kernel.org, bpf@vger.kernel.org, linux-kernel@vger.kernel.org, paulmck@kernel.org, willy@infradead.org, surenb@google.com, akpm@linux-foundation.org, linux-mm@kvack.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: 8FC91180007 X-Stat-Signature: tg9dfiyw4js746gwa4q879t79f7tj3pj X-Rspam-User: X-HE-Tag: 1725384924-707500 X-HE-Meta: U2FsdGVkX1+JjC2J9pbAewMdcl22/LJXeJ5UHd/YYnBsrxMtL3pMB0/k4Ksvr1hIKICYNAfzk0+gNOy5T25fNqoR50UaFUXkVVKYZA6TII07CWQ5lUiVh/pdHJquRQsUSMIHLCF6MW/TsfOaaMGSpabG60FRJCba/19mF6Nug65TUvszp438NxdUhPfqYf6QpCFWItpypgRtTIQKDr7hGCCijYLpMJjANs/M6zoKAJXmaSIJApx9BhbsKtHQKDDVv2ujx4yFe1kAMvGEqR1pXWI7znbbg5rAtOyXMQuk7tGs+uphiFij2OkCrfFxmSz5BHtfxY+fF5zcev80ZgYNtiLZB6Bjb268OlIdIdYe83UWkijPaM7bJIgyk53zxDDTW5M34eZuWzfZwm+3Oq1aK2hA4Y9WAJQmVJjLR3GyqEumuZDZ28RMhGKbBdU8FTbOLxq1T3ZnVx/fsmyjsHbSoB6FMUgtI+ansX7MwE8OllfZjge0j+03Awr0KOhVdVqZ+5t+BKArDIQDP3TsD+T/57x8Id77wTkCo2j12wXy8502UitUUZ2cC2CSWC+c4GdVVgbG1AuaTvcRpj09IO1bkPsL69Rbmm9CTCVAQKi6oGurJcomrQ/zjh7Y4awOmdYgB7g96NWJ4NwdYoOPQ4QDx7D+WdB1x31Yoa3QZ8gynFbrBjZ2rECvUYhn9rySa/wFhaPpk0MLecVXJP1W96C8ofU2muEHKsenVTYb0N1WWIyoT4jUd/McNf7EbeAoWfwOVdx8uLZE7ki0QI+vXozpYpRQbClAqHuLNmCGLOJNr6/tMhKnrsvNzjhTqQAekwb0Fx4A+gdgqnaQtFiuPAjUz6rqzP8YBC7DsE0cXkRgO68dpjMo0eLHWtDOBLZ60VwOFIBwzxu04VVrM+wwoKcehjwgg3CxCcKduj2pROKZAgmageXyZ2I2mE5DZUbKEpzYCeV/3yxtf3xd/AT8kDO w1W7TgrN QTI5xlo++/Y6U92684HO7S2RHBGj9zQHqbNw3+vZVozMp+I5uLg6t7ArMNEvOkC5JIFZ6gWMoTxbS72Jn+yiiipMgvJMckEpJk//7UcABk+PGfIgdkRDxpC+gEtyLQaqg5KiekT0kFLSInHIqvKgfCgvAvExBYx+iQQqc884WV1ZJe6Je6sK580p3TxFhGgcpO/g2T5RVOVlm8/rcDPz1I5dIMf3MaiG7tsl+xv38DGGbxeXP9JRJOpnXG11qYy0rwPIEU8eV50iC0Ml7dsPmmJ4oVr47HZJNkbTHmW0HWBV1Q9IcdfPONWR7aYf1fXhWiXthWFsL2/1azM1n4uxpshWGcOgXsdNqSlIEaKMJxhukRL029reKzCJ5SI0vm9ZHqed/FYZvQTBn3atI7G2svcpKrg== 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 Tue, Sep 3, 2024 at 10:27=E2=80=AFAM Andrii Nakryiko wrote: > > On Sat, Aug 31, 2024 at 9:19=E2=80=AFAM Oleg Nesterov w= rote: > > > > On 08/30, Andrii Nakryiko wrote: > > > > > > On Fri, Aug 30, 2024 at 1:21=E2=80=AFPM Oleg Nesterov wrote: > > > > > > > > I'll probably write another email (too late for me today), but I ag= ree > > > > that "avoid register_rwsem in handler_chain" is obviously a good go= al, > > > > lets discuss the possible cleanups or even fixlets later, when this > > > > series is already applied. > > > > > > > > > > Sounds good. It seems like I'll need another revision due to missing > > > include, so if there is any reasonably straightforward clean up we > > > should do, I can just incorporate that into my series. > > > > I was thinking about another seq counter incremented in register(), so > > that handler_chain() can detect the race with uprobe_register() and ski= p > > unapply_uprobe() in this case. This is what Peter did in one of his ser= ies. > > Still changes the current behaviour, but not too much. > > We could do that, but then worst case, when we do detect registration > race, what do we do? We still have to do the same. So instead of > polluting the logic with seq counter it's best to just codify the > protocol and take advantage of that. > > But as you said, this all can/should be addressed as a follow up > discussion. You mentioned some clean ups you wanted to do, let's > discuss all that as part of that? > > > > > But see below, > > > > > I still think it's fine, tbh. > > > > and perhaps you are right, > > > > > Which uprobe user violates this contract > > > in the kernel? > > > > The only in-kernel user of UPROBE_HANDLER_REMOVE is perf, and it is fin= e. > > > > Well, BPF program can accidentally trigger this as well, but that's a > bug, we should fix it ASAP in the bpf tree. > > > > But there are out-of-tree users, say systemtap, I have no idea if this > > change can affect them. > > > > And in general, this change makes the API less "flexible". > > it maybe makes a weird and too-flexible case a bit more work to > implement. Because if consumer want to be that flexible, they can > still define filter that will be coordinated between filter() and > handler() implementation. > > > > > But once again, I agree that it would be better to apply your series fi= rst, > > then add the fixes in (unlikely) case it breaks something. > > Yep, agreed, thanks! Will send a new version ASAP, so we have a common > base to work on top of. > > > > > But. Since you are going to send another version, may I ask you to add = a > > note into the changelog to explain that this patch assumes (and enforce= s) > > the rule about handler/filter consistency? > > Yep, will do. I will also leave a comment next to the filter callback > definition in uprobe_consumer about this. > Ok, I'm adding this: diff --git a/include/linux/uprobes.h b/include/linux/uprobes.h index 29c935b0d504..33236d689d60 100644 --- a/include/linux/uprobes.h +++ b/include/linux/uprobes.h @@ -29,6 +29,14 @@ struct page; #define MAX_URETPROBE_DEPTH 64 struct uprobe_consumer { + /* + * handler() can return UPROBE_HANDLER_REMOVE to signal the need to + * unregister uprobe for current process. If UPROBE_HANDLER_REMOVE = is + * returned, filter() callback has to be implemented as well and it + * should return false to "confirm" the decision to uninstall uprob= e + * for the current process. If filter() is omitted or returns true, + * UPROBE_HANDLER_REMOVE is effectively ignored. + */ int (*handler)(struct uprobe_consumer *self, struct pt_regs *regs); int (*ret_handler)(struct uprobe_consumer *self, unsigned long func, > > > > Oleg. > >