Re: [patch 1/4] kref: warn kref_put() with unreferenced kref

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Apr 25, 2006 at 12:19:15AM -0400, [email protected] wrote:
> On Mon, 24 Apr 2006 20:51:28 PDT, Greg KH said:
> > On Mon, Apr 24, 2006 at 04:33:34PM +0800, Akinobu Mita wrote:
> 
> > > --- 2.6-git.orig/lib/kref.c
> > > +++ 2.6-git/lib/kref.c
> > > @@ -49,6 +49,7 @@ void kref_get(struct kref *kref)
> > >   */
> > >  int kref_put(struct kref *kref, void (*release)(struct kref *kref))
> > >  {
> > > +	WARN_ON(atomic_read(&kref->refcount) < 1);
> > 
> > How can this ever be true?  If the refcount _ever_ goes below 1, the
> > object is freed.
> 
> Maybe it should BUG_ON instead in that case. ;)
> 
> And strictly speaking, as long as the kref.c stuff is the only stuff to
> play with ->refcount, that *should* be true.  On the other hand, if somebody
> has a bad pointer that just did a fandango on core, it would be a nice thing
> to know that.  Looking at the *next* few lines:
> 
>         if ((atomic_read(&kref->refcount) == 1) ||
>             (atomic_dec_and_test(&kref->refcount))) {
>                 release(kref); 
>                 return 1; 
>         }    
>         return 0;
> 
> If we managed to get a -1 smashed in there, this won't actually trigger
> for another 2**32-2 or so kref_put calls - the first test is for "exactly 1",
> and the dec_and_test is for "exactly zero"...

Those two lines were recently added to make this test faster.  See the
archives for details.  If you are really worried about some memory
getting clobbered in there, we should worry about this for the entire
kernel :)

thanks,

greg k-h
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Stuff]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]     [Linux Resources]
  Powered by Linux