Re: [Freedreno] [PATCH 0/9] drm: Annotate structs with __counted_by
On Thu, Oct 05, 2023 at 11:42:38AM +0200, Christian König wrote: > Am 02.10.23 um 20:22 schrieb Kees Cook: > > On Mon, Oct 02, 2023 at 08:11:41PM +0200, Christian König wrote: > > > Am 02.10.23 um 20:08 schrieb Kees Cook: > > > > On Mon, Oct 02, 2023 at 08:01:57PM +0200, Christian König wrote: > > > > > Am 02.10.23 um 18:53 schrieb Kees Cook: > > > > > > On Mon, Oct 02, 2023 at 11:06:19AM -0400, Alex Deucher wrote: > > > > > > > On Mon, Oct 2, 2023 at 5:20 AM Christian König > > > > > > > wrote: > > > > > > > > Am 29.09.23 um 21:33 schrieb Kees Cook: > > > > > > > > > On Fri, 22 Sep 2023 10:32:05 -0700, Kees Cook wrote: > > > > > > > > > > This is a batch of patches touching drm for preparing for > > > > > > > > > > the coming > > > > > > > > > > implementation by GCC and Clang of the __counted_by > > > > > > > > > > attribute. Flexible > > > > > > > > > > array members annotated with __counted_by can have their > > > > > > > > > > accesses > > > > > > > > > > bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS > > > > > > > > > > (for array > > > > > > > > > > indexing) and CONFIG_FORTIFY_SOURCE (for > > > > > > > > > > strcpy/memcpy-family functions). > > > > > > > > > > > > > > > > > > > > As found with Coccinelle[1], add __counted_by to structs > > > > > > > > > > that would > > > > > > > > > > benefit from the annotation. > > > > > > > > > > > > > > > > > > > > [...] > > > > > > > > > Since this got Acks, I figure I should carry it in my tree. > > > > > > > > > Let me know > > > > > > > > > if this should go via drm instead. > > > > > > > > > > > > > > > > > > Applied to for-next/hardening, thanks! > > > > > > > > > > > > > > > > > > [1/9] drm/amd/pm: Annotate struct > > > > > > > > > smu10_voltage_dependency_table with __counted_by > > > > > > > > > https://git.kernel.org/kees/c/a6046ac659d6 > > > > > > > > STOP! In a follow up discussion Alex and I figured out that > > > > > > > > this won't work. > > > > > > I'm so confused; from the discussion I saw that Alex said both > > > > > > instances > > > > > > were false positives? > > > > > > > > > > > > > > The value in the structure is byte swapped based on some > > > > > > > > firmware > > > > > > > > endianness which not necessary matches the CPU endianness. > > > > > > > SMU10 is APU only so the endianess of the SMU firmware and the CPU > > > > > > > will always match. > > > > > > Which I think is what is being said here? > > > > > > > > > > > > > > Please revert that one from going upstream if it's already on > > > > > > > > it's way. > > > > > > > > > > > > > > > > And because of those reasons I strongly think that patches like > > > > > > > > this > > > > > > > > should go through the DRM tree :) > > > > > > Sure, that's fine -- please let me know. It was others Acked/etc. > > > > > > Who > > > > > > should carry these patches? > > > > > Probably best if the relevant maintainer pick them up individually. > > > > > > > > > > Some of those structures are filled in by firmware/hardware and only > > > > > the > > > > > maintainers can judge if that value actually matches what the compiler > > > > > needs. > > > > > > > > > > We have cases where individual bits are used as flags or when the > > > > > size is > > > > > byte swapped etc... > > > > > > > > > > Even Alex and I didn't immediately say how and where that field is > > > > > actually > > > > > used and had to dig that up. That's where the confusion came from. > > > > Okay, I've dropped them all from my tree. Several had Acks/Reviews, so > > > > hopefully those can get picked up for the DRM tree? > > > I will pick those up to go through drm-misc-next. > > > > > > Going to ping maintainers once more when I'm not sure if stuff is correct > > > or > > > not. > > Sounds great; thanks! > > I wasn't 100% sure for the VC4 patch, but pushed the whole set to > drm-misc-next anyway. > > This also means that the patches are now auto merged into the drm-tip > integration branch and should any build or unit test go boom we should > notice immediately and can revert it pretty easily. Thanks very much; I'll keep an eye out for any reports. -- Kees Cook
Re: [Freedreno] [PATCH 0/9] drm: Annotate structs with __counted_by
On Mon, Oct 02, 2023 at 08:11:41PM +0200, Christian König wrote: > Am 02.10.23 um 20:08 schrieb Kees Cook: > > On Mon, Oct 02, 2023 at 08:01:57PM +0200, Christian König wrote: > > > Am 02.10.23 um 18:53 schrieb Kees Cook: > > > > On Mon, Oct 02, 2023 at 11:06:19AM -0400, Alex Deucher wrote: > > > > > On Mon, Oct 2, 2023 at 5:20 AM Christian König > > > > > wrote: > > > > > > Am 29.09.23 um 21:33 schrieb Kees Cook: > > > > > > > On Fri, 22 Sep 2023 10:32:05 -0700, Kees Cook wrote: > > > > > > > > This is a batch of patches touching drm for preparing for the > > > > > > > > coming > > > > > > > > implementation by GCC and Clang of the __counted_by attribute. > > > > > > > > Flexible > > > > > > > > array members annotated with __counted_by can have their > > > > > > > > accesses > > > > > > > > bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS > > > > > > > > (for array > > > > > > > > indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family > > > > > > > > functions). > > > > > > > > > > > > > > > > As found with Coccinelle[1], add __counted_by to structs that > > > > > > > > would > > > > > > > > benefit from the annotation. > > > > > > > > > > > > > > > > [...] > > > > > > > Since this got Acks, I figure I should carry it in my tree. Let > > > > > > > me know > > > > > > > if this should go via drm instead. > > > > > > > > > > > > > > Applied to for-next/hardening, thanks! > > > > > > > > > > > > > > [1/9] drm/amd/pm: Annotate struct smu10_voltage_dependency_table > > > > > > > with __counted_by > > > > > > > https://git.kernel.org/kees/c/a6046ac659d6 > > > > > > STOP! In a follow up discussion Alex and I figured out that this > > > > > > won't work. > > > > I'm so confused; from the discussion I saw that Alex said both instances > > > > were false positives? > > > > > > > > > > The value in the structure is byte swapped based on some firmware > > > > > > endianness which not necessary matches the CPU endianness. > > > > > SMU10 is APU only so the endianess of the SMU firmware and the CPU > > > > > will always match. > > > > Which I think is what is being said here? > > > > > > > > > > Please revert that one from going upstream if it's already on it's > > > > > > way. > > > > > > > > > > > > And because of those reasons I strongly think that patches like this > > > > > > should go through the DRM tree :) > > > > Sure, that's fine -- please let me know. It was others Acked/etc. Who > > > > should carry these patches? > > > Probably best if the relevant maintainer pick them up individually. > > > > > > Some of those structures are filled in by firmware/hardware and only the > > > maintainers can judge if that value actually matches what the compiler > > > needs. > > > > > > We have cases where individual bits are used as flags or when the size is > > > byte swapped etc... > > > > > > Even Alex and I didn't immediately say how and where that field is > > > actually > > > used and had to dig that up. That's where the confusion came from. > > Okay, I've dropped them all from my tree. Several had Acks/Reviews, so > > hopefully those can get picked up for the DRM tree? > > I will pick those up to go through drm-misc-next. > > Going to ping maintainers once more when I'm not sure if stuff is correct or > not. Sounds great; thanks! -Kees -- Kees Cook
Re: [Freedreno] [PATCH 0/9] drm: Annotate structs with __counted_by
On Mon, Oct 02, 2023 at 08:01:57PM +0200, Christian König wrote: > Am 02.10.23 um 18:53 schrieb Kees Cook: > > On Mon, Oct 02, 2023 at 11:06:19AM -0400, Alex Deucher wrote: > > > On Mon, Oct 2, 2023 at 5:20 AM Christian König > > > wrote: > > > > Am 29.09.23 um 21:33 schrieb Kees Cook: > > > > > On Fri, 22 Sep 2023 10:32:05 -0700, Kees Cook wrote: > > > > > > This is a batch of patches touching drm for preparing for the coming > > > > > > implementation by GCC and Clang of the __counted_by attribute. > > > > > > Flexible > > > > > > array members annotated with __counted_by can have their accesses > > > > > > bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS (for > > > > > > array > > > > > > indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family > > > > > > functions). > > > > > > > > > > > > As found with Coccinelle[1], add __counted_by to structs that would > > > > > > benefit from the annotation. > > > > > > > > > > > > [...] > > > > > Since this got Acks, I figure I should carry it in my tree. Let me > > > > > know > > > > > if this should go via drm instead. > > > > > > > > > > Applied to for-next/hardening, thanks! > > > > > > > > > > [1/9] drm/amd/pm: Annotate struct smu10_voltage_dependency_table with > > > > > __counted_by > > > > > https://git.kernel.org/kees/c/a6046ac659d6 > > > > STOP! In a follow up discussion Alex and I figured out that this won't > > > > work. > > I'm so confused; from the discussion I saw that Alex said both instances > > were false positives? > > > > > > The value in the structure is byte swapped based on some firmware > > > > endianness which not necessary matches the CPU endianness. > > > SMU10 is APU only so the endianess of the SMU firmware and the CPU > > > will always match. > > Which I think is what is being said here? > > > > > > Please revert that one from going upstream if it's already on it's way. > > > > > > > > And because of those reasons I strongly think that patches like this > > > > should go through the DRM tree :) > > Sure, that's fine -- please let me know. It was others Acked/etc. Who > > should carry these patches? > > Probably best if the relevant maintainer pick them up individually. > > Some of those structures are filled in by firmware/hardware and only the > maintainers can judge if that value actually matches what the compiler > needs. > > We have cases where individual bits are used as flags or when the size is > byte swapped etc... > > Even Alex and I didn't immediately say how and where that field is actually > used and had to dig that up. That's where the confusion came from. Okay, I've dropped them all from my tree. Several had Acks/Reviews, so hopefully those can get picked up for the DRM tree? Thanks! -Kees -- Kees Cook
Re: [Freedreno] [PATCH 0/9] drm: Annotate structs with __counted_by
On Mon, Oct 02, 2023 at 11:06:19AM -0400, Alex Deucher wrote: > On Mon, Oct 2, 2023 at 5:20 AM Christian König > wrote: > > > > Am 29.09.23 um 21:33 schrieb Kees Cook: > > > On Fri, 22 Sep 2023 10:32:05 -0700, Kees Cook wrote: > > >> This is a batch of patches touching drm for preparing for the coming > > >> implementation by GCC and Clang of the __counted_by attribute. Flexible > > >> array members annotated with __counted_by can have their accesses > > >> bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS (for array > > >> indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family functions). > > >> > > >> As found with Coccinelle[1], add __counted_by to structs that would > > >> benefit from the annotation. > > >> > > >> [...] > > > Since this got Acks, I figure I should carry it in my tree. Let me know > > > if this should go via drm instead. > > > > > > Applied to for-next/hardening, thanks! > > > > > > [1/9] drm/amd/pm: Annotate struct smu10_voltage_dependency_table with > > > __counted_by > > >https://git.kernel.org/kees/c/a6046ac659d6 > > > > STOP! In a follow up discussion Alex and I figured out that this won't work. I'm so confused; from the discussion I saw that Alex said both instances were false positives? > > > > The value in the structure is byte swapped based on some firmware > > endianness which not necessary matches the CPU endianness. > > SMU10 is APU only so the endianess of the SMU firmware and the CPU > will always match. Which I think is what is being said here? > > Please revert that one from going upstream if it's already on it's way. > > > > And because of those reasons I strongly think that patches like this > > should go through the DRM tree :) Sure, that's fine -- please let me know. It was others Acked/etc. Who should carry these patches? Thanks! -Kees > > > > Regards, > > Christian. > > > > > [2/9] drm/amdgpu/discovery: Annotate struct ip_hw_instance with > > > __counted_by > > >https://git.kernel.org/kees/c/4df33089b46f > > > [3/9] drm/i915/selftests: Annotate struct perf_series with __counted_by > > >https://git.kernel.org/kees/c/ffd3f823bdf6 > > > [4/9] drm/msm/dpu: Annotate struct dpu_hw_intr with __counted_by > > >https://git.kernel.org/kees/c/2de35a989b76 > > > [5/9] drm/nouveau/pm: Annotate struct nvkm_perfdom with __counted_by > > >https://git.kernel.org/kees/c/188aeb08bfaa > > > [6/9] drm/vc4: Annotate struct vc4_perfmon with __counted_by > > >https://git.kernel.org/kees/c/59a54dc896c3 > > > [7/9] drm/virtio: Annotate struct virtio_gpu_object_array with > > > __counted_by > > >https://git.kernel.org/kees/c/5cd476de33af > > > [8/9] drm/vmwgfx: Annotate struct vmw_surface_dirty with __counted_by > > >https://git.kernel.org/kees/c/b426f2e5356a > > > [9/9] drm/v3d: Annotate struct v3d_perfmon with __counted_by > > >https://git.kernel.org/kees/c/dc662fa1b0e4 > > > > > > Take care, > > > > > -- Kees Cook
Re: [Freedreno] [PATCH 0/9] drm: Annotate structs with __counted_by
On Fri, 22 Sep 2023 10:32:05 -0700, Kees Cook wrote: > This is a batch of patches touching drm for preparing for the coming > implementation by GCC and Clang of the __counted_by attribute. Flexible > array members annotated with __counted_by can have their accesses > bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS (for array > indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family functions). > > As found with Coccinelle[1], add __counted_by to structs that would > benefit from the annotation. > > [...] Since this got Acks, I figure I should carry it in my tree. Let me know if this should go via drm instead. Applied to for-next/hardening, thanks! [1/9] drm/amd/pm: Annotate struct smu10_voltage_dependency_table with __counted_by https://git.kernel.org/kees/c/a6046ac659d6 [2/9] drm/amdgpu/discovery: Annotate struct ip_hw_instance with __counted_by https://git.kernel.org/kees/c/4df33089b46f [3/9] drm/i915/selftests: Annotate struct perf_series with __counted_by https://git.kernel.org/kees/c/ffd3f823bdf6 [4/9] drm/msm/dpu: Annotate struct dpu_hw_intr with __counted_by https://git.kernel.org/kees/c/2de35a989b76 [5/9] drm/nouveau/pm: Annotate struct nvkm_perfdom with __counted_by https://git.kernel.org/kees/c/188aeb08bfaa [6/9] drm/vc4: Annotate struct vc4_perfmon with __counted_by https://git.kernel.org/kees/c/59a54dc896c3 [7/9] drm/virtio: Annotate struct virtio_gpu_object_array with __counted_by https://git.kernel.org/kees/c/5cd476de33af [8/9] drm/vmwgfx: Annotate struct vmw_surface_dirty with __counted_by https://git.kernel.org/kees/c/b426f2e5356a [9/9] drm/v3d: Annotate struct v3d_perfmon with __counted_by https://git.kernel.org/kees/c/dc662fa1b0e4 Take care, -- Kees Cook
Re: [Freedreno] [PATCH 1/9] drm/amd/pm: Annotate struct smu10_voltage_dependency_table with __counted_by
On Mon, Sep 25, 2023 at 08:30:30AM +0200, Christian König wrote:
> Am 22.09.23 um 19:41 schrieb Alex Deucher:
> > On Fri, Sep 22, 2023 at 1:32 PM Kees Cook wrote:
> > > Prepare for the coming implementation by GCC and Clang of the __counted_by
> > > attribute. Flexible array members annotated with __counted_by can have
> > > their accesses bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS
> > > (for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family
> > > functions).
> > >
> > > As found with Coccinelle[1], add __counted_by for struct
> > > smu10_voltage_dependency_table.
> > >
> > > [1]
> > > https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci
> > >
> > > Cc: Evan Quan
> > > Cc: Alex Deucher
> > > Cc: "Christian König"
> > > Cc: "Pan, Xinhui"
> > > Cc: David Airlie
> > > Cc: Daniel Vetter
> > > Cc: Xiaojian Du
> > > Cc: Huang Rui
> > > Cc: Kevin Wang
> > > Cc: amd-...@lists.freedesktop.org
> > > Cc: dri-de...@lists.freedesktop.org
> > > Signed-off-by: Kees Cook
> > Acked-by: Alex Deucher
>
> Mhm, I'm not sure if this is a good idea. That is a structure filled in by
> the firmware, isn't it?
>
> That would imply that we might need to byte swap count before it is
> checkable.
The script found this instance because of this:
static int smu10_get_clock_voltage_dependency_table(struct pp_hwmgr *hwmgr,
struct smu10_voltage_dependency_table **pptable,
uint32_t num_entry, const DpmClock_t
*pclk_dependency_table)
{
uint32_t i;
struct smu10_voltage_dependency_table *ptable;
ptable = kzalloc(struct_size(ptable, entries, num_entry), GFP_KERNEL);
if (NULL == ptable)
return -ENOMEM;
ptable->count = num_entry;
So the implication is that it's native byte order... but you tell me! I
certainly don't want this annotation if it's going to break stuff. :)
--
Kees Cook
Re: [Freedreno] [PATCH 3/9] drm/i915/selftests: Annotate struct perf_series with __counted_by
On Mon, Sep 25, 2023 at 12:08:36PM +0200, Andrzej Hajda wrote: > > > On 22.09.2023 19:32, Kees Cook wrote: > > Prepare for the coming implementation by GCC and Clang of the __counted_by > > attribute. Flexible array members annotated with __counted_by can have > > their accesses bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS > > (for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family > > functions). > > > > As found with Coccinelle[1], add __counted_by for struct perf_series. > > > > [1] > > https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci > > > > Cc: Jani Nikula > > Cc: Joonas Lahtinen > > Cc: Rodrigo Vivi > > Cc: Tvrtko Ursulin > > Cc: David Airlie > > Cc: Daniel Vetter > > Cc: Chris Wilson > > Cc: John Harrison > > Cc: Andi Shyti > > Cc: Matthew Brost > > Cc: intel-...@lists.freedesktop.org > > Cc: dri-de...@lists.freedesktop.org > > Signed-off-by: Kees Cook > > I am surprised this is the only finding in i915, I would expected more. I'm sure there are more, but it's likely my Coccinelle pattern didn't catch it. There are many many flexible arrays in drm. :) $ grep -nRH '\[\];$' drivers/gpu/drm include/uapi/drm | grep -v :extern | wc -l 122 If anyone has some patterns I can add to the Coccinelle script, I can take another pass at it. > Anyway: > > Reviewed-by: Andrzej Hajda Thank you! -Kees -- Kees Cook
[Freedreno] [PATCH 9/9] drm/v3d: Annotate struct v3d_perfmon with __counted_by
Prepare for the coming implementation by GCC and Clang of the __counted_by
attribute. Flexible array members annotated with __counted_by can have
their accesses bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS
(for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family
functions).
As found with Coccinelle[1], add __counted_by for struct v3d_perfmon.
[1]
https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci
Cc: Emma Anholt
Cc: Melissa Wen
Cc: David Airlie
Cc: Daniel Vetter
Cc: dri-de...@lists.freedesktop.org
Signed-off-by: Kees Cook
---
drivers/gpu/drm/v3d/v3d_drv.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/v3d/v3d_drv.h b/drivers/gpu/drm/v3d/v3d_drv.h
index 7f664a4b2a75..106454f28956 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.h
+++ b/drivers/gpu/drm/v3d/v3d_drv.h
@@ -59,7 +59,7 @@ struct v3d_perfmon {
* values can't be reset, but you can fake a reset by
* destroying the perfmon and creating a new one.
*/
- u64 values[];
+ u64 values[] __counted_by(ncounters);
};
struct v3d_dev {
--
2.34.1
[Freedreno] [PATCH 8/9] drm/vmwgfx: Annotate struct vmw_surface_dirty with __counted_by
Prepare for the coming implementation by GCC and Clang of the __counted_by
attribute. Flexible array members annotated with __counted_by can have
their accesses bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS
(for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family
functions).
As found with Coccinelle[1], add __counted_by for struct vmw_surface_dirty.
[1]
https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci
Cc: Zack Rusin
Cc: VMware Graphics Reviewers
Cc: David Airlie
Cc: Daniel Vetter
Cc: dri-de...@lists.freedesktop.org
Signed-off-by: Kees Cook
---
drivers/gpu/drm/vmwgfx/vmwgfx_surface.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
index 5db403ee8261..2d1d857f99ae 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
@@ -77,7 +77,7 @@ struct vmw_surface_offset {
struct vmw_surface_dirty {
struct vmw_surface_cache cache;
u32 num_subres;
- SVGA3dBox boxes[];
+ SVGA3dBox boxes[] __counted_by(num_subres);
};
static void vmw_user_surface_free(struct vmw_resource *res);
--
2.34.1
[Freedreno] [PATCH 4/9] drm/msm/dpu: Annotate struct dpu_hw_intr with __counted_by
Prepare for the coming implementation by GCC and Clang of the __counted_by
attribute. Flexible array members annotated with __counted_by can have
their accesses bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS
(for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family
functions).
As found with Coccinelle[1], add __counted_by for struct dpu_hw_intr.
[1]
https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci
Cc: Rob Clark
Cc: Abhinav Kumar
Cc: Dmitry Baryshkov
Cc: Sean Paul
Cc: Marijn Suijten
Cc: David Airlie
Cc: Daniel Vetter
Cc: Bjorn Andersson
Cc: linux-arm-...@vger.kernel.org
Cc: dri-de...@lists.freedesktop.org
Cc: freedreno@lists.freedesktop.org
Signed-off-by: Kees Cook
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h
index dab761e54863..50cf9523d367 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h
@@ -61,7 +61,7 @@ struct dpu_hw_intr {
void (*cb)(void *arg, int irq_idx);
void *arg;
atomic_t count;
- } irq_tbl[];
+ } irq_tbl[] __counted_by(total_irqs);
};
/**
--
2.34.1
[Freedreno] [PATCH 5/9] drm/nouveau/pm: Annotate struct nvkm_perfdom with __counted_by
Prepare for the coming implementation by GCC and Clang of the __counted_by
attribute. Flexible array members annotated with __counted_by can have
their accesses bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS
(for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family
functions).
As found with Coccinelle[1], add __counted_by for struct nvkm_perfdom.
[1]
https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci
Cc: Ben Skeggs
Cc: Karol Herbst
Cc: Lyude Paul
Cc: David Airlie
Cc: Daniel Vetter
Cc: dri-de...@lists.freedesktop.org
Cc: nouv...@lists.freedesktop.org
Signed-off-by: Kees Cook
---
drivers/gpu/drm/nouveau/nvkm/engine/pm/priv.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/pm/priv.h
b/drivers/gpu/drm/nouveau/nvkm/engine/pm/priv.h
index 6ae25d3e7f45..c011227f7052 100644
--- a/drivers/gpu/drm/nouveau/nvkm/engine/pm/priv.h
+++ b/drivers/gpu/drm/nouveau/nvkm/engine/pm/priv.h
@@ -82,7 +82,7 @@ struct nvkm_perfdom {
u8 mode;
u32 clk;
u16 signal_nr;
- struct nvkm_perfsig signal[];
+ struct nvkm_perfsig signal[] __counted_by(signal_nr);
};
struct nvkm_funcdom {
--
2.34.1
[Freedreno] [PATCH 3/9] drm/i915/selftests: Annotate struct perf_series with __counted_by
Prepare for the coming implementation by GCC and Clang of the __counted_by
attribute. Flexible array members annotated with __counted_by can have
their accesses bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS
(for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family
functions).
As found with Coccinelle[1], add __counted_by for struct perf_series.
[1]
https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci
Cc: Jani Nikula
Cc: Joonas Lahtinen
Cc: Rodrigo Vivi
Cc: Tvrtko Ursulin
Cc: David Airlie
Cc: Daniel Vetter
Cc: Chris Wilson
Cc: John Harrison
Cc: Andi Shyti
Cc: Matthew Brost
Cc: intel-...@lists.freedesktop.org
Cc: dri-de...@lists.freedesktop.org
Signed-off-by: Kees Cook
---
drivers/gpu/drm/i915/selftests/i915_request.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/selftests/i915_request.c
b/drivers/gpu/drm/i915/selftests/i915_request.c
index a9b79888c193..acae30a04a94 100644
--- a/drivers/gpu/drm/i915/selftests/i915_request.c
+++ b/drivers/gpu/drm/i915/selftests/i915_request.c
@@ -1924,7 +1924,7 @@ struct perf_stats {
struct perf_series {
struct drm_i915_private *i915;
unsigned int nengines;
- struct intel_context *ce[];
+ struct intel_context *ce[] __counted_by(nengines);
};
static int cmp_u32(const void *A, const void *B)
--
2.34.1
[Freedreno] [PATCH 7/9] drm/virtio: Annotate struct virtio_gpu_object_array with __counted_by
Prepare for the coming implementation by GCC and Clang of the __counted_by
attribute. Flexible array members annotated with __counted_by can have
their accesses bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS
(for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family
functions).
As found with Coccinelle[1], add __counted_by for struct
virtio_gpu_object_array.
[1]
https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci
Cc: David Airlie
Cc: Gerd Hoffmann
Cc: Gurchetan Singh
Cc: Chia-I Wu
Cc: Daniel Vetter
Cc: dri-de...@lists.freedesktop.org
Cc: virtualizat...@lists.linux-foundation.org
Signed-off-by: Kees Cook
---
drivers/gpu/drm/virtio/virtgpu_drv.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h
b/drivers/gpu/drm/virtio/virtgpu_drv.h
index 8513b671f871..96365a772f77 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.h
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
@@ -119,7 +119,7 @@ struct virtio_gpu_object_array {
struct ww_acquire_ctx ticket;
struct list_head next;
u32 nents, total;
- struct drm_gem_object *objs[];
+ struct drm_gem_object *objs[] __counted_by(total);
};
struct virtio_gpu_vbuffer;
--
2.34.1
[Freedreno] [PATCH 6/9] drm/vc4: Annotate struct vc4_perfmon with __counted_by
Prepare for the coming implementation by GCC and Clang of the __counted_by
attribute. Flexible array members annotated with __counted_by can have
their accesses bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS
(for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family
functions).
As found with Coccinelle[1], add __counted_by for struct vc4_perfmon.
[1]
https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci
Cc: Emma Anholt
Cc: Maxime Ripard
Cc: David Airlie
Cc: Daniel Vetter
Cc: dri-de...@lists.freedesktop.org
Signed-off-by: Kees Cook
---
drivers/gpu/drm/vc4/vc4_drv.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
index bf66499765fb..ab61e96e7e14 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -76,7 +76,7 @@ struct vc4_perfmon {
* Note that counter values can't be reset, but you can fake a reset by
* destroying the perfmon and creating a new one.
*/
- u64 counters[];
+ u64 counters[] __counted_by(ncounters);
};
struct vc4_dev {
--
2.34.1
[Freedreno] [PATCH 2/9] drm/amdgpu/discovery: Annotate struct ip_hw_instance with __counted_by
Prepare for the coming implementation by GCC and Clang of the __counted_by
attribute. Flexible array members annotated with __counted_by can have
their accesses bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS
(for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family
functions).
As found with Coccinelle[1], add __counted_by for struct ip_hw_instance.
[1]
https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci
Cc: Alex Deucher
Cc: "Christian König"
Cc: "Pan, Xinhui"
Cc: David Airlie
Cc: Daniel Vetter
Cc: Hawking Zhang
Cc: amd-...@lists.freedesktop.org
Cc: dri-de...@lists.freedesktop.org
Signed-off-by: Kees Cook
---
drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
index d1bc7b212520..be4c97a3d7bf 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
@@ -662,7 +662,7 @@ struct ip_hw_instance {
u8 harvest;
int num_base_addresses;
- u32 base_addr[];
+ u32 base_addr[] __counted_by(num_base_addresses);
};
struct ip_hw_id {
--
2.34.1
[Freedreno] [PATCH 0/9] drm: Annotate structs with __counted_by
Hi, This is a batch of patches touching drm for preparing for the coming implementation by GCC and Clang of the __counted_by attribute. Flexible array members annotated with __counted_by can have their accesses bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS (for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family functions). As found with Coccinelle[1], add __counted_by to structs that would benefit from the annotation. Since the element count member must be set before accessing the annotated flexible array member, some patches also move the member's initialization earlier. (These are noted in the individual patches.) -Kees [1] https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci Kees Cook (9): drm/amd/pm: Annotate struct smu10_voltage_dependency_table with __counted_by drm/amdgpu/discovery: Annotate struct ip_hw_instance with __counted_by drm/i915/selftests: Annotate struct perf_series with __counted_by drm/msm/dpu: Annotate struct dpu_hw_intr with __counted_by drm/nouveau/pm: Annotate struct nvkm_perfdom with __counted_by drm/vc4: Annotate struct vc4_perfmon with __counted_by drm/virtio: Annotate struct virtio_gpu_object_array with __counted_by drm/vmwgfx: Annotate struct vmw_surface_dirty with __counted_by drm/v3d: Annotate struct v3d_perfmon with __counted_by drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c| 2 +- drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.h | 2 +- drivers/gpu/drm/i915/selftests/i915_request.c| 2 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h| 2 +- drivers/gpu/drm/nouveau/nvkm/engine/pm/priv.h| 2 +- drivers/gpu/drm/v3d/v3d_drv.h| 2 +- drivers/gpu/drm/vc4/vc4_drv.h| 2 +- drivers/gpu/drm/virtio/virtgpu_drv.h | 2 +- drivers/gpu/drm/vmwgfx/vmwgfx_surface.c | 2 +- 9 files changed, 9 insertions(+), 9 deletions(-) -- 2.34.1
[Freedreno] [PATCH 1/9] drm/amd/pm: Annotate struct smu10_voltage_dependency_table with __counted_by
Prepare for the coming implementation by GCC and Clang of the __counted_by
attribute. Flexible array members annotated with __counted_by can have
their accesses bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS
(for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family
functions).
As found with Coccinelle[1], add __counted_by for struct
smu10_voltage_dependency_table.
[1]
https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci
Cc: Evan Quan
Cc: Alex Deucher
Cc: "Christian König"
Cc: "Pan, Xinhui"
Cc: David Airlie
Cc: Daniel Vetter
Cc: Xiaojian Du
Cc: Huang Rui
Cc: Kevin Wang
Cc: amd-...@lists.freedesktop.org
Cc: dri-de...@lists.freedesktop.org
Signed-off-by: Kees Cook
---
drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.h
b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.h
index 808e0ecbe1f0..42adc2a3dcbc 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.h
+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.h
@@ -192,7 +192,7 @@ struct smu10_clock_voltage_dependency_record {
struct smu10_voltage_dependency_table {
uint32_t count;
- struct smu10_clock_voltage_dependency_record entries[];
+ struct smu10_clock_voltage_dependency_record entries[]
__counted_by(count);
};
struct smu10_clock_voltage_information {
--
2.34.1
Re: [Freedreno] [PATCH 3/3] drm/pl111: depend on CONFIG_VEXPRESS_CONFIG
On Thu, Jun 03, 2021 at 11:41:01PM +0200, Daniel Vetter wrote: > On Thu, Jun 3, 2021 at 11:29 PM Kees Cook wrote: > > > > On Thu, Jun 03, 2021 at 02:19:52PM -0700, Kees Cook wrote: > > > On Thu, Jun 03, 2021 at 09:19:42PM +0200, Daniel Vetter wrote: > > > > On Thu, Jun 3, 2021 at 8:43 PM Rob Herring wrote: > > > > > > > > > > On Wed, Jun 2, 2021 at 4:53 PM Kees Cook > > > > > wrote: > > > > > > > > > > > > Avoid randconfig build failures by requiring VEXPRESS_CONFIG: > > > > > > > > > > > > aarch64-linux-gnu-ld: drivers/gpu/drm/pl111/pl111_versatile.o: in > > > > > > function `pl111_vexpress_clcd_init': > > > > > > pl111_versatile.c:(.text+0x220): undefined reference to > > > > > > `devm_regmap_init_vexpress_config' > > > > > > > > > > pl111_vexpress_clcd_init() starts with: > > > > > > > > > > if (!IS_ENABLED(CONFIG_VEXPRESS_CONFIG)) > > > > > return -ENODEV; > > > > > > > > > > Isn't that supposed to be enough to avoid an undefined reference? > > > > > > Ah! I missed that when reading the code. I see the problem now. It's > > > because of: > > > > > > CONFIG_VEXPRESS_CONFIG=m > > > CONFIG_DRM_PL111=y > > > > > > I think the right fix is: > > > > > > diff --git a/drivers/gpu/drm/pl111/Kconfig b/drivers/gpu/drm/pl111/Kconfig > > > index 80f6748055e3..662fc38f92ba 100644 > > > --- a/drivers/gpu/drm/pl111/Kconfig > > > +++ b/drivers/gpu/drm/pl111/Kconfig > > > @@ -3,6 +3,7 @@ config DRM_PL111 > > > tristate "DRM Support for PL111 CLCD Controller" > > > depends on DRM > > > depends on ARM || ARM64 || COMPILE_TEST > > > + depends on VEXPRESS_CONFIG=y || VEXPRESS_CONFIG=DRM > > > > Oops, no, I had this backwairds: > > > > depends on !VEXPRESS_CONFIG || VEXPRESS_CONFIG=DRM > > Can you pls throw this into an incremental patch on top of > drm-misc-next? It's a non-rebasing tree and all that (linux-next > should have it next day too I guess). Ah! Yes, sorry. I wasn't sure if you'd reverted it yet. One moment... -Kees -- Kees Cook ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH 3/3] drm/pl111: depend on CONFIG_VEXPRESS_CONFIG
On Thu, Jun 03, 2021 at 02:19:52PM -0700, Kees Cook wrote:
> On Thu, Jun 03, 2021 at 09:19:42PM +0200, Daniel Vetter wrote:
> > On Thu, Jun 3, 2021 at 8:43 PM Rob Herring wrote:
> > >
> > > On Wed, Jun 2, 2021 at 4:53 PM Kees Cook wrote:
> > > >
> > > > Avoid randconfig build failures by requiring VEXPRESS_CONFIG:
> > > >
> > > > aarch64-linux-gnu-ld: drivers/gpu/drm/pl111/pl111_versatile.o: in
> > > > function `pl111_vexpress_clcd_init':
> > > > pl111_versatile.c:(.text+0x220): undefined reference to
> > > > `devm_regmap_init_vexpress_config'
> > >
> > > pl111_vexpress_clcd_init() starts with:
> > >
> > > if (!IS_ENABLED(CONFIG_VEXPRESS_CONFIG))
> > > return -ENODEV;
> > >
> > > Isn't that supposed to be enough to avoid an undefined reference?
>
> Ah! I missed that when reading the code. I see the problem now. It's
> because of:
>
> CONFIG_VEXPRESS_CONFIG=m
> CONFIG_DRM_PL111=y
>
> I think the right fix is:
>
> diff --git a/drivers/gpu/drm/pl111/Kconfig b/drivers/gpu/drm/pl111/Kconfig
> index 80f6748055e3..662fc38f92ba 100644
> --- a/drivers/gpu/drm/pl111/Kconfig
> +++ b/drivers/gpu/drm/pl111/Kconfig
> @@ -3,6 +3,7 @@ config DRM_PL111
> tristate "DRM Support for PL111 CLCD Controller"
> depends on DRM
> depends on ARM || ARM64 || COMPILE_TEST
> + depends on VEXPRESS_CONFIG=y || VEXPRESS_CONFIG=DRM
Oops, no, I had this backwairds:
depends on !VEXPRESS_CONFIG || VEXPRESS_CONFIG=DRM
_that_ lets me build with:
# CONFIG_VEXPRESS_CONFIG is not set
CONFIG_DRM_PL111=y
CONFIG_VEXPRESS_CONFIG=y
CONFIG_DRM_PL111=y
CONFIG_VEXPRESS_CONFIG=m
CONFIG_DRM_PL111=m
CONFIG_VEXPRESS_CONFIG=y
CONFIG_DRM_PL111=m
and disallows:
CONFIG_VEXPRESS_CONFIG=m
CONFIG_DRM_PL111=y
(this will force CONFIG_DRM_PL111=m)
-Kees
> depends on COMMON_CLK
> select DRM_KMS_HELPER
> select DRM_KMS_CMA_HELPER
>
> I will go check the defconfigs Rob mentioned...
>
> > > Making the whole file depend on VEXPRESS_CONFIG is not right either.
> > > Not all platforms need it.
> >
> > It needs a compile-time status inline then for the functions we're
> > using in pl111.
>
> FYI, this is the config I was working from, which was throwing link errors:
> https://lore.kernel.org/lkml/202105300926.fx0myysp-...@intel.com/
>
> > -Daniel
> >
> > >
> > > >
> > > > Fixes: 826fc86b5903 ("drm: pl111: Move VExpress setup into versatile
> > > > init")
> > > > Signed-off-by: Kees Cook
> > > > ---
> > > > drivers/gpu/drm/pl111/Kconfig | 2 +-
> > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/pl111/Kconfig
> > > > b/drivers/gpu/drm/pl111/Kconfig
> > > > index 80f6748055e3..c5210a5bef1b 100644
> > > > --- a/drivers/gpu/drm/pl111/Kconfig
> > > > +++ b/drivers/gpu/drm/pl111/Kconfig
> > > > @@ -2,7 +2,7 @@
> > > > config DRM_PL111
> > > > tristate "DRM Support for PL111 CLCD Controller"
> > > > depends on DRM
> > > > - depends on ARM || ARM64 || COMPILE_TEST
> > > > + depends on VEXPRESS_CONFIG
> > > > depends on COMMON_CLK
> > > > select DRM_KMS_HELPER
> > > > select DRM_KMS_CMA_HELPER
> > > > --
> > > > 2.25.1
> > > >
> >
> >
> >
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
>
> --
> Kees Cook
--
Kees Cook
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH 3/3] drm/pl111: depend on CONFIG_VEXPRESS_CONFIG
On Thu, Jun 03, 2021 at 09:19:42PM +0200, Daniel Vetter wrote:
> On Thu, Jun 3, 2021 at 8:43 PM Rob Herring wrote:
> >
> > On Wed, Jun 2, 2021 at 4:53 PM Kees Cook wrote:
> > >
> > > Avoid randconfig build failures by requiring VEXPRESS_CONFIG:
> > >
> > > aarch64-linux-gnu-ld: drivers/gpu/drm/pl111/pl111_versatile.o: in
> > > function `pl111_vexpress_clcd_init':
> > > pl111_versatile.c:(.text+0x220): undefined reference to
> > > `devm_regmap_init_vexpress_config'
> >
> > pl111_vexpress_clcd_init() starts with:
> >
> > if (!IS_ENABLED(CONFIG_VEXPRESS_CONFIG))
> > return -ENODEV;
> >
> > Isn't that supposed to be enough to avoid an undefined reference?
Ah! I missed that when reading the code. I see the problem now. It's
because of:
CONFIG_VEXPRESS_CONFIG=m
CONFIG_DRM_PL111=y
I think the right fix is:
diff --git a/drivers/gpu/drm/pl111/Kconfig b/drivers/gpu/drm/pl111/Kconfig
index 80f6748055e3..662fc38f92ba 100644
--- a/drivers/gpu/drm/pl111/Kconfig
+++ b/drivers/gpu/drm/pl111/Kconfig
@@ -3,6 +3,7 @@ config DRM_PL111
tristate "DRM Support for PL111 CLCD Controller"
depends on DRM
depends on ARM || ARM64 || COMPILE_TEST
+ depends on VEXPRESS_CONFIG=y || VEXPRESS_CONFIG=DRM
depends on COMMON_CLK
select DRM_KMS_HELPER
select DRM_KMS_CMA_HELPER
I will go check the defconfigs Rob mentioned...
> > Making the whole file depend on VEXPRESS_CONFIG is not right either.
> > Not all platforms need it.
>
> It needs a compile-time status inline then for the functions we're
> using in pl111.
FYI, this is the config I was working from, which was throwing link errors:
https://lore.kernel.org/lkml/202105300926.fx0myysp-...@intel.com/
> -Daniel
>
> >
> > >
> > > Fixes: 826fc86b5903 ("drm: pl111: Move VExpress setup into versatile
> > > init")
> > > Signed-off-by: Kees Cook
> > > ---
> > > drivers/gpu/drm/pl111/Kconfig | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/pl111/Kconfig b/drivers/gpu/drm/pl111/Kconfig
> > > index 80f6748055e3..c5210a5bef1b 100644
> > > --- a/drivers/gpu/drm/pl111/Kconfig
> > > +++ b/drivers/gpu/drm/pl111/Kconfig
> > > @@ -2,7 +2,7 @@
> > > config DRM_PL111
> > > tristate "DRM Support for PL111 CLCD Controller"
> > > depends on DRM
> > > - depends on ARM || ARM64 || COMPILE_TEST
> > > + depends on VEXPRESS_CONFIG
> > > depends on COMMON_CLK
> > > select DRM_KMS_HELPER
> > > select DRM_KMS_CMA_HELPER
> > > --
> > > 2.25.1
> > >
>
>
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
--
Kees Cook
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH 2/3] drm/msm/a6xx: add CONFIG_QCOM_LLCC dependency
From: Arnd Bergmann
When LLCC support is in a loadable module, the adreno support
cannot be built-in:
aarch64-linux-ld: drivers/gpu/drm/msm/adreno/a6xx_gpu.o: in function
`a6xx_gpu_init':
a6xx_gpu.c:(.text+0xe0): undefined reference to `llcc_slice_getd'
a6xx_gpu.c:(.text+0xe0): relocation truncated to fit: R_AARCH64_CALL26 against
undefined symbol `llcc_slice_getd'
aarch64-linux-ld: a6xx_gpu.c:(.text+0xec): undefined reference to
`llcc_slice_getd'
a6xx_gpu.c:(.text+0xec): relocation truncated to fit: R_AARCH64_CALL26 against
undefined symbol `llcc_slice_getd'
aarch64-linux-ld: drivers/gpu/drm/msm/adreno/a6xx_gpu.o: in function
`a6xx_destroy':
a6xx_gpu.c:(.text+0x274): undefined reference to `llcc_slice_putd'
a6xx_gpu.c:(.text+0x274): relocation truncated to fit: R_AARCH64_CALL26 against
undefined symbol `llcc_slice_putd'
aarch64-linux-ld: a6xx_gpu.c:(.text+0x27c): undefined reference to
`llcc_slice_putd'
Add a Kconfig dependency that disallows the broken configuration
but allows all working ones.
Fixes: 474dadb8b0d5 ("drm/msm/a6xx: Add support for using system cache(LLC)")
Signed-off-by: Arnd Bergmann
Reported-by: kernel test robot
Reviewed-by: Sai Prakash Ranjan
Signed-off-by: Kees Cook
Link: https://lore.kernel.org/r/20210103140407.3917405-1-a...@kernel.org
---
drivers/gpu/drm/msm/Kconfig | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig
index 10f693ea89d3..52536e7adb95 100644
--- a/drivers/gpu/drm/msm/Kconfig
+++ b/drivers/gpu/drm/msm/Kconfig
@@ -7,6 +7,8 @@ config DRM_MSM
depends on IOMMU_SUPPORT
depends on OF && COMMON_CLK
depends on QCOM_OCMEM || QCOM_OCMEM=n
+ depends on QCOM_LLCC || QCOM_LLCC=n
+ depends on QCOM_COMMAND_DB || QCOM_COMMAND_DB=n
select IOMMU_IO_PGTABLE
select QCOM_MDT_LOADER if ARCH_QCOM
select REGULATOR
@@ -15,7 +17,6 @@ config DRM_MSM
select SHMEM
select TMPFS
select QCOM_SCM if ARCH_QCOM
- select QCOM_COMMAND_DB if ARCH_QCOM
select WANT_DEV_COREDUMP
select SND_SOC_HDMI_CODEC if SND_SOC
select SYNC_FILE
--
2.25.1
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH 3/3] drm/pl111: depend on CONFIG_VEXPRESS_CONFIG
Avoid randconfig build failures by requiring VEXPRESS_CONFIG:
aarch64-linux-gnu-ld: drivers/gpu/drm/pl111/pl111_versatile.o: in function
`pl111_vexpress_clcd_init':
pl111_versatile.c:(.text+0x220): undefined reference to
`devm_regmap_init_vexpress_config'
Fixes: 826fc86b5903 ("drm: pl111: Move VExpress setup into versatile init")
Signed-off-by: Kees Cook
---
drivers/gpu/drm/pl111/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/pl111/Kconfig b/drivers/gpu/drm/pl111/Kconfig
index 80f6748055e3..c5210a5bef1b 100644
--- a/drivers/gpu/drm/pl111/Kconfig
+++ b/drivers/gpu/drm/pl111/Kconfig
@@ -2,7 +2,7 @@
config DRM_PL111
tristate "DRM Support for PL111 CLCD Controller"
depends on DRM
- depends on ARM || ARM64 || COMPILE_TEST
+ depends on VEXPRESS_CONFIG
depends on COMMON_CLK
select DRM_KMS_HELPER
select DRM_KMS_CMA_HELPER
--
2.25.1
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH 1/3] drm: Avoid circular dependencies for CONFIG_FB
When cleaning up other drm config dependencies, it is too easy to create larger problems. Instead, mark CONFIG_FB as a "depends": drivers/gpu/drm/Kconfig:74:error: recursive dependency detected! Suggested-by: Arnd Bergmann Link: https://lore.kernel.org/lkml/cak8p3a3juqs6c5tessnmbqfuymewj9fhqrizyhcfoxf8rgy...@mail.gmail.com/ Signed-off-by: Kees Cook --- drivers/gpu/drm/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index 3c16bd1afd87..90891284ccec 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -83,7 +83,7 @@ config DRM_KMS_HELPER config DRM_KMS_FB_HELPER bool depends on DRM_KMS_HELPER - select FB + depends on FB select FRAMEBUFFER_CONSOLE if !EXPERT select FRAMEBUFFER_CONSOLE_DETECT_PRIMARY if FRAMEBUFFER_CONSOLE select FB_SYS_FOPS -- 2.25.1 ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH 0/3] drm: Fix randconfig link failures
Hi, While tracking down spurious "orphan section" warnings on arm and arm64, I needed to fix several other issues that it seems other folks have tripped over before. Here's the series that fixed everything for me... -Kees Arnd Bergmann (1): drm/msm/a6xx: add CONFIG_QCOM_LLCC dependency Kees Cook (2): drm: Avoid circular dependencies for CONFIG_FB drm/pl111: depend on CONFIG_VEXPRESS_CONFIG drivers/gpu/drm/Kconfig | 2 +- drivers/gpu/drm/msm/Kconfig | 3 ++- drivers/gpu/drm/pl111/Kconfig | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) -- 2.25.1 ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH] drm/msm/adreno: Remove VLA usage
On Fri, Jun 29, 2018 at 2:20 PM, Jordan Crouse wrote: > On Fri, Jun 29, 2018 at 11:48:18AM -0700, Kees Cook wrote: >> In the quest to remove all stack VLA usage from the kernel[1], this >> switches to using a kasprintf()ed buffer. Return paths are updated >> to free the allocation. >> >> [1] >> https://lkml.kernel.org/r/CA+55aFzCG-zNmZwX4A2FQpadafLfEzK6CC=qpxydaacu1rq...@mail.gmail.com > > Thanks for doing this. > > Acked-by: Jordan Crouse Hi Rob, Is this something you can take via your tree? Thanks, -Kees -- Kees Cook Pixel Security ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH] drm/msm/adreno: Remove VLA usage
In the quest to remove all stack VLA usage from the kernel[1], this
switches to using a kasprintf()ed buffer. Return paths are updated
to free the allocation.
[1]
https://lkml.kernel.org/r/CA+55aFzCG-zNmZwX4A2FQpadafLfEzK6CC=qpxydaacu1rq...@mail.gmail.com
Signed-off-by: Kees Cook
---
drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 7 +--
drivers/gpu/drm/msm/adreno/adreno_gpu.c | 28 +
2 files changed, 24 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
index d39400e5bc42..f5f76f8151f9 100644
--- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
@@ -11,6 +11,7 @@
*
*/
+#include
#include
#include
#include
@@ -19,6 +20,7 @@
#include
#include
#include
+#include
#include "msm_gem.h"
#include "msm_mmu.h"
#include "a5xx_gpu.h"
@@ -91,12 +93,13 @@ static int zap_shader_load_mdt(struct msm_gpu *gpu, const
char *fwname)
ret = qcom_mdt_load(dev, fw, fwname, GPU_PAS_ID,
mem_region, mem_phys, mem_size, NULL);
} else {
- char newname[strlen("qcom/") + strlen(fwname) + 1];
+ char *newname;
- sprintf(newname, "qcom/%s", fwname);
+ newname = kasprintf(GFP_KERNEL, "qcom/%s", fwname);
ret = qcom_mdt_load(dev, fw, newname, GPU_PAS_ID,
mem_region, mem_phys, mem_size, NULL);
+ kfree(newname);
}
if (ret)
goto out;
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
index bcbf9f2a29f9..bfaafdfc7eb2 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
@@ -17,7 +17,9 @@
* this program. If not, see <http://www.gnu.org/licenses/>.
*/
+#include
#include
+#include
#include "adreno_gpu.h"
#include "msm_gem.h"
#include "msm_mmu.h"
@@ -70,10 +72,12 @@ adreno_request_fw(struct adreno_gpu *adreno_gpu, const char
*fwname)
{
struct drm_device *drm = adreno_gpu->base.dev;
const struct firmware *fw = NULL;
- char newname[strlen("qcom/") + strlen(fwname) + 1];
+ char *newname;
int ret;
- sprintf(newname, "qcom/%s", fwname);
+ newname = kasprintf(GFP_KERNEL, "qcom/%s", fwname);
+ if (!newname)
+ return ERR_PTR(-ENOMEM);
/*
* Try first to load from qcom/$fwfile using a direct load (to avoid
@@ -87,11 +91,12 @@ adreno_request_fw(struct adreno_gpu *adreno_gpu, const char
*fwname)
dev_info(drm->dev, "loaded %s from new location\n",
newname);
adreno_gpu->fwloc = FW_LOCATION_NEW;
- return fw;
+ goto out;
} else if (adreno_gpu->fwloc != FW_LOCATION_UNKNOWN) {
dev_err(drm->dev, "failed to load %s: %d\n",
newname, ret);
- return ERR_PTR(ret);
+ fw = ERR_PTR(ret);
+ goto out;
}
}
@@ -106,11 +111,12 @@ adreno_request_fw(struct adreno_gpu *adreno_gpu, const
char *fwname)
dev_info(drm->dev, "loaded %s from legacy location\n",
newname);
adreno_gpu->fwloc = FW_LOCATION_LEGACY;
- return fw;
+ goto out;
} else if (adreno_gpu->fwloc != FW_LOCATION_UNKNOWN) {
dev_err(drm->dev, "failed to load %s: %d\n",
fwname, ret);
- return ERR_PTR(ret);
+ fw = ERR_PTR(ret);
+ goto out;
}
}
@@ -126,16 +132,20 @@ adreno_request_fw(struct adreno_gpu *adreno_gpu, const
char *fwname)
dev_info(drm->dev, "loaded %s with helper\n",
newname);
adreno_gpu->fwloc = FW_LOCATION_HELPER;
- return fw;
+ goto out;
} else if (adreno_gpu->fwloc != FW_LOCATION_UNKNOWN) {
dev_err(drm->dev, "failed to load %s: %d\n",
newname, ret);
- return ERR_PTR(ret);
+ fw = ERR_PTR(ret);
+ goto out;
}
}
dev_err(drm->dev, "failed to load %s\n", fwname);
- return ERR_PTR(-ENOENT);
+ fw = ERR_PTR(-ENOENT);
+out:
+ kfree(newname);
+ return fw;
