All of lore.kernel.org
 help / color / mirror / Atom feed
From: Luis Chamberlain <mcgrof@kernel.org>
To: NeilBrown <neilb@suse.de>
Cc: Eric Biggers <ebiggers@kernel.org>,
	linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	Alexei Starovoitov <ast@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Jeff Vander Stoep <jeffv@google.com>,
	Jessica Yu <jeyu@kernel.org>, Kees Cook <keescook@chromium.org>,
	NeilBrown <neilb@suse.com>,
	stable@vger.kernel.org
Subject: Re: [PATCH v2 2/4] fs/filesystems.c: downgrade user-reachable WARN_ONCE() to pr_warn_once()
Date: Fri, 13 Mar 2020 01:00:53 +0000	[thread overview]
Message-ID: <20200313010053.GS11244@42.do-not-panic.com> (raw)
In-Reply-To: <87imj9teh5.fsf@notabene.neil.brown.name>

[-- Attachment #1: Type: text/plain, Size: 2195 bytes --]

On Fri, Mar 13, 2020 at 09:06:46AM +1100, NeilBrown wrote:
> On Thu, Mar 12 2020, Eric Biggers wrote:
> 
> > From: Eric Biggers <ebiggers@google.com>
> >
> > After request_module(), nothing is stopping the module from being
> > unloaded until someone takes a reference to it via try_get_module().
> >
> > The WARN_ONCE() in get_fs_type() is thus user-reachable, via userspace
> > running 'rmmod' concurrently.
> >
> > Since WARN_ONCE() is for kernel bugs only, not for user-reachable
> > situations, downgrade this warning to pr_warn_once().
> >
> > Cc: stable@vger.kernel.org
> > Cc: Alexei Starovoitov <ast@kernel.org>
> > Cc: Andrew Morton <akpm@linux-foundation.org>
> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Cc: Jeff Vander Stoep <jeffv@google.com>
> > Cc: Jessica Yu <jeyu@kernel.org>
> > Cc: Kees Cook <keescook@chromium.org>
> > Cc: Luis Chamberlain <mcgrof@kernel.org>
> > Cc: NeilBrown <neilb@suse.com>
> > Signed-off-by: Eric Biggers <ebiggers@google.com>
> > ---
> >  fs/filesystems.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/filesystems.c b/fs/filesystems.c
> > index 77bf5f95362da..90b8d879fbaf3 100644
> > --- a/fs/filesystems.c
> > +++ b/fs/filesystems.c
> > @@ -272,7 +272,9 @@ struct file_system_type *get_fs_type(const char *name)
> >  	fs = __get_fs_type(name, len);
> >  	if (!fs && (request_module("fs-%.*s", len, name) == 0)) {
> >  		fs = __get_fs_type(name, len);
> > -		WARN_ONCE(!fs, "request_module fs-%.*s succeeded, but still no fs?\n", len, name);
> > +		if (!fs)
> > +			pr_warn_once("request_module fs-%.*s succeeded, but still no fs?\n",
> > +				     len, name);
> 
> I strongly support the replacement of "WARN" by "pr_warn".
> I wonder if we really want the "once" now.  Possibly using rate_limited
> would be justified, but I think that in general we should see a warning
> every time this event happens.

Since the usefulness of the print is at boot, I think pr_warn_once() is
good right now but just because I cannot think of a case where multiple
prints are currently desirable, or where this should be possible
post-boot. Can you?

  Luis

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  reply	other threads:[~2020-03-13  1:00 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-12 20:25 [PATCH v2 0/4] module autoloading fixes and cleanups Eric Biggers
2020-03-12 20:25 ` [PATCH v2 1/4] kmod: make request_module() return an error when autoloading is disabled Eric Biggers
2020-03-13  0:57   ` Luis Chamberlain
2020-03-12 20:25 ` [PATCH v2 2/4] fs/filesystems.c: downgrade user-reachable WARN_ONCE() to pr_warn_once() Eric Biggers
2020-03-12 22:06   ` NeilBrown
2020-03-13  1:00     ` Luis Chamberlain [this message]
2020-03-13  0:58   ` Luis Chamberlain
2020-03-12 20:25 ` [PATCH v2 3/4] docs: admin-guide: document the kernel.modprobe sysctl Eric Biggers
2020-03-12 22:04   ` NeilBrown
2020-03-13  1:07     ` Luis Chamberlain
2020-03-13 19:05       ` Eric Biggers
2020-03-13 23:31         ` Luis Chamberlain
2020-03-12 20:25 ` [PATCH v2 4/4] selftests: kmod: test disabling module autoloading Eric Biggers
2020-03-13  1:09   ` Luis Chamberlain

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200313010053.GS11244@42.do-not-panic.com \
    --to=mcgrof@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=ast@kernel.org \
    --cc=ebiggers@kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=jeffv@google.com \
    --cc=jeyu@kernel.org \
    --cc=keescook@chromium.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=neilb@suse.com \
    --cc=neilb@suse.de \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.