Project

General

Profile

Actions

Bug #2933

closed

rbd: bio_pair leak in bio_chain_clone()

Added by Alex Elder over 11 years ago. Updated over 11 years ago.

Status:
Resolved
Priority:
Normal
Assignee:
Target version:
-
% Done:

0%

Source:
Development
Tags:
Backport:
Regression:
Severity:
Reviewed:
Affected Versions:
ceph-qa-suite:
Pull request ID:
Crash signature (v1):
Crash signature (v2):

Description

Guangliang Zhao <> pointed out this problem on the
mailing list. Here's the latest edition of his proposed fix:
http://www.spinics.net/lists/ceph-devel/msg08008.html

When bio_split() is called in rbd.c:bio_chain_clone() it returns
a bio_pair pointer. But what we do with it is not right, and as
a result the bio_pair is leaked.

All the other places callers use bio_split(), they are immediately
followed by a sequence, something like:

bp = bio_split(...)
make_request(dev, &bp->bio1);
make_request(dev, &bp->bio2);
bio_pair_release(bp);

When bio_split() is called, it sets the reference count on the
returned bio_pair to 3 and sets up the callbacks for bp->bio1
and bp->bio2 will have been set up so they result in a call to
bio_pair_release(). Thus, all three releases for the references
the bio_pair is initialized with are accounted for, and whichever
path drops the reference last will free the bio_pair structure.

Instead, the rbd code does something more like:

bp = bio_split(...);
__bio_clone(new, &bp->bio1);
...
bio_pair_release(bp);

I'm not even sure what the outcome of this is with respect to
the bio pair...

In any case the structure of the rbd code isn't quite like the
other callers of bio_split() so it may be a little more involved
get things to be done correctly. We call bio_split() in
bio_chain_clone(), which is used to clone an entire chain of
bios, whereas the other users of bio_split() are handling a
single bio.

It may be that one of the two bio chains resulting from the
bio_chain_clone() call should end with the first bio in the
bio_pair structure, and the other should begin with the
second one, and that will result in the desired behavior.

But it'll take a little study to make sure it gets done right.

We may get an updated patch from Guangliang Zhao; his last one
had at least one bug in it.

Actions

Also available in: Atom PDF