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 X-Spam-Level: X-Spam-Status: No, score=-6.0 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,NICE_REPLY_A, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9DBD0C4361B for ; Wed, 9 Dec 2020 13:52:15 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 5EFD023B1F for ; Wed, 9 Dec 2020 13:52:15 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1732440AbgLINv7 (ORCPT ); Wed, 9 Dec 2020 08:51:59 -0500 Received: from mail.kernel.org ([198.145.29.99]:50358 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726555AbgLINv6 (ORCPT ); Wed, 9 Dec 2020 08:51:58 -0500 Date: Wed, 9 Dec 2020 22:51:14 +0900 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1607521877; bh=/LQG4cARkA17CaFYwLH0PCTQMyl71BRL/5TpniO12f4=; h=From:To:Cc:Subject:In-Reply-To:References:From; b=hOdIma+vmYQhM0v5m8mn082Sb/t63aSEcKu9Aqh8lW5eCnxcYDcM4Bb5XnUCouK/i lJgdYK+hmAn416FqDVphDKiRBk5x4Yjr4jqKHjiHX4nLc8Ob9t2IIcSe+sXBH7bUVl 7KmOI+nbYDPvsjvegcyblrmF9/qc0uhdLNQgJxu4WbHs8sPGhH1EZ6VEO5R/70y41J pR6VgazP7aEc1Y+MYim2Q0g0YncJbgsYqx693fgRqkwEnfpVc9+5E5S+tpBjRa9Pr/ Q6JpkUOy23JIINlpGa0X4aFb6bX6Bxku8chzYIfntiFBK9cETsIeSdIFU8W38nxsBO Lj0NHohmFjOIA== From: Masami Hiramatsu To: Steven Rostedt Cc: Tom Zanussi , axelrasmussen@google.com, mhiramat@kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v3 3/5] tracing: Update synth command errors Message-Id: <20201209225114.9bb1f53caa5bc101a6e31a4b@kernel.org> In-Reply-To: <20201208125340.407150f2@gandalf.local.home> References: <8671adc7ce95ff1d5c7b037d371467e96f7f2914.1603723933.git.zanussi@kernel.org> <20201207201304.627bfe48@oasis.local.home> <44b9e471f0d3b77ab0a2bf11024e2e72c1f1a80d.camel@kernel.org> <20201208125340.407150f2@gandalf.local.home> X-Mailer: Sylpheed 3.7.0 (GTK+ 2.24.32; x86_64-pc-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, 8 Dec 2020 12:53:40 -0500 Steven Rostedt wrote: > On Tue, 08 Dec 2020 11:34:41 -0600 > Tom Zanussi wrote: > > > Unfortunately, you're correct, if you have a script that creates a > > synthetic event without semicolons, this patchset will break it, as I > > myself found out and fixed in patch 4 ([PATCH v3 4/5] selftests/ftrace: > > Add synthetic event field separators) [4]. > > > > So whereas before this would work, even though it shouldn't have in the > > first place: > > > > # echo 'wakeup_latency u64 lat pid_t pid char comm[16]' > > > synthetic_events > > > > it now has to be: > > > > # echo 'wakeup_latency u64 lat; pid_t pid; char comm[16]' > > > synthetic_events > > > > So yeah, this patchset fixes a set of parsing bugs for things that > > shouldn't have been accepted as valid, but shouldn't break things that > > are obviously valid. > > > > If it's too late to fix them, though, I guess we'll just have to live > > with them, or some other option? > > > I would suggest allowing the old interface work (with no new features, for > backward compatibility), but new things like "char comm[16]" we require > semicolons. > > One method to do this is to add to the start of reading the string, and > checking if it has semicolons. If it does not, we create a new string with > them, but make sure that the string does not include new changes. > > strncpy_from_user(buffer, user_buff, sizeof(buffer)); > > if (!strstr(buffer, ";")) { > if (!audit_old_buffer(buffer)) > goto error; > insert_colons(buffer); > } > > > That is, if the buffer does not have semicolons, then check if it is a > valid "old format", and if not, we error out. Otherwise, we insert the > colons into the buffer, and process that as if the user put in colons: > > That is: > > echo 'wakeup_latency u64 lat pid_t pid' > synthetic_events > > would change the buffer to: > > "wakeup_latency u64 lat; pid_t pid;" > > And then put it through the normal processing. I think its OK that if the > user were to cat out the synthetic events, it would see the semicolons even > if it did not add them. As I don't think that will break userspace. > > Does that make sense? This makes sense. Anyway, what I considered were - synthetic_events interface doesn't provide syntax error reports - synthetic_events interface is not self-reproducive*. *) I meant $ cat synthetic_events > saved_events $ cat saved_events > synthetic_events should work. But this does *NOT* mean $ cat user-input > synthetic_events $ cat synthetic_events > saved_events $ diff user-input saved_events # no diff So input and output can be different, but the output can be input again. Thank you, -- Masami Hiramatsu