Received: by 2002:a05:6a10:8c0a:0:0:0:0 with SMTP id go10csp5804725pxb; Tue, 16 Feb 2021 08:02:32 -0800 (PST) X-Google-Smtp-Source: ABdhPJwqYizak03sELEHh16pSldtp6JYLE40w4uWb/ImFBFReWKiFu6iQ2ALqwq5BWO3Uor0vl/z X-Received: by 2002:aa7:db55:: with SMTP id n21mr1527625edt.258.1613491352200; Tue, 16 Feb 2021 08:02:32 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1613491352; cv=none; d=google.com; s=arc-20160816; b=UuVupWqD9KCqVmqmnE/7715K0IILJkYl3kRHlTQwaDZlROANcbg2d/AR2uNTAv6eTv VeqTQE2NhIh3oDiH+BL+z6Z0zvAFekNuS1fGmtLV+bYf8DfjAnjGT8tgF2ku9SvlVR7B H55b1Muke/fdPBKg5AYHO+zNTapzJQy59E0BSxjdKOQEjCQUavPhtPYAmJ89kf9JzAWi tbJUTr2LgPNuROC4kC2o6HopOHk0ZuEdWmuc/p8i1GAq3Lqoj9QKH0aUN1ZZLkXHMhia w9JOa0aESkYEDZ15aFqQtAqg88zE2bEQdE7ZewlcjzhKGHMkFRKNIkPICXitRoiHaAjt iI3Q== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:in-reply-to:content-disposition:mime-version :references:message-id:subject:cc:to:from:date:dkim-signature; bh=JipK6xkF+8oNavIVFVqoSZnucmL1iuE/CDXl5MQc5pU=; b=NsMbNPyjdKFQ8pN0KIK1UOTTwerUbJQd2lokastkzaYftqG7qspAuAqQUbD5CCtJeO SrKOHxKIeyIvBv4UtnwExA8ZOleGfUyaYfGo7mzjHf/xzLkPy08ggzs+3j9MxM1ngIix zldJlb4f3XmvvG0vN7EYhdNGAHMuxzYUMO5sPV+URVEhaSpIAM/WZBBnF79dOUcJKSOw 7ikU2FY1v+pOaA0xVchm/RTDs9W954V+Z4KknXdkNWA//gUbFBvQwCUMNARyMuwqT9Yq DBVw+LQVbyaoQjMkWbGrkGxBAKHIw71UTbDQqzaCVP7f3xYYBhdxSLlqe8QIKf9OdMu/ lCPw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@suse.com header.s=susede1 header.b=vMjqzU3j; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=QUARANTINE sp=NONE dis=NONE) header.from=suse.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id j10si19148365edj.128.2021.02.16.08.02.05; Tue, 16 Feb 2021 08:02:32 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@suse.com header.s=susede1 header.b=vMjqzU3j; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=QUARANTINE sp=NONE dis=NONE) header.from=suse.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230337AbhBPQBX (ORCPT + 99 others); Tue, 16 Feb 2021 11:01:23 -0500 Received: from mx2.suse.de ([195.135.220.15]:38208 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229708AbhBPQBO (ORCPT ); Tue, 16 Feb 2021 11:01:14 -0500 X-Virus-Scanned: by amavisd-new at test-mx.suse.de DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1613491228; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=JipK6xkF+8oNavIVFVqoSZnucmL1iuE/CDXl5MQc5pU=; b=vMjqzU3jM4ZE03eciuSLI8Yj8wdJrupjqdIZlTuUhziJRXvq3ThnvsqBz0ijeI+6NIlNqN UbEhVy98N5IOqDkbaevsm4AgzgG5OLA3OUyZvpFbGaYNpuL6F7IBaogBynVzpaXEBvqKsJ s54P6RSVumEs7cS05H/7r7hIyixsNG4= Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id CE113AE47; Tue, 16 Feb 2021 16:00:27 +0000 (UTC) Date: Tue, 16 Feb 2021 17:00:27 +0100 From: Petr Mladek To: Chris Down Cc: linux-kernel@vger.kernel.org, Sergey Senozhatsky , John Ogness , Johannes Weiner , Andrew Morton , Steven Rostedt , Greg Kroah-Hartman , Kees Cook , kernel-team@fb.com Subject: debugfs: was: Re: [PATCH v4] printk: Userspace format enumeration support Message-ID: References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri 2021-02-12 15:30:16, Chris Down wrote: > We have a number of systems industry-wide that have a subset of their > functionality that works as follows: > > 1. Receive a message from local kmsg, serial console, or netconsole; > 2. Apply a set of rules to classify the message; > 3. Do something based on this classification (like scheduling a > remediation for the machine), rinse, and repeat. > > As a couple of examples of places we have this implemented just inside > Facebook, although this isn't a Facebook-specific problem, we have this > inside our netconsole processing (for alarm classification), and as part > of our machine health checking. We use these messages to determine > fairly important metrics around production health, and it's important > that we get them right. > > > This patch provides a solution to the issue of silently changed or > deleted printks: we record pointers to all printk format strings known > at compile time into a new .printk_fmts section, both in vmlinux and > modules. At runtime, this can then be iterated by looking at > /printk/formats/, which emits the same format as > `printk` itself, which we already export elsewhere (for example, in > netconsole). > diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c > index 5a95c688621f..adf545ba9eb9 100644 > --- a/kernel/printk/printk.c > +++ b/kernel/printk/printk.c > + > +static const struct file_operations dfs_formats_fops = { > + .open = debugfs_pf_open, > + .read = seq_read, > + .llseek = seq_lseek, > + .release = single_release, > +}; > + > +static size_t printk_fmt_size(const char *fmt) > +{ > + size_t sz = strlen(fmt) + 1; > + > + /* > + * Some printk formats don't start with KERN_SOH + level. We will add > + * it later when rendering the output. > + */ > + if (unlikely(fmt[0] != KERN_SOH_ASCII)) > + sz += 2; This approach is hard to maintain. It might be pretty hard and error prone to count the size if we want to provide more information. There are many files in debugfs with not-well defined size. They are opened by seq_open_private(). It allows to add a line by line by an iterator. For example: + /sys/kernel/debug/dynamic_debug/control is opened by ddebug_proc_open() in lib/dynamic_debug.c + /sys/kernel/debug/tracing/available_filter_functions is opened by ftrace_avail_open() in kernel/trace/ftrace.c > + > + return sz; > +} > + > +static struct printk_fmt_sec *find_printk_fmt_sec(struct module *mod) > +{ > + struct printk_fmt_sec *ps = NULL; > + > + hash_for_each_possible(printk_fmts_mod_sections, ps, hnode, > + (unsigned long)mod) > + if (ps->module == mod) > + return ps; > + > + return NULL; > +} > + > +static void store_printk_fmt_sec(struct module *mod, const char **start, > + const char **end) > +{ > + struct printk_fmt_sec *ps = NULL; > + const char **fptr = NULL; > + size_t size = 0; > + > + ps = kmalloc(sizeof(struct printk_fmt_sec), GFP_KERNEL); > + if (!ps) > + return; > + > + ps->module = mod; > + ps->start = start; > + ps->end = end; > + > + for (fptr = ps->start; fptr < ps->end; fptr++) > + size += printk_fmt_size(*fptr); > + > + mutex_lock(&printk_fmts_mutex); > + hash_add(printk_fmts_mod_sections, &ps->hnode, (unsigned long)mod); > + mutex_unlock(&printk_fmts_mutex); > + > + ps->file = debugfs_create_file(ps_get_module_name(ps), 0444, > + dfs_formats, mod, &dfs_formats_fops); > + > + if (!IS_ERR(ps->file)) > + d_inode(ps->file)->i_size = size; We should revert the changes when the file could not get crated. It does not make sense to keep the structure when the file is not there. I guess that remove_printk_fmt_sec() would even crash when ps->file was set to an error code. > +} > + > +#ifdef CONFIG_MODULES > +static void remove_printk_fmt_sec(struct module *mod) > +{ > + struct printk_fmt_sec *ps = NULL; > + > + if (WARN_ON_ONCE(!mod)) > + return; > + > + mutex_lock(&printk_fmts_mutex); > + > + ps = find_printk_fmt_sec(mod); > + if (!ps) { > + mutex_unlock(&printk_fmts_mutex); > + return; > + } > + > + hash_del(&ps->hnode); > + > + mutex_unlock(&printk_fmts_mutex); > + > + debugfs_remove(ps->file); IMHO, we should remove the file before we remove the way how to read it. This should be done in the opposite order than in store_printk_fmt_sec(). > + kfree(ps); > +} > + Best Regards, Petr