Nouveau - Apr 2023 - [PATCH 2/9] drm/nouveau/debugfs: Move a variable assignment behind a null pointer check in nouveau_debugfs_pstate_get()

Date: Sun, 16 Apr 2023 11:22:23 +0200

Several update suggestions were taken into account
from static source code analysis.

Markus Elfring (9):
  debugfs: Move an expression into a function call parameter
    in nouveau_debugfs_pstate_set()
  debugfs: Move a variable assignment behind a null pointer check
    in nouveau_debugfs_pstate_get()
  debugfs: Use seq_putc()
    in nouveau_debugfs_pstate_get()
  debugfs: Replace five seq_printf() calls by seq_puts()
    in nouveau_debugfs_pstate_get()
  power_budget: Move an expression into a macro call parameter
    in nvbios_power_budget_header()
  clk: Move a variable assignment behind a null pointer check
    in nvkm_pstate_new()
  pci: Move a variable assignment behind condition checks
    in nvkm_pcie_set_link()
  pci: Move an expression into a function call parameter
    in nvkm_pcie_set_link()
  therm: Move an assignment statement behind a null pointer check
    in two functions

 drivers/gpu/drm/nouveau/nouveau_debugfs.c     | 19 ++++++++++---------
 .../nouveau/nvkm/subdev/bios/power_budget.c   |  3 +--
 .../gpu/drm/nouveau/nvkm/subdev/clk/base.c    |  2 +-
 .../gpu/drm/nouveau/nvkm/subdev/pci/pcie.c    |  7 +++----
 .../drm/nouveau/nvkm/subdev/therm/fanpwm.c    |  2 +-
 .../drm/nouveau/nvkm/subdev/therm/fantog.c    |  2 +-
 6 files changed, 17 insertions(+), 18 deletions(-)

--
2.40.0

Date: Sat, 15 Apr 2023 21:06:06 +0200

The address of a data structure member was determined before
a corresponding null pointer check in the implementation of
the function ?nouveau_debugfs_pstate_set?.

Thus avoid the risk for undefined behaviour by moving the usage
of an expression into a parameter for a function call at the end.

This issue was detected by using the Coccinelle software.

Fixes: 6e9fc177399f08446293fec7607913fdbc95e191 ("drm/nouveau/debugfs: add
copy of sysfs pstate interface ported to debugfs")
Signed-off-by: Markus Elfring <elfring at users.sourceforge.net>
---
 drivers/gpu/drm/nouveau/nouveau_debugfs.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_debugfs.c
b/drivers/gpu/drm/nouveau/nouveau_debugfs.c
index 2a36d1ca8fda..44e26b6e74c7 100644
--- a/drivers/gpu/drm/nouveau/nouveau_debugfs.c
+++ b/drivers/gpu/drm/nouveau/nouveau_debugfs.c
@@ -144,7 +144,6 @@ nouveau_debugfs_pstate_set(struct file *file, const char
__user *ubuf,
 	struct seq_file *m = file->private_data;
 	struct drm_device *drm = m->private;
 	struct nouveau_debugfs *debugfs = nouveau_debugfs(drm);
-	struct nvif_object *ctrl = &debugfs->ctrl;
 	struct nvif_control_pstate_user_v0 args = { .pwrsrc = -EINVAL };
 	char buf[32] = {}, *tmp, *cur = buf;
 	long value, ret;
@@ -188,7 +187,8 @@ nouveau_debugfs_pstate_set(struct file *file, const char
__user *ubuf,
 		return ret;
 	}

-	ret = nvif_mthd(ctrl, NVIF_CONTROL_PSTATE_USER, &args, sizeof(args));
+	ret = nvif_mthd(&debugfs->ctrl, NVIF_CONTROL_PSTATE_USER,
+			&args, sizeof(args));
 	pm_runtime_put_autosuspend(drm->dev);
 	if (ret < 0)
 		return ret;
--
2.40.0

Date: Sat, 15 Apr 2023 21:24:43 +0200

The address of a data structure member was determined before
a corresponding null pointer check in the implementation of
the function ?nouveau_debugfs_pstate_get?.

Thus avoid the risk for undefined behaviour by moving the assignment
for the variable ?ctrl? behind the null pointer check.

This issue was detected by using the Coccinelle software.

Fixes: 6e9fc177399f08446293fec7607913fdbc95e191 ("drm/nouveau/debugfs: add
copy of sysfs pstate interface ported to debugfs")
Signed-off-by: Markus Elfring <elfring at users.sourceforge.net>
---
 drivers/gpu/drm/nouveau/nouveau_debugfs.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_debugfs.c
b/drivers/gpu/drm/nouveau/nouveau_debugfs.c
index 44e26b6e74c7..a859a086f308 100644
--- a/drivers/gpu/drm/nouveau/nouveau_debugfs.c
+++ b/drivers/gpu/drm/nouveau/nouveau_debugfs.c
@@ -73,13 +73,14 @@ nouveau_debugfs_pstate_get(struct seq_file *m, void *data)
 {
 	struct drm_device *drm = m->private;
 	struct nouveau_debugfs *debugfs = nouveau_debugfs(drm);
-	struct nvif_object *ctrl = &debugfs->ctrl;
+	struct nvif_object *ctrl;
 	struct nvif_control_pstate_info_v0 info = {};
 	int ret, i;

 	if (!debugfs)
 		return -ENODEV;

+	ctrl = &debugfs->ctrl;
 	ret = nvif_mthd(ctrl, NVIF_CONTROL_PSTATE_INFO, &info, sizeof(info));
 	if (ret)
 		return ret;
--
2.40.0

Date: Sat, 15 Apr 2023 21:48:47 +0200

A single character (line break) should be put into a sequence.
Thus use the corresponding function ?seq_putc?.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring at users.sourceforge.net>
---
 drivers/gpu/drm/nouveau/nouveau_debugfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_debugfs.c
b/drivers/gpu/drm/nouveau/nouveau_debugfs.c
index a859a086f308..13c82eea8828 100644
--- a/drivers/gpu/drm/nouveau/nouveau_debugfs.c
+++ b/drivers/gpu/drm/nouveau/nouveau_debugfs.c
@@ -132,7 +132,7 @@ nouveau_debugfs_pstate_get(struct seq_file *m, void *data)
 				seq_printf(m, " DC");
 		}

-		seq_printf(m, "\n");
+		seq_putc(m, '\n');
 	}

 	return 0;
--
2.40.0

Date: Sat, 15 Apr 2023 22:02:31 +0200

Five strings which did not contain a data format specification should
be put into a sequence. Thus use the corresponding function ?seq_puts?.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring at users.sourceforge.net>
---
 drivers/gpu/drm/nouveau/nouveau_debugfs.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_debugfs.c
b/drivers/gpu/drm/nouveau/nouveau_debugfs.c
index 13c82eea8828..99d022a91afc 100644
--- a/drivers/gpu/drm/nouveau/nouveau_debugfs.c
+++ b/drivers/gpu/drm/nouveau/nouveau_debugfs.c
@@ -120,16 +120,16 @@ nouveau_debugfs_pstate_get(struct seq_file *m, void *data)

 		if (state >= 0) {
 			if (info.ustate_ac == state)
-				seq_printf(m, " AC");
+				seq_puts(m, " AC");
 			if (info.ustate_dc == state)
-				seq_printf(m, " DC");
+				seq_puts(m, " DC");
 			if (info.pstate == state)
-				seq_printf(m, " *");
+				seq_puts(m, " *");
 		} else {
 			if (info.ustate_ac < -1)
-				seq_printf(m, " AC");
+				seq_puts(m, " AC");
 			if (info.ustate_dc < -1)
-				seq_printf(m, " DC");
+				seq_puts(m, " DC");
 		}

 		seq_putc(m, '\n');
--
2.40.0

Date: Sat, 15 Apr 2023 22:30:30 +0200

The address of a data structure member was determined before
a corresponding null pointer check in the implementation of
the function ?nvbios_power_budget_header?.

Thus avoid the risk for undefined behaviour by moving the usage
of an expression into a parameter for a macro call in one if branch.

This issue was detected by using the Coccinelle software.

Fixes: e5f8eabc0077ea3f77b3362e28d3969ae62e70f0
("drm/nouveau/bios/power_budget: Add basic power budget parsing")
Signed-off-by: Markus Elfring <elfring at users.sourceforge.net>
---
 drivers/gpu/drm/nouveau/nvkm/subdev/bios/power_budget.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/bios/power_budget.c
b/drivers/gpu/drm/nouveau/nvkm/subdev/bios/power_budget.c
index 03d2f970a29f..2ba992bdb19d 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/bios/power_budget.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/bios/power_budget.c
@@ -59,7 +59,6 @@ int
 nvbios_power_budget_header(struct nvkm_bios *bios,
                            struct nvbios_power_budget *budget)
 {
-	struct nvkm_subdev *subdev = &bios->subdev;
 	u8 ver, hdr, cnt, len, cap_entry;
 	u32 header;

@@ -82,7 +81,7 @@ nvbios_power_budget_header(struct nvkm_bios *bios,
 	}

 	if (cap_entry >= cnt && cap_entry != 0xff) {
-		nvkm_warn(subdev,
+		nvkm_warn(&bios->subdev,
 		          "invalid cap_entry in power budget table found\n");
 		budget->cap_entry = 0xff;
 		return -EINVAL;
--
2.40.0

Date: Sun, 16 Apr 2023 07:45:54 +0200

The address of a data structure member was determined before
a corresponding null pointer check in the implementation of
the function ?nvkm_pstate_new?.

Thus avoid the risk for undefined behaviour by moving the assignment
for the variable ?cstate? behind the null pointer check.

This issue was detected by using the Coccinelle software.

Fixes: 7c856522069755ab9d163a24ac332cd3cb35fe30 ("drm/nouveau/clk:
implement power state and engine clock control in core")
Signed-off-by: Markus Elfring <elfring at users.sourceforge.net>
---
 drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
b/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
index da07a2fbef06..178dc56909c2 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
@@ -417,7 +417,6 @@ nvkm_pstate_new(struct nvkm_clk *clk, int idx)
 		return 0;

 	pstate = kzalloc(sizeof(*pstate), GFP_KERNEL);
-	cstate = &pstate->base;
 	if (!pstate)
 		return -ENOMEM;

@@ -427,6 +426,7 @@ nvkm_pstate_new(struct nvkm_clk *clk, int idx)
 	pstate->fanspeed = perfE.fanspeed;
 	pstate->pcie_speed = perfE.pcie_speed;
 	pstate->pcie_width = perfE.pcie_width;
+	cstate = &pstate->base;
 	cstate->voltage = perfE.voltage;
 	cstate->domain[nv_clk_src_core] = perfE.core;
 	cstate->domain[nv_clk_src_shader] = perfE.shader;
--
2.40.0

Date: Sun, 16 Apr 2023 08:18:40 +0200

The address of a data structure member was determined before
a corresponding null pointer check in the implementation of
the function ?nvkm_pcie_set_link?.

Thus avoid the risk for undefined behaviour by moving the assignment
for the variable ?subdev? behind some condition checks.

This issue was detected by using the Coccinelle software.

Fixes: bcc19d9bf5cd8d49428c487adced1aa101271b18 ("drm/nouveau/pci:
implement generic code for pcie speed change")
Signed-off-by: Markus Elfring <elfring at users.sourceforge.net>
---
 drivers/gpu/drm/nouveau/nvkm/subdev/pci/pcie.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/pci/pcie.c
b/drivers/gpu/drm/nouveau/nvkm/subdev/pci/pcie.c
index d71e5db5028a..b295f100e1c1 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/pci/pcie.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/pci/pcie.c
@@ -114,7 +114,7 @@ nvkm_pcie_init(struct nvkm_pci *pci)
 int
 nvkm_pcie_set_link(struct nvkm_pci *pci, enum nvkm_pcie_speed speed, u8 width)
 {
-	struct nvkm_subdev *subdev = &pci->subdev;
+	struct nvkm_subdev *subdev;
 	enum nvkm_pcie_speed cur_speed, max_speed;
 	struct pci_bus *pbus;
 	int ret;
@@ -126,6 +126,7 @@ nvkm_pcie_set_link(struct nvkm_pci *pci, enum
nvkm_pcie_speed speed, u8 width)
 	if (!pci->func->pcie.set_link)
 		return -ENOSYS;

+	subdev = &pci->subdev;
 	nvkm_trace(subdev, "requested %s\n", nvkm_pcie_speeds[speed]);

 	if (pci->func->pcie.version(pci) < 2) {
--
2.40.0

Date: Sun, 16 Apr 2023 08:45:31 +0200

The variable ?pbus? was read only once in the implementation of
the function ?nvkm_pcie_set_link?.
Thus move the usage of an expression into a parameter for a function call.

Signed-off-by: Markus Elfring <elfring at users.sourceforge.net>
---
 drivers/gpu/drm/nouveau/nvkm/subdev/pci/pcie.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/pci/pcie.c
b/drivers/gpu/drm/nouveau/nvkm/subdev/pci/pcie.c
index b295f100e1c1..dd18d9d0bade 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/pci/pcie.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/pci/pcie.c
@@ -116,12 +116,10 @@ nvkm_pcie_set_link(struct nvkm_pci *pci, enum
nvkm_pcie_speed speed, u8 width)
 {
 	struct nvkm_subdev *subdev;
 	enum nvkm_pcie_speed cur_speed, max_speed;
-	struct pci_bus *pbus;
 	int ret;

 	if (!pci || !pci_is_pcie(pci->pdev))
 		return 0;
-	pbus = pci->pdev->bus;

 	if (!pci->func->pcie.set_link)
 		return -ENOSYS;
@@ -135,7 +133,7 @@ nvkm_pcie_set_link(struct nvkm_pci *pci, enum
nvkm_pcie_speed speed, u8 width)
 	}

 	cur_speed = pci->func->pcie.cur_speed(pci);
-	max_speed = min(nvkm_pcie_speed(pbus->max_bus_speed),
+	max_speed = min(nvkm_pcie_speed(pci->pdev->bus->max_bus_speed),
 			pci->func->pcie.max_speed(pci));

 	nvkm_trace(subdev, "current speed: %s\n",
nvkm_pcie_speeds[cur_speed]);
--
2.40.0

Date: Sun, 16 Apr 2023 10:50:12 +0200

The address of a data structure member was determined before
a corresponding null pointer check in the implementation of
the functions ?nvkm_fanpwm_create? and ?nvkm_fantog_create?.

Thus avoid the risk for undefined behaviour by moving the assignment
for the data structure member ?fan? behind two null pointer checks.

This issue was detected by using the Coccinelle software.

Fixes: da06b46b720687117178d3ee85a601762f1c36b5 ("drm/nouveau/therm:
cosmetic changes")
Signed-off-by: Markus Elfring <elfring at users.sourceforge.net>
---
 drivers/gpu/drm/nouveau/nvkm/subdev/therm/fanpwm.c | 2 +-
 drivers/gpu/drm/nouveau/nvkm/subdev/therm/fantog.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/therm/fanpwm.c
b/drivers/gpu/drm/nouveau/nvkm/subdev/therm/fanpwm.c
index 340f37a299dc..b13ba9b2f6be 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/therm/fanpwm.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/therm/fanpwm.c
@@ -98,10 +98,10 @@ nvkm_fanpwm_create(struct nvkm_therm *therm, struct
dcb_gpio_func *func)
 		return -ENODEV;

 	fan = kzalloc(sizeof(*fan), GFP_KERNEL);
-	therm->fan = &fan->base;
 	if (!fan)
 		return -ENOMEM;

+	therm->fan = &fan->base;
 	fan->base.type = "PWM";
 	fan->base.get = nvkm_fanpwm_get;
 	fan->base.set = nvkm_fanpwm_set;
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/therm/fantog.c
b/drivers/gpu/drm/nouveau/nvkm/subdev/therm/fantog.c
index ff9fbe7950e5..bfdf4ca5625c 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/therm/fantog.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/therm/fantog.c
@@ -100,10 +100,10 @@ nvkm_fantog_create(struct nvkm_therm *therm, struct
dcb_gpio_func *func)
 	}

 	fan = kzalloc(sizeof(*fan), GFP_KERNEL);
-	therm->fan = &fan->base;
 	if (!fan)
 		return -ENOMEM;

+	therm->fan = &fan->base;
 	fan->base.type = "toggle";
 	fan->base.get = nvkm_fantog_get;
 	fan->base.set = nvkm_fantog_set;
--
2.40.0

On Sun, Apr 16, 2023 at 11:30?AM Markus Elfring <Markus.Elfring at web.de>
wrote:
>
> Date: Sun, 16 Apr 2023 11:22:23 +0200
>
> Several update suggestions were taken into account
> from static source code analysis.
>
Reviewed-by: Karol Herbst <kherbst at redhat.com>
> Markus Elfring (9):
>   debugfs: Move an expression into a function call parameter
>     in nouveau_debugfs_pstate_set()
>   debugfs: Move a variable assignment behind a null pointer check
>     in nouveau_debugfs_pstate_get()
>   debugfs: Use seq_putc()
>     in nouveau_debugfs_pstate_get()
>   debugfs: Replace five seq_printf() calls by seq_puts()
>     in nouveau_debugfs_pstate_get()
>   power_budget: Move an expression into a macro call parameter
>     in nvbios_power_budget_header()
>   clk: Move a variable assignment behind a null pointer check
>     in nvkm_pstate_new()
>   pci: Move a variable assignment behind condition checks
>     in nvkm_pcie_set_link()
>   pci: Move an expression into a function call parameter
>     in nvkm_pcie_set_link()
>   therm: Move an assignment statement behind a null pointer check
>     in two functions
>
>  drivers/gpu/drm/nouveau/nouveau_debugfs.c     | 19 ++++++++++---------
>  .../nouveau/nvkm/subdev/bios/power_budget.c   |  3 +--
>  .../gpu/drm/nouveau/nvkm/subdev/clk/base.c    |  2 +-
>  .../gpu/drm/nouveau/nvkm/subdev/pci/pcie.c    |  7 +++----
>  .../drm/nouveau/nvkm/subdev/therm/fanpwm.c    |  2 +-
>  .../drm/nouveau/nvkm/subdev/therm/fantog.c    |  2 +-
>  6 files changed, 17 insertions(+), 18 deletions(-)
>
> --
> 2.40.0
>