[PATCH 0/2] drm/nouveau: avoid usage of list iterator after loop

Jakob Koschel posted 2 patches 2 years, 6 months ago
There is a newer version of this series
drivers/gpu/drm/nouveau/nvkm/engine/device/ctrl.c | 9 ++++++---
drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c    | 9 ++++++---
2 files changed, 12 insertions(+), 6 deletions(-)
[PATCH 0/2] drm/nouveau: avoid usage of list iterator after loop
Posted by Jakob Koschel 2 years, 6 months ago
This patch set includes two instances where the list iterator variable
'pstate' is implicitly assumed to be valid after the iterator loop.
While in pratice that is most likely the case (if
'pstatei'/'args->v0.state' is <= the elements in clk->states), we should
explicitly only allow 'pstate' to always point to correct 'nvkm_pstate'
structs.

That allows catching potential bugs with BUG_ON(!pstate) that otherwise
would be completely undetectable.

It also helps the greater mission to hopefully move the list iterator
variable into the iterating macro directly [1].

Link: https://lore.kernel.org/all/CAHk-=wgRr_D8CB-D9Kg-c=EHreAsk5SqXPwr9Y7k9sA6cWXJ6w@mail.gmail.com/ [1]
Signed-off-by: Jakob Koschel <jkl820.git@gmail.com>
---
Jakob Koschel (2):
      drm/nouveau/device: avoid usage of list iterator after loop
      drm/nouveau/clk: avoid usage of list iterator after loop

 drivers/gpu/drm/nouveau/nvkm/engine/device/ctrl.c | 9 ++++++---
 drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c    | 9 ++++++---
 2 files changed, 12 insertions(+), 6 deletions(-)
---
base-commit: c0927a7a5391f7d8e593e5e50ead7505a23cadf9
change-id: 20230301-drm-nouveau-avoid-iter-after-loop-4bff97166efa

Best regards,
-- 
Jakob Koschel <jkl820.git@gmail.com>
Re: [PATCH 0/2] drm/nouveau: avoid usage of list iterator after loop
Posted by Lyude Paul 2 years, 6 months ago
Reviewed-by: Lyude Paul <lyude@redhat.com>

Will push upstream in just a moment

On Wed, 2023-03-01 at 18:25 +0100, Jakob Koschel wrote:
> This patch set includes two instances where the list iterator variable
> 'pstate' is implicitly assumed to be valid after the iterator loop.
> While in pratice that is most likely the case (if
> 'pstatei'/'args->v0.state' is <= the elements in clk->states), we should
> explicitly only allow 'pstate' to always point to correct 'nvkm_pstate'
> structs.
> 
> That allows catching potential bugs with BUG_ON(!pstate) that otherwise
> would be completely undetectable.
> 
> It also helps the greater mission to hopefully move the list iterator
> variable into the iterating macro directly [1].
> 
> Link: https://lore.kernel.org/all/CAHk-=wgRr_D8CB-D9Kg-c=EHreAsk5SqXPwr9Y7k9sA6cWXJ6w@mail.gmail.com/ [1]
> Signed-off-by: Jakob Koschel <jkl820.git@gmail.com>
> ---
> Jakob Koschel (2):
>       drm/nouveau/device: avoid usage of list iterator after loop
>       drm/nouveau/clk: avoid usage of list iterator after loop
> 
>  drivers/gpu/drm/nouveau/nvkm/engine/device/ctrl.c | 9 ++++++---
>  drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c    | 9 ++++++---
>  2 files changed, 12 insertions(+), 6 deletions(-)
> ---
> base-commit: c0927a7a5391f7d8e593e5e50ead7505a23cadf9
> change-id: 20230301-drm-nouveau-avoid-iter-after-loop-4bff97166efa
> 
> Best regards,

-- 
Cheers,
 Lyude Paul (she/her)
 Software Engineer at Red Hat
Re: [PATCH 0/2] drm/nouveau: avoid usage of list iterator after loop
Posted by Lyude Paul 2 years, 6 months ago
Actually hm, dim is warning me about this and making me realize there's
probably a better way to handle this, going to revoke the previous r-b I gave
and respond inline

On Tue, 2023-03-07 at 17:43 -0500, Lyude Paul wrote:
> Reviewed-by: Lyude Paul <lyude@redhat.com>
> 
> Will push upstream in just a moment
> 
> On Wed, 2023-03-01 at 18:25 +0100, Jakob Koschel wrote:
> > This patch set includes two instances where the list iterator variable
> > 'pstate' is implicitly assumed to be valid after the iterator loop.
> > While in pratice that is most likely the case (if
> > 'pstatei'/'args->v0.state' is <= the elements in clk->states), we should
> > explicitly only allow 'pstate' to always point to correct 'nvkm_pstate'
> > structs.
> > 
> > That allows catching potential bugs with BUG_ON(!pstate) that otherwise
> > would be completely undetectable.
> > 
> > It also helps the greater mission to hopefully move the list iterator
> > variable into the iterating macro directly [1].
> > 
> > Link: https://lore.kernel.org/all/CAHk-=wgRr_D8CB-D9Kg-c=EHreAsk5SqXPwr9Y7k9sA6cWXJ6w@mail.gmail.com/ [1]
> > Signed-off-by: Jakob Koschel <jkl820.git@gmail.com>
> > ---
> > Jakob Koschel (2):
> >       drm/nouveau/device: avoid usage of list iterator after loop
> >       drm/nouveau/clk: avoid usage of list iterator after loop
> > 
> >  drivers/gpu/drm/nouveau/nvkm/engine/device/ctrl.c | 9 ++++++---
> >  drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c    | 9 ++++++---
> >  2 files changed, 12 insertions(+), 6 deletions(-)
> > ---
> > base-commit: c0927a7a5391f7d8e593e5e50ead7505a23cadf9
> > change-id: 20230301-drm-nouveau-avoid-iter-after-loop-4bff97166efa
> > 
> > Best regards,
> 

-- 
Cheers,
 Lyude Paul (she/her)
 Software Engineer at Red Hat