Hello!
On Dec 3, 2007, at 12:00 PM, Felix Blyakher wrote:
Per our discussion, I am resending this patch that fixes a leak
in nlmsvc_testlock.
It is addition to another leak fixing patch you already have.
Without the patch, there is a leakage of nlmblock structure
refcount that holds a
reference nlmfile structure, that holds a reference to struct
file, when async GETFL
is used (-EINPROGRESS return from file_ops->lock()), and also in
some error cases.
> @@ -502,15 +509,19 @@
> }
> else {
> nlmsvc_unlink_block(block);
> - return nlm_granted;
> + ret = nlm_granted;
> + goto out;
> }
Do we really need to release block with 'goto out' here?
Yes, absolutely.
nlmsvc_unlink_block() is doing it internally in the following
call chain:
...
And on a last reference 'block' will be freed in nlmsvc_free_block.
Are you sure we have an extra reference here and won't go through
nlmsvc_free_block?
It releases reference that was obtained when lock was linked into a
queue by
nlmsvc_insert_block (which is indicated by B_QUEUED flag in the
nlmblock)
And then we need to release another reference that we got from block
lookup.
This path you pointed out is definitely leaking, it is a success path
for
async GETFL handling so I tested this case.
Bye,
Oleg
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html