From: Dave Wysochanski <dwysocha@redhat.com>
To: bfields@redhat.com
Cc: linux-nfs@vger.kernel.org
Subject: [PATCH 1/1] Simplify logic in cache_listeners_exist - only return true if someone has the file open.
Date: Wed, 4 Jun 2014 10:19:40 -0400 [thread overview]
Message-ID: <1401891580-28510-1-git-send-email-dwysocha@redhat.com> (raw)
The logic inside cache_listeners_exist contains heuristics to
determine whether there is a userspace process listening to the cache
file. If there is a listener, the kernel will send a request to service
the cache to userspace, and wait for a response.
The logic is a bit hard to read, here's some comments which explain the
existing logic:
/*
* If at least one process has the channel file open currently,
* someone is listening
*/
if (atomic_read(&detail->readers))
return true;
/*
* If no process has ever opened the channel file,
* no one is listening
*/
if (detail->last_close == 0)
/* This cache was never opened */
return false;
/*
* If the last time we closed the file was more than 30 seconds
* ago, no one is listening.
*/
if (detail->last_close < seconds_since_boot() - 30)
/*
* We allow for the possibility that someone might
* restart a userspace daemon without restarting the
* server; but after 30 seconds, we give up.
*/
return false;
/*
* In all other cases, assume someone is listening
*/
return true;
The logic is unduly complicated and unfortunately can be 'fooled' into
thinking some userspace listener daemon exists when it does not. For
example, we've seen where a simple diagnostic process reading all files
in /proc/net/rpc (for example, tarring the files up into an archive) can
fool the kernel due to this logic. Once fooled, the kernel will then
send requests to validate the cache to userspace thinking some 'cache listener'
exists, and will timeout. In the case of the nfs server, this leads to
silently dropped NFS requests due to failing RPC authentication.
A simple while loop as follows is enough to DoS the NFS server indefinitely:
while true; do
cat /proc/net/rpc/auth.unix.gid/channel>/dev/null
sleep 3
done
While a better userspace / kernel registration mechanism for cache listeners
would be the best solution, for now let's just simplify this logic by requiring
that there actually be someone holding the 'channel' file open for the kernel
to consider there's someone actually listening and servicing the cache.
The only downside is that now userspace daemons which restart will be noticed
by the kernel during the restart, but I think this makes sense since there's
no guarantee the listener will come back.
Signed-off-by: Dave Wysochanski <dwysocha@redhat.com>
---
net/sunrpc/cache.c | 14 +-------------
1 file changed, 1 insertion(+), 13 deletions(-)
diff --git a/net/sunrpc/cache.c b/net/sunrpc/cache.c
index ae333c1..d5adefc 100644
--- a/net/sunrpc/cache.c
+++ b/net/sunrpc/cache.c
@@ -1137,19 +1137,7 @@ static void warn_no_listener(struct cache_detail *detail)
static bool cache_listeners_exist(struct cache_detail *detail)
{
- if (atomic_read(&detail->readers))
- return true;
- if (detail->last_close == 0)
- /* This cache was never opened */
- return false;
- if (detail->last_close < seconds_since_boot() - 30)
- /*
- * We allow for the possibility that someone might
- * restart a userspace daemon without restarting the
- * server; but after 30 seconds, we give up.
- */
- return false;
- return true;
+ return atomic_read(&detail->readers);
}
/*
--
1.9.3
next reply other threads:[~2014-06-04 14:33 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-06-04 14:19 Dave Wysochanski [this message]
2014-06-04 21:03 ` [PATCH 1/1] Simplify logic in cache_listeners_exist - only return true if someone has the file open J. Bruce Fields
2014-06-06 19:40 ` J. Bruce Fields
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=1401891580-28510-1-git-send-email-dwysocha@redhat.com \
--to=dwysocha@redhat.com \
--cc=bfields@redhat.com \
--cc=linux-nfs@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).