All of lore.kernel.org
 help / color / mirror / Atom feed
From: Trond Myklebust <trond.myklebust@fys.uio.no>
To: Jeff Layton <jlayton@redhat.com>
Cc: Daniel J Blueman <daniel.blueman@gmail.com>,
	linux-nfs@vger.kernel.org, nfsv4@linux-nfs.org,
	Linux Kernel <linux-kernel@vger.kernel.org>
Subject: Re: [2.6.26-rc4] mount.nfsv4/memory poisoning issues...
Date: Tue, 10 Jun 2008 15:58:29 -0400	[thread overview]
Message-ID: <1213127909.20459.48.camel@localhost> (raw)
In-Reply-To: <20080610151357.150b6f69-RtJpwOs3+0O+kQycOl6kW4xkIHaj4LzF@public.gmane.org>

On Tue, 2008-06-10 at 15:13 -0400, Jeff Layton wrote:

> I think you're basically correct, but it looks to me like the
> nfs_callback_mutex actually protects nfs_callback_info.task as well.
> 
> If we're starting the thread, then we can't call kthread_stop on it
> until we release the mutex. So the thread can't exit until we release
> the mutex, and we can be guaranteed that this:
> 
>      nfs_callback_info.task = NULL;
> 
> ...can't happen until after kthread_run returns and nfs_callback_up
> sets it.
> 
> If that's right, then maybe this (untested, RFC only) patch would make sense?

Hmm... I suppose that is correct, but what if nfs_alloc_client() does

	nfs_callback_up();
	<kstrdup() fails>
	nfs_callback_down();

AFAICS, if nfs_callback_down() gets called before the kthread() function
gets scheduled back in, then you can get left with a value of
nfs_callback_info.task != NULL, since nfs_callback_svc() will never be
called.

Wouldn't it therefore make more sense to clear nfs_callback_info.task in
nfs_callback_down()?

Cheers
  Trond


WARNING: multiple messages have this Message-ID (diff)
From: Trond Myklebust <trond.myklebust@fys.uio.no>
To: Jeff Layton <jlayton@redhat.com>
Cc: Daniel J Blueman <daniel.blueman@gmail.com>,
	linux-nfs@vger.kernel.org, nfsv4@linux-nfs.org,
	Linux Kernel <linux-kernel@vger.kernel.org>
Subject: Re: [2.6.26-rc4] mount.nfsv4/memory poisoning issues...
Date: Tue, 10 Jun 2008 15:58:29 -0400	[thread overview]
Message-ID: <1213127909.20459.48.camel@localhost> (raw)
In-Reply-To: <20080610151357.150b6f69@tleilax.poochiereds.net>

On Tue, 2008-06-10 at 15:13 -0400, Jeff Layton wrote:

> I think you're basically correct, but it looks to me like the
> nfs_callback_mutex actually protects nfs_callback_info.task as well.
> 
> If we're starting the thread, then we can't call kthread_stop on it
> until we release the mutex. So the thread can't exit until we release
> the mutex, and we can be guaranteed that this:
> 
>      nfs_callback_info.task = NULL;
> 
> ...can't happen until after kthread_run returns and nfs_callback_up
> sets it.
> 
> If that's right, then maybe this (untested, RFC only) patch would make sense?

Hmm... I suppose that is correct, but what if nfs_alloc_client() does

	nfs_callback_up();
	<kstrdup() fails>
	nfs_callback_down();

AFAICS, if nfs_callback_down() gets called before the kthread() function
gets scheduled back in, then you can get left with a value of
nfs_callback_info.task != NULL, since nfs_callback_svc() will never be
called.

Wouldn't it therefore make more sense to clear nfs_callback_info.task in
nfs_callback_down()?

Cheers
  Trond


  parent reply	other threads:[~2008-06-10 19:58 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-06-04 23:33 [2.6.26-rc4] mount.nfsv4/memory poisoning issues Daniel J Blueman
2008-06-04 23:33 ` Daniel J Blueman
     [not found] ` <6278d2220806041633n3bfe3dd2ke9602697697228b-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2008-06-04 23:43   ` Chuck Lever
2008-06-04 23:43     ` Chuck Lever
     [not found]     ` <76bd70e30806041643j4d632a6exf64b29c34173d40f-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2008-06-15 18:10       ` Daniel J Blueman
2008-06-15 18:10         ` Daniel J Blueman
2008-06-16 16:17         ` Chuck Lever
2008-06-16 16:17           ` Chuck Lever
     [not found]         ` <6278d2220806151110x68ee91fej8cf8e6b591ce1319-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2008-06-19 12:14           ` Jeff Layton
2008-06-19 12:14             ` Jeff Layton
     [not found]             ` <20080619081420.24645bc4-RtJpwOs3+0O+kQycOl6kW4xkIHaj4LzF@public.gmane.org>
2008-06-19 12:37               ` Daniel J Blueman
2008-06-19 12:37                 ` Daniel J Blueman
     [not found]                 ` <6278d2220806190537u7b781309q415f904390e02f3-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2008-06-19 17:32                   ` Chuck Lever
2008-06-19 17:32                     ` Chuck Lever
2008-06-05  0:35 ` Jeff Layton
2008-06-05  8:28   ` Daniel J Blueman
     [not found]     ` <6278d2220806050128x6e892df3p1632d6ae6b40b55b-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2008-06-05 10:32       ` Jeff Layton
2008-06-05 10:32         ` Jeff Layton
     [not found]   ` <20080604203504.62730951-RtJpwOs3+0O+kQycOl6kW4xkIHaj4LzF@public.gmane.org>
2008-06-10 18:54     ` Trond Myklebust
2008-06-10 18:54       ` Trond Myklebust
2008-06-10 19:13       ` Jeff Layton
     [not found]         ` <20080610151357.150b6f69-RtJpwOs3+0O+kQycOl6kW4xkIHaj4LzF@public.gmane.org>
2008-06-10 19:18           ` Jeff Layton
2008-06-10 19:18             ` Jeff Layton
     [not found]             ` <20080610151829.3c4d6c1e-RtJpwOs3+0O+kQycOl6kW4xkIHaj4LzF@public.gmane.org>
2008-06-10 20:27               ` Daniel J Blueman
2008-06-10 20:27                 ` Daniel J Blueman
2008-06-18 12:07                 ` Jeff Layton
2008-06-18 12:07                   ` Jeff Layton
2008-06-21 17:52                   ` Daniel J Blueman
2008-06-21 17:52                     ` Daniel J Blueman
2008-06-10 19:58           ` Trond Myklebust [this message]
2008-06-10 19:58             ` Trond Myklebust
2008-06-10 20:13             ` Jeff Layton
2008-06-10 20:33               ` Trond Myklebust
2008-06-10 20:33                 ` Trond Myklebust
2008-06-10 20:41                 ` Jeff Layton
2008-06-10 20:41                   ` Jeff Layton
2008-06-10 21:01                 ` Jeff Layton
2008-06-10 21:01                   ` Jeff Layton
2008-06-10 21:37                   ` Trond Myklebust
2008-06-10 21:37                     ` Trond Myklebust
2008-06-10 22:04                     ` Jeff Layton

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=1213127909.20459.48.camel@localhost \
    --to=trond.myklebust@fys.uio.no \
    --cc=daniel.blueman@gmail.com \
    --cc=jlayton@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=nfsv4@linux-nfs.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.