Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
In-Reply-To: <20210426011647.3561-1-lyl2019@mail.ustc.edu.cn>
Subject: Re: [PATCH v2] rdma/siw: Fix a use after free in siw_alloc_mr
From:   "Bernard Metzler" <BMT@zurich.ibm.com>
To:     "Lv Yunlong" <lyl2019@mail.ustc.edu.cn>
Cc:     "dledford" <dledford@redhat.com>, "jgg" <jgg@ziepe.ca>,
        "linux-rdma" <linux-rdma@vger.kernel.org>,
        "linux-kernel" <linux-kernel@vger.kernel.org>, leon@kernel.org
Date:   Mon, 26 Apr 2021 07:26:45 +0000
MIME-Version: 1.0
Sensitivity: 
Importance: Normal
References: <20210426011647.3561-1-lyl2019@mail.ustc.edu.cn>
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset=UTF-8
Message-Id: <OFEDD7FB92.558D79F3-ON002586C3.0028E6C4-002586C3.0028E6CD@notes.na.collabserv.com>
Precedence: bulk

-----"Lv Yunlong" <lyl2019@mail.ustc.edu.cn> wrote: -----

>To: bmt@zurich.ibm.com, dledford@redhat.com, jgg@ziepe.ca
>From: "Lv Yunlong" <lyl2019@mail.ustc.edu.cn>
>Date: 04/26/2021 03:17AM
>Cc: linux-rdma@vger.kernel.org, linux-kernel@vger.kernel.org, "Lv
>Yunlong" <lyl2019@mail.ustc.edu.cn>
>Subject: [EXTERNAL] [PATCH v2] rdma/siw: Fix a use after free in
>siw=5Falloc=5Fmr
>
>Our code analyzer reported a uaf.
>
>In siw=5Falloc=5Fmr, it calls siw=5Fmr=5Fadd=5Fmem(mr,..). In the
>implementation
>of siw=5Fmr=5Fadd=5Fmem(), mem is assigned to mr->mem and then mem is freed
>via kfree(mem) if xa=5Falloc=5Fcyclic() failed. Here, mr->mem still point
>to a freed object. After, the execution continue up to the err=5Fout
>branch
>of siw=5Falloc=5Fmr, and the freed mr->mem is used in
>siw=5Fmr=5Fdrop=5Fmem(mr).
>
>My patch moves "mr->mem =3D mem" behind the if (xa=5Falloc=5Fcyclic(..)<0)
>{}
>section, to avoid the uaf.
>
>Fixes: 2251334dcac9e ("rdma/siw: application buffer management")
>Signed-off-by: Lv Yunlong <lyl2019@mail.ustc.edu.cn>
>---
> drivers/infiniband/sw/siw/siw=5Fmem.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/infiniband/sw/siw/siw=5Fmem.c
>b/drivers/infiniband/sw/siw/siw=5Fmem.c
>index 34a910cf0edb..96b38cfbb513 100644
>--- a/drivers/infiniband/sw/siw/siw=5Fmem.c
>+++ b/drivers/infiniband/sw/siw/siw=5Fmem.c
>@@ -106,8 +106,6 @@ int siw=5Fmr=5Fadd=5Fmem(struct siw=5Fmr *mr, struct
>ib=5Fpd *pd, void *mem=5Fobj,
> 	mem->perms =3D rights & IWARP=5FACCESS=5FMASK;
> 	kref=5Finit(&mem->ref);
>=20
>-	mr->mem =3D mem;
>-
> 	get=5Frandom=5Fbytes(&next, 4);
> 	next &=3D 0x00ffffff;
>=20
>@@ -116,6 +114,8 @@ int siw=5Fmr=5Fadd=5Fmem(struct siw=5Fmr *mr, struct
>ib=5Fpd *pd, void *mem=5Fobj,
> 		kfree(mem);
> 		return -ENOMEM;
> 	}
>+
>+	mr->mem =3D mem;
> 	/* Set the STag index part */
> 	mem->stag =3D id << 8;
> 	mr->base=5Fmr.lkey =3D mr->base=5Fmr.rkey =3D mem->stag;
>--=20
>2.25.1
>
>
>
Lv Yunlong, many thanks for catching, and thanks to
Leon for improving it.

Reviewed-by: Bernard Metzler <bmt@zurich.ihm.com>