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 [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to