* [PATCH] clk: base: fix an incorrect NULL check on list iterator
@ 2022-03-27 7:58 ` Xiaomeng Tong
0 siblings, 0 replies; 6+ messages in thread
From: Xiaomeng Tong @ 2022-03-27 7:58 UTC (permalink / raw)
To: bskeggs, kherbst, lyude, airlied, daniel
Cc: martin.peres, dri-devel, nouveau, linux-kernel, Xiaomeng Tong, stable
The bug is here:
if (nvkm_cstate_valid(clk, cstate, max_volt, clk->temp))
return cstate;
The list iterator value 'cstate' will *always* be set and non-NULL
by list_for_each_entry_from_reverse(), so it is incorrect to assume
that the iterator value will be unchanged if the list is empty or no
element is found (In fact, it will be a bogus pointer to an invalid
structure object containing the HEAD). Also it missed a NULL check
at callsite and may lead to invalid memory access after that.
To fix this bug, just return 'encoder' when found, otherwise return
NULL. And add the NULL check.
Cc: stable@vger.kernel.org
Fixes: 1f7f3d91ad38a ("drm/nouveau/clk: Respect voltage limits in nvkm_cstate_prog")
Signed-off-by: Xiaomeng Tong <xiam0nd.tong@gmail.com>
---
drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c b/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
index 57199be082fd..c2b5cc5f97ed 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
@@ -135,10 +135,10 @@ nvkm_cstate_find_best(struct nvkm_clk *clk, struct nvkm_pstate *pstate,
list_for_each_entry_from_reverse(cstate, &pstate->list, head) {
if (nvkm_cstate_valid(clk, cstate, max_volt, clk->temp))
- break;
+ return cstate;
}
- return cstate;
+ return NULL;
}
static struct nvkm_cstate *
@@ -169,6 +169,8 @@ nvkm_cstate_prog(struct nvkm_clk *clk, struct nvkm_pstate *pstate, int cstatei)
if (!list_empty(&pstate->list)) {
cstate = nvkm_cstate_get(clk, pstate, cstatei);
cstate = nvkm_cstate_find_best(clk, pstate, cstate);
+ if (!cstate)
+ return -EINVAL;
} else {
cstate = &pstate->base;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH] clk: base: fix an incorrect NULL check on list iterator
@ 2022-03-27 7:58 ` Xiaomeng Tong
0 siblings, 0 replies; 6+ messages in thread
From: Xiaomeng Tong @ 2022-03-27 7:58 UTC (permalink / raw)
To: bskeggs, kherbst, lyude, airlied, daniel
Cc: nouveau, linux-kernel, stable, Xiaomeng Tong, dri-devel
The bug is here:
if (nvkm_cstate_valid(clk, cstate, max_volt, clk->temp))
return cstate;
The list iterator value 'cstate' will *always* be set and non-NULL
by list_for_each_entry_from_reverse(), so it is incorrect to assume
that the iterator value will be unchanged if the list is empty or no
element is found (In fact, it will be a bogus pointer to an invalid
structure object containing the HEAD). Also it missed a NULL check
at callsite and may lead to invalid memory access after that.
To fix this bug, just return 'encoder' when found, otherwise return
NULL. And add the NULL check.
Cc: stable@vger.kernel.org
Fixes: 1f7f3d91ad38a ("drm/nouveau/clk: Respect voltage limits in nvkm_cstate_prog")
Signed-off-by: Xiaomeng Tong <xiam0nd.tong@gmail.com>
---
drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c b/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
index 57199be082fd..c2b5cc5f97ed 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
@@ -135,10 +135,10 @@ nvkm_cstate_find_best(struct nvkm_clk *clk, struct nvkm_pstate *pstate,
list_for_each_entry_from_reverse(cstate, &pstate->list, head) {
if (nvkm_cstate_valid(clk, cstate, max_volt, clk->temp))
- break;
+ return cstate;
}
- return cstate;
+ return NULL;
}
static struct nvkm_cstate *
@@ -169,6 +169,8 @@ nvkm_cstate_prog(struct nvkm_clk *clk, struct nvkm_pstate *pstate, int cstatei)
if (!list_empty(&pstate->list)) {
cstate = nvkm_cstate_get(clk, pstate, cstatei);
cstate = nvkm_cstate_find_best(clk, pstate, cstate);
+ if (!cstate)
+ return -EINVAL;
} else {
cstate = &pstate->base;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [Nouveau] [PATCH] clk: base: fix an incorrect NULL check on list iterator
@ 2022-03-27 7:58 ` Xiaomeng Tong
0 siblings, 0 replies; 6+ messages in thread
From: Xiaomeng Tong @ 2022-03-27 7:58 UTC (permalink / raw)
To: bskeggs, kherbst, lyude, airlied, daniel
Cc: nouveau, linux-kernel, stable, Xiaomeng Tong, dri-devel
The bug is here:
if (nvkm_cstate_valid(clk, cstate, max_volt, clk->temp))
return cstate;
The list iterator value 'cstate' will *always* be set and non-NULL
by list_for_each_entry_from_reverse(), so it is incorrect to assume
that the iterator value will be unchanged if the list is empty or no
element is found (In fact, it will be a bogus pointer to an invalid
structure object containing the HEAD). Also it missed a NULL check
at callsite and may lead to invalid memory access after that.
To fix this bug, just return 'encoder' when found, otherwise return
NULL. And add the NULL check.
Cc: stable@vger.kernel.org
Fixes: 1f7f3d91ad38a ("drm/nouveau/clk: Respect voltage limits in nvkm_cstate_prog")
Signed-off-by: Xiaomeng Tong <xiam0nd.tong@gmail.com>
---
drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c b/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
index 57199be082fd..c2b5cc5f97ed 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
@@ -135,10 +135,10 @@ nvkm_cstate_find_best(struct nvkm_clk *clk, struct nvkm_pstate *pstate,
list_for_each_entry_from_reverse(cstate, &pstate->list, head) {
if (nvkm_cstate_valid(clk, cstate, max_volt, clk->temp))
- break;
+ return cstate;
}
- return cstate;
+ return NULL;
}
static struct nvkm_cstate *
@@ -169,6 +169,8 @@ nvkm_cstate_prog(struct nvkm_clk *clk, struct nvkm_pstate *pstate, int cstatei)
if (!list_empty(&pstate->list)) {
cstate = nvkm_cstate_get(clk, pstate, cstatei);
cstate = nvkm_cstate_find_best(clk, pstate, cstate);
+ if (!cstate)
+ return -EINVAL;
} else {
cstate = &pstate->base;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Nouveau] [PATCH] clk: base: fix an incorrect NULL check on list iterator
2022-03-27 7:58 ` Xiaomeng Tong
(?)
@ 2022-04-04 21:23 ` Lyude Paul
-1 siblings, 0 replies; 6+ messages in thread
From: Lyude Paul @ 2022-04-04 21:23 UTC (permalink / raw)
To: Xiaomeng Tong, bskeggs, kherbst, airlied, daniel
Cc: nouveau, stable, linux-kernel, dri-devel
This should probably be prefixed with the title "drm/nouveau/clk:", but I can
fix that before pushing it.
Reviewed-by: Lyude Paul <lyude@redhat.com>
Will push it to the appropriate repository shortly
On Sun, 2022-03-27 at 15:58 +0800, Xiaomeng Tong wrote:
> The bug is here:
> if (nvkm_cstate_valid(clk, cstate, max_volt, clk->temp))
> return cstate;
>
> The list iterator value 'cstate' will *always* be set and non-NULL
> by list_for_each_entry_from_reverse(), so it is incorrect to assume
> that the iterator value will be unchanged if the list is empty or no
> element is found (In fact, it will be a bogus pointer to an invalid
> structure object containing the HEAD). Also it missed a NULL check
> at callsite and may lead to invalid memory access after that.
>
> To fix this bug, just return 'encoder' when found, otherwise return
> NULL. And add the NULL check.
>
> Cc: stable@vger.kernel.org
> Fixes: 1f7f3d91ad38a ("drm/nouveau/clk: Respect voltage limits in
> nvkm_cstate_prog")
> Signed-off-by: Xiaomeng Tong <xiam0nd.tong@gmail.com>
> ---
> drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
> b/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
> index 57199be082fd..c2b5cc5f97ed 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
> @@ -135,10 +135,10 @@ nvkm_cstate_find_best(struct nvkm_clk *clk, struct
> nvkm_pstate *pstate,
>
> list_for_each_entry_from_reverse(cstate, &pstate->list, head) {
> if (nvkm_cstate_valid(clk, cstate, max_volt, clk->temp))
> - break;
> + return cstate;
> }
>
> - return cstate;
> + return NULL;
> }
>
> static struct nvkm_cstate *
> @@ -169,6 +169,8 @@ nvkm_cstate_prog(struct nvkm_clk *clk, struct
> nvkm_pstate *pstate, int cstatei)
> if (!list_empty(&pstate->list)) {
> cstate = nvkm_cstate_get(clk, pstate, cstatei);
> cstate = nvkm_cstate_find_best(clk, pstate, cstate);
> + if (!cstate)
> + return -EINVAL;
> } else {
> cstate = &pstate->base;
> }
--
Cheers,
Lyude Paul (she/her)
Software Engineer at Red Hat
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] clk: base: fix an incorrect NULL check on list iterator
@ 2022-04-04 21:23 ` Lyude Paul
0 siblings, 0 replies; 6+ messages in thread
From: Lyude Paul @ 2022-04-04 21:23 UTC (permalink / raw)
To: Xiaomeng Tong, bskeggs, kherbst, airlied, daniel
Cc: nouveau, stable, linux-kernel, dri-devel
This should probably be prefixed with the title "drm/nouveau/clk:", but I can
fix that before pushing it.
Reviewed-by: Lyude Paul <lyude@redhat.com>
Will push it to the appropriate repository shortly
On Sun, 2022-03-27 at 15:58 +0800, Xiaomeng Tong wrote:
> The bug is here:
> if (nvkm_cstate_valid(clk, cstate, max_volt, clk->temp))
> return cstate;
>
> The list iterator value 'cstate' will *always* be set and non-NULL
> by list_for_each_entry_from_reverse(), so it is incorrect to assume
> that the iterator value will be unchanged if the list is empty or no
> element is found (In fact, it will be a bogus pointer to an invalid
> structure object containing the HEAD). Also it missed a NULL check
> at callsite and may lead to invalid memory access after that.
>
> To fix this bug, just return 'encoder' when found, otherwise return
> NULL. And add the NULL check.
>
> Cc: stable@vger.kernel.org
> Fixes: 1f7f3d91ad38a ("drm/nouveau/clk: Respect voltage limits in
> nvkm_cstate_prog")
> Signed-off-by: Xiaomeng Tong <xiam0nd.tong@gmail.com>
> ---
> drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
> b/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
> index 57199be082fd..c2b5cc5f97ed 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
> @@ -135,10 +135,10 @@ nvkm_cstate_find_best(struct nvkm_clk *clk, struct
> nvkm_pstate *pstate,
>
> list_for_each_entry_from_reverse(cstate, &pstate->list, head) {
> if (nvkm_cstate_valid(clk, cstate, max_volt, clk->temp))
> - break;
> + return cstate;
> }
>
> - return cstate;
> + return NULL;
> }
>
> static struct nvkm_cstate *
> @@ -169,6 +169,8 @@ nvkm_cstate_prog(struct nvkm_clk *clk, struct
> nvkm_pstate *pstate, int cstatei)
> if (!list_empty(&pstate->list)) {
> cstate = nvkm_cstate_get(clk, pstate, cstatei);
> cstate = nvkm_cstate_find_best(clk, pstate, cstate);
> + if (!cstate)
> + return -EINVAL;
> } else {
> cstate = &pstate->base;
> }
--
Cheers,
Lyude Paul (she/her)
Software Engineer at Red Hat
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] clk: base: fix an incorrect NULL check on list iterator
@ 2022-04-04 21:23 ` Lyude Paul
0 siblings, 0 replies; 6+ messages in thread
From: Lyude Paul @ 2022-04-04 21:23 UTC (permalink / raw)
To: Xiaomeng Tong, bskeggs, kherbst, airlied, daniel
Cc: martin.peres, dri-devel, nouveau, linux-kernel, stable
This should probably be prefixed with the title "drm/nouveau/clk:", but I can
fix that before pushing it.
Reviewed-by: Lyude Paul <lyude@redhat.com>
Will push it to the appropriate repository shortly
On Sun, 2022-03-27 at 15:58 +0800, Xiaomeng Tong wrote:
> The bug is here:
> if (nvkm_cstate_valid(clk, cstate, max_volt, clk->temp))
> return cstate;
>
> The list iterator value 'cstate' will *always* be set and non-NULL
> by list_for_each_entry_from_reverse(), so it is incorrect to assume
> that the iterator value will be unchanged if the list is empty or no
> element is found (In fact, it will be a bogus pointer to an invalid
> structure object containing the HEAD). Also it missed a NULL check
> at callsite and may lead to invalid memory access after that.
>
> To fix this bug, just return 'encoder' when found, otherwise return
> NULL. And add the NULL check.
>
> Cc: stable@vger.kernel.org
> Fixes: 1f7f3d91ad38a ("drm/nouveau/clk: Respect voltage limits in
> nvkm_cstate_prog")
> Signed-off-by: Xiaomeng Tong <xiam0nd.tong@gmail.com>
> ---
> drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
> b/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
> index 57199be082fd..c2b5cc5f97ed 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
> @@ -135,10 +135,10 @@ nvkm_cstate_find_best(struct nvkm_clk *clk, struct
> nvkm_pstate *pstate,
>
> list_for_each_entry_from_reverse(cstate, &pstate->list, head) {
> if (nvkm_cstate_valid(clk, cstate, max_volt, clk->temp))
> - break;
> + return cstate;
> }
>
> - return cstate;
> + return NULL;
> }
>
> static struct nvkm_cstate *
> @@ -169,6 +169,8 @@ nvkm_cstate_prog(struct nvkm_clk *clk, struct
> nvkm_pstate *pstate, int cstatei)
> if (!list_empty(&pstate->list)) {
> cstate = nvkm_cstate_get(clk, pstate, cstatei);
> cstate = nvkm_cstate_find_best(clk, pstate, cstate);
> + if (!cstate)
> + return -EINVAL;
> } else {
> cstate = &pstate->base;
> }
--
Cheers,
Lyude Paul (she/her)
Software Engineer at Red Hat
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-04-04 22:12 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-27 7:58 [PATCH] clk: base: fix an incorrect NULL check on list iterator Xiaomeng Tong
2022-03-27 7:58 ` [Nouveau] " Xiaomeng Tong
2022-03-27 7:58 ` Xiaomeng Tong
2022-04-04 21:23 ` [Nouveau] " Lyude Paul
2022-04-04 21:23 ` Lyude Paul
2022-04-04 21:23 ` Lyude Paul
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.