lkml.org 
[lkml]   [2010]   [Feb]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH 2/2] dlm: Remove obsolete lockspace lookup
On Thu, Feb 18, 2010 at 09:16:03AM +0000, Steven Whitehouse wrote:
> I'm not sure what more I can say here.... this is a sysfs file store
> function and one of the reasons for using it is that sysfs looks after
> the ref counting for you.
>
> Even aside from that, if you don't have a reference to the lockspace,
> then the dereference that is done to discover the lockspace name would
> be invalid, since the structure might have already been freed before the
> reference is obtained.
>
> You could also compare with with the other store and show functions in
> that same file and notice that none of them try to grab a reference to
> the lockspace in that way. So if this is required, then it must be
> required for those functions too.
>
> Either way there is something not quite right here and having studied
> the code in some detail, I'm pretty sure this is the correct fix,

I guess you didn't see this oops in your tests. Can you show that the
situation in this commit is no longer possible?

commit e2de7f565521a76fbbb927f701c5a1d381c71a93
Author: Patrick Caulfield <pcaulfie@redhat.com>
Date: Mon Nov 6 08:53:28 2006 +0000

[DLM] fix oops in kref_put when removing a lockspace

Now that the lockspace struct is freed when the last sysfs object is released
this patch prevents use of that lockspace by sysfs. We attempt to re-get the
lockspace from the lockspace list and fail the request if it has been removed.

Signed-Off-By: Patrick Caulfield <pcaulfie@redhat.com>
Signed-off-by: Steven Whitehouse <swhiteho@redhat.com>

diff --git a/fs/dlm/lockspace.c b/fs/dlm/lockspace.c
index 499ee11..f8842ca 100644
--- a/fs/dlm/lockspace.c
+++ b/fs/dlm/lockspace.c
@@ -43,6 +43,10 @@ static ssize_t dlm_control_store(struct dlm_ls *ls, const char *buf, size_t len)
ssize_t ret = len;
int n = simple_strtol(buf, NULL, 0);

+ ls = dlm_find_lockspace_local(ls->ls_local_handle);
+ if (!ls)
+ return -EINVAL;
+
switch (n) {
case 0:
dlm_ls_stop(ls);
@@ -53,6 +57,7 @@ static ssize_t dlm_control_store(struct dlm_ls *ls, const char *buf, size_t len)
default:
ret = -EINVAL;
}
+ dlm_put_lockspace(ls);
return ret;
}

>
> Steve.
>
> > > Signed-off-by: Steven Whitehouse <swhiteho@redhat.com>
> > > ---
> > > fs/dlm/lockspace.c | 6 +-----
> > > 1 files changed, 1 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/fs/dlm/lockspace.c b/fs/dlm/lockspace.c
> > > index 26a8bd4..ce0fdf5 100644
> > > --- a/fs/dlm/lockspace.c
> > > +++ b/fs/dlm/lockspace.c
> > > @@ -37,10 +37,6 @@ static ssize_t dlm_control_store(struct dlm_ls *ls, const char *buf, size_t len)
> > > ssize_t ret = len;
> > > int n = simple_strtol(buf, NULL, 0);
> > >
> > > - ls = dlm_find_lockspace_local(ls->ls_local_handle);
> > > - if (!ls)
> > > - return -EINVAL;
> > > -
> > > switch (n) {
> > > case 0:
> > > dlm_ls_stop(ls);
> > > @@ -51,7 +47,7 @@ static ssize_t dlm_control_store(struct dlm_ls *ls, const char *buf, size_t len)
> > > default:
> > > ret = -EINVAL;
> > > }
> > > - dlm_put_lockspace(ls);
> > > +
> > > return ret;
> > > }
> > >
> > > --
> > > 1.6.2.5
>


\
 
 \ /
  Last update: 2010-02-18 22:07    [W:0.053 / U:0.064 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site