On Tue, 2017-03-14 at 12:29 +0000, Reshetova, Elena wrote:

quoted

quoted

Elena Reshetova [off-list ref] writes:

quoted

refcount_t type and corresponding API should be
used instead of atomic_t when the variable is used as
a reference counter. This allows to avoid accidental
refcounter overflows that might lead to use-after-free
situations.

Signed-off-by: Elena Reshetova <redacted>
Signed-off-by: Hans Liljestrand <redacted>
Signed-off-by: Kees Cook <redacted>
Signed-off-by: David Windsor <redacted>
---
 drivers/md/md.c | 6 +++---
 drivers/md/md.h | 3 ++-
 2 files changed, 5 insertions(+), 4 deletions(-)

When booting linux-next (specifically 5be4921c9958ec) I'm seeing
the
backtrace below. I suspect this patch is just exposing an existing
issue?

Yes, we have actually been following this issue in the another
thread.
It looks like the object is re-used somehow, but I can't quite
understand how just by reading the code.
This was what I put into the previous thread:

"The log below indicates that you are using your refcounter in a bit
weird way in mddev_find().
However, I can't find the place (just by reading the code) where you
would increment refcounter from zero (vs. setting it to one).
It looks like you either iterate over existing nodes (and increment
their counters, which should be >= 1 at the time of increment) or
create a new node, but then mddev_init() sets the counter to 1. "

If you can help to understand what is going on with the object
creation/destruction, would be appreciated!

Also Shaohua Li stopped this patch coming from his tree since the
issue was caught at that time, so we are not going to merge this
until we figure it out.

Asking on the correct list (dm-devel) would have got you the easy
answer:  The refcount behind mddev->active is a genuine atomic.  It has
refcount properties but only if the array fails to initialise (in that
case, final put kills it).  Once it's added to the system as a gendisk,
it cannot be freed until md_free().  Thus its ->active count can go to
zero (when it becomes inactive; usually because of an unmount). On a
simple allocation regardless of outcome, the last executed statement in
md_alloc is mddev_put(): that destroys the device if we didn't manage
to create it or returns 0 and adds an inactive device to the system
which the user can get with mddev_find().

Thank you James for explaining this! I guess in this case, the conversion doesn't make sense. 
And sorry about not asking in a correct place: we are handling many similar patches now and while I try to reach the right audience using get_maintainer script, it doesn't always succeeds. 

Best Regards,
Elena.

James

-- 
You received this message because you are subscribed to the Google Groups "open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to open-iscsi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
To post to this group, send email to open-iscsi-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
Visit this group at https://groups.google.com/group/open-iscsi.
For more options, visit https://groups.google.com/d/optout.