Alex Deucher
2023-Oct-02 15:06 UTC
[Nouveau] [PATCH 0/9] drm: Annotate structs with __counted_by
On Mon, Oct 2, 2023 at 5:20?AM Christian K?nig <ckoenig.leichtzumerken at gmail.com> 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. > > 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. Alex> > 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 :) > > 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
2023-Oct-02 16:53 UTC
[Nouveau] [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 > <ckoenig.leichtzumerken at gmail.com> 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