diff mbox series

[ndctl] libcxl: fix potential NULL dereference in cxl_memdev_nvdimm_bridge_active()

Message ID 20211218022511.314928-1-vishal.l.verma@intel.com (mailing list archive)
State Accepted
Commit c55b18181281b2fffadb9e0e8955d74b8b719349
Headers show
Series [ndctl] libcxl: fix potential NULL dereference in cxl_memdev_nvdimm_bridge_active() | expand

Commit Message

Verma, Vishal L Dec. 18, 2021, 2:25 a.m. UTC 
Static analysis points out that the function above has a check for
'if (!bridge)', implying that bridge maybe NULL, but it is dereferenced
before the check, which could result in a NULL dereference.

Fix this by moving any accesses to the bridge structure after the NULL
check.

Cc: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
---
 cxl/lib/libcxl.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)


base-commit: 8f4e42c0c526e85b045fd0329df7cb904f511c98
prerequisite-patch-id: acc28fefb6680c477864561902d1560c300fa4a9
prerequisite-patch-id: c749c331eb4a521c8f0b0820e3dda7ac521926d0
prerequisite-patch-id: 9fc7ac4fe29689b9c4de00924a9e455cd9b58d53
prerequisite-patch-id: e019f2c873ac1b93d80b9c260336e5d3f46b0925
prerequisite-patch-id: b69ecc9d68ce5e7c79b62cea257663519f630da2
prerequisite-patch-id: 928a32f4c1ff844df809ccf1aba8315cac723d93
prerequisite-patch-id: ae29f23cedf529da1fe8e39f8cde1df827d75fa1
prerequisite-patch-id: f3d8fc575f5afed65be8a7b486962e8384eabc1a
prerequisite-patch-id: 74859f43302dcc442dfb1e29f6babad75d229bf1
prerequisite-patch-id: 696c2d5caf0bd4eac645035f72a7077efbf3e6cd
prerequisite-patch-id: e3ef7893a1df9ecc6b76dee78f2b27cc933ad891

Comments

Dan Williams Dec. 18, 2021, 2:28 a.m. UTC | #1 
On Fri, Dec 17, 2021 at 6:25 PM Vishal Verma <vishal.l.verma@intel.com> wrote:
>
> Static analysis points out that the function above has a check for
> 'if (!bridge)', implying that bridge maybe NULL, but it is dereferenced
> before the check, which could result in a NULL dereference.
>
> Fix this by moving any accesses to the bridge structure after the NULL
> check.
>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>

LGTM

Reviewed-by: Dan Williams <dan.j.williams@intel.com>

> ---
>  cxl/lib/libcxl.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
> index f0664be..3390eb9 100644
> --- a/cxl/lib/libcxl.c
> +++ b/cxl/lib/libcxl.c
> @@ -420,12 +420,15 @@ CXL_EXPORT int cxl_memdev_nvdimm_bridge_active(struct cxl_memdev *memdev)
>  {
>         struct cxl_ctx *ctx = cxl_memdev_get_ctx(memdev);
>         struct cxl_nvdimm_bridge *bridge = memdev->bridge;
> -       char *path = bridge->dev_buf;
> -       int len = bridge->buf_len;
> +       char *path;
> +       int len;
>
>         if (!bridge)
>                 return 0;
>
> +       path = bridge->dev_buf;
> +       len = bridge->buf_len;
> +
>         if (snprintf(path, len, "%s/driver", bridge->dev_path) >= len) {
>                 err(ctx, "%s: nvdimm bridge buffer too small!\n",
>                                 cxl_memdev_get_devname(memdev));
>
> base-commit: 8f4e42c0c526e85b045fd0329df7cb904f511c98
> prerequisite-patch-id: acc28fefb6680c477864561902d1560c300fa4a9
> prerequisite-patch-id: c749c331eb4a521c8f0b0820e3dda7ac521926d0
> prerequisite-patch-id: 9fc7ac4fe29689b9c4de00924a9e455cd9b58d53
> prerequisite-patch-id: e019f2c873ac1b93d80b9c260336e5d3f46b0925
> prerequisite-patch-id: b69ecc9d68ce5e7c79b62cea257663519f630da2
> prerequisite-patch-id: 928a32f4c1ff844df809ccf1aba8315cac723d93
> prerequisite-patch-id: ae29f23cedf529da1fe8e39f8cde1df827d75fa1
> prerequisite-patch-id: f3d8fc575f5afed65be8a7b486962e8384eabc1a
> prerequisite-patch-id: 74859f43302dcc442dfb1e29f6babad75d229bf1
> prerequisite-patch-id: 696c2d5caf0bd4eac645035f72a7077efbf3e6cd
> prerequisite-patch-id: e3ef7893a1df9ecc6b76dee78f2b27cc933ad891

git-send-email adds this by default now?
Verma, Vishal L Dec. 18, 2021, 2:33 a.m. UTC | #2 
On Fri, 2021-12-17 at 18:28 -0800, Dan Williams wrote:
> On Fri, Dec 17, 2021 at 6:25 PM Vishal Verma <vishal.l.verma@intel.com> wrote:
> > 
> > Static analysis points out that the function above has a check for
> > 'if (!bridge)', implying that bridge maybe NULL, but it is dereferenced
> > before the check, which could result in a NULL dereference.
> > 
> > Fix this by moving any accesses to the bridge structure after the NULL
> > check.
> > 
> > Cc: Dan Williams <dan.j.williams@intel.com>
> > Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
> 
> LGTM
> 
> Reviewed-by: Dan Williams <dan.j.williams@intel.com>

Thanks Dan

> 
> > ---
> > 
> > base-commit: 8f4e42c0c526e85b045fd0329df7cb904f511c98
> > prerequisite-patch-id: acc28fefb6680c477864561902d1560c300fa4a9
> > prerequisite-patch-id: c749c331eb4a521c8f0b0820e3dda7ac521926d0
> > prerequisite-patch-id: 9fc7ac4fe29689b9c4de00924a9e455cd9b58d53
> > prerequisite-patch-id: e019f2c873ac1b93d80b9c260336e5d3f46b0925
> > prerequisite-patch-id: b69ecc9d68ce5e7c79b62cea257663519f630da2
> > prerequisite-patch-id: 928a32f4c1ff844df809ccf1aba8315cac723d93
> > prerequisite-patch-id: ae29f23cedf529da1fe8e39f8cde1df827d75fa1
> > prerequisite-patch-id: f3d8fc575f5afed65be8a7b486962e8384eabc1a
> > prerequisite-patch-id: 74859f43302dcc442dfb1e29f6babad75d229bf1
> > prerequisite-patch-id: 696c2d5caf0bd4eac645035f72a7077efbf3e6cd
> > prerequisite-patch-id: e3ef7893a1df9ecc6b76dee78f2b27cc933ad891
> 
> git-send-email adds this by default now?

I think I know why git-format-patch added them.. I had --base turned on
via gitconfig. I didn't manually specify the base, so it tried to
deduce it using @upstream, but the upstream's remote wasn't fetched, so
it used the last known point in the local history.

I just did a git fetch --all, and format-patch -1 again, and now it
only generates the base-commit as you'd expect. Lesson - fetch your
remotes frequently :)  The whole thing was a side effect of using a
hack for multiple push-urls in a single remote.
diff mbox series

Patch

diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
index f0664be..3390eb9 100644
--- a/cxl/lib/libcxl.c
+++ b/cxl/lib/libcxl.c
@@ -420,12 +420,15 @@  CXL_EXPORT int cxl_memdev_nvdimm_bridge_active(struct cxl_memdev *memdev)
 {
 	struct cxl_ctx *ctx = cxl_memdev_get_ctx(memdev);
 	struct cxl_nvdimm_bridge *bridge = memdev->bridge;
-	char *path = bridge->dev_buf;
-	int len = bridge->buf_len;
+	char *path;
+	int len;
 
 	if (!bridge)
 		return 0;
 
+	path = bridge->dev_buf;
+	len = bridge->buf_len;
+
 	if (snprintf(path, len, "%s/driver", bridge->dev_path) >= len) {
 		err(ctx, "%s: nvdimm bridge buffer too small!\n",
 				cxl_memdev_get_devname(memdev));