On Sat, 08 May 2021, Linus Torvalds torvalds@linux-foundation.org wrote:
I have heard nothing about this, and it remains the only warning from my allmodconfig build (I have another one for drm compiled with clang, but there I at least heard back that a fix exists).
Since I am going to release rc1 tomorrow, and I don't want to release it with an ugly compiler warning, I took it upon myself to just fix the code:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i...
HOWEVER.
That commit fixes the warning, and is at worst harmless. At best it fixes an access to random stack memory. But it does smell like somebody who actually knows how these arrays work should look at that code.
IOW, maybe the code should actually have read 16 bytes from the Event Status Indicator? Maybe offset 10 was wrong? Maybe drm_dp_channel_eq_ok() should never have taken six bytes to begin with?
It's a mystery, and I haven't heard anything otherwise, so there it is.
Fair enough. My bad for not getting this fixed.
The fix is harmless. drm_dp_channel_eq_ok() only ever accesses 3 bytes instead of 6. I figure the DP_LINK_STATUS_SIZE (=6) is there because in the normal case you'd read that much, and use a family of functions on that data, some of which do access the full 6 bytes, some don't.
In our case, we use drm_dp_channel_eq_ok() to check 3 bytes of similarly encoded data elsewhere in the DPCD address space, and the DP_LINK_STATUS_SIZE is meaningless there.
The straightforward fix would be to replace link_status[DP_LINK_STATUS_SIZE] with link_status[3], and that likely needs changes in dp_link_status() and dp_get_lane_status() as well.
BR, Jani.
LinusOn Wed, Apr 28, 2021 at 12:27 AM Jani Nikula jani.nikula@intel.com wrote:
On Tue, 27 Apr 2021, Linus Torvalds torvalds@linux-foundation.org wrote:
I've updated to Fedora 34 on one of my machines, and it causes a lot of i915 warnings like
drivers/gpu/drm/i915/intel_pm.c: In function ‘ilk_setup_wm_latency’: drivers/gpu/drm/i915/intel_pm.c:3059:9: note: referencing argument 3 of type ‘const u16 *’ {aka ‘const short unsigned int *’} drivers/gpu/drm/i915/intel_pm.c:2994:13: note: in a call to function ‘intel_print_wm_latency’
and the reason is that gcc now seems to look at the argument array size more, and notices that
Arnd Bergmann reported some of these a while back. I think we have some of them fixed in our -next already, but not all. Thanks for the reminder.