Re: [PATCH 0/9] drm: Annotate structs with __counted_by

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
<ckoenig.leichtzumerken@xxxxxxxxx> 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.

Regards,
Christian.


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,





[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux