Re: [PATCH for-next 06/16] IB/hfi1: hfi1_open_file() missing kobject_put in err path

Jason Gunthorpe
2018-01-26T15:51:52Z

On Fri, Jan 26, 2018 at 06:32:06AM -0800, Dennis Dalessandro wrote:
> From: Alex Estrin <alex.estrin-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
>
> The dd refcount is speculatively incremented prior to allocating
> the fd memory with kzalloc(). If that kzalloc() failed the dd
> refcount leaks.
>
> Fixes: e11ffbd57520 ("IB/hfi1: Do not free hfi1 cdev parent structure early")
> Reviewed-by: Michael J Ruhl <michael.j.ruhl-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Alex Estrin <alex.estrin-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Dennis Dalessandro <dennis.dalessandro-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> drivers/infiniband/hw/hfi1/file_ops.c | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/infiniband/hw/hfi1/file_ops.c b/drivers/infiniband/hw/hfi1/file_ops.c
> index 7750a9c..d7bdf3d 100644
> +++ b/drivers/infiniband/hw/hfi1/file_ops.c
> @@ -210,6 +210,7 @@ static int hfi1_file_open(struct inode *inode, struct file *fp)
> fd->dd = dd;
> fp->private_data = fd;
> } else {
> + kobject_put(&dd->kobj);
> fp->private_data = NULL;
>
> if (atomic_dec_and_test(&dd->user_refcount))

Ugh that reads horribly.

Put the kobject_get when you copy the pointer:

fd->dd = dd;
kobject_get(&fd->dd->kobj);

Then it actually makes sense what is happening.

Jsaon