On Sat, 08 May 2021, Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> 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/?id=fec4d42724a1bf3dcba52307e55375fdb967b852 > > 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. > > Linus > > On Wed, Apr 28, 2021 at 12:27 AM Jani Nikula <jani.nikula@xxxxxxxxx> wrote: >> >> On Tue, 27 Apr 2021, Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> 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. -- Jani Nikula, Intel Open Source Graphics Center
