On Tue, 05 Oct 2010 16:00:29 +0530
Suresh Jayaraman <[email protected]> wrote:

> On 10/04/2010 11:22 PM, Jeff Layton wrote:
> > To prevent races we need to be able to do an atomic_dec_and_lock with
> > it, and that won't work with a rwlock.
> 
> It's not clear to me where a potential race window is. Could you please
> elaborate?
> 
> 
> Thanks,
> 

The specific race window that I was thinking about doesn't exist today
as such, but that's just because the way that filehandles are managed
is so different.

When we do refcounting with atomic_t's though we need to take care that
we don't act on them without taking steps to prevent races. For
instance, suppose I had left the code using an rwlock_t and did this in
cifsFileInfo_put:

if (atomic_dec_and_test(&open_file->count))
        return;
write_lock(&GlobalSMBSeslock)

...and then tear down the cifsFileInfo. There would always be a
possibility that the count increased after I checked it but before
taking the lock, which could result in a use-after-free.

Now that I think about this though...it might be better to just make the
count non-atomic, and simply move it under the protection of the lock.
You generally need the lock anyway when you're searching for a file so
there's little benefit to using an atomic here.

-- 
Jeff Layton <[email protected]>
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to