| Message ID | 20221010060512.3064524-1-linux@roeck-us.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/amd/display: fix array-bounds error in dc_stream_remove_writeback() [take 2] | expand |
Applied. Thanks. Alex On Mon, Oct 10, 2022 at 2:05 AM Guenter Roeck <linux@roeck-us.net> wrote: > > Commit 5d8c3e836fc2 ("drm/amd/display: fix array-bounds error in > dc_stream_remove_writeback()") tried to fix an array bounds error seen > with gcc 12.0. Unfortunately, that results in another array bounds error, > seen with older versions of gcc. > > Building csky:allmodconfig ... failed > -------------- > Error log: > drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_stream.c: > In function 'dc_stream_remove_writeback': > drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_stream.c:527:83: > error: array subscript 1 is above array bounds of 'struct dc_writeback_info[1]' [-Werror=array-bounds] > 527 | stream->writeback_info[j] = stream->writeback_info[i]; > | ~~~~~~~~~~~~~~~~~~~~~~^~~ > In file included from drivers/gpu/drm/amd/amdgpu/../display/dc/dc.h:1269, > from drivers/gpu/drm/amd/amdgpu/../display/dc/inc/core_types.h:29, > from drivers/gpu/drm/amd/amdgpu/../display/dc/basics/dc_common.h:29, > from drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_stream.c:27: > drivers/gpu/drm/amd/amdgpu/../display/dc/dc_stream.h:241:34: note: while referencing 'writeback_info' > 241 | struct dc_writeback_info writeback_info[MAX_DWB_PIPES]; > > We could check both i and j for overflow to fix the problem. That would, > however, be not make much sense since it is known and provable that j <= i. > Also, the check introduced with commit 5d8c3e836fc2 does not really add > value since it checks if j < MAX_DWB_PIPES. Since it is known that j <= i, > it would make more sense to check if i < MAX_DWB_PIPES. Unfortunately, that > does not help to solve the problem observed here: gcc still complains. > > To solve the problem, replace the subsequent check for 'i != j' with > 'j < i'. This is identical to the original check since we know that j <= i, > and it makes all versions of gcc happy. Drop the check introduced with > commit 5d8c3e836fc2 since it is not really useful and does not solve the > problem. > > Cc: Aurabindo Pillai <aurabindo.pillai@amd.com> > Cc: Hamza Mahfooz <hamza.mahfooz@amd.com> > Fixes: 5d8c3e836fc2 ("drm/amd/display: fix array-bounds error in dc_stream_remove_writeback()") > Signed-off-by: Guenter Roeck <linux@roeck-us.net> > --- > drivers/gpu/drm/amd/display/dc/core/dc_stream.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_stream.c b/drivers/gpu/drm/amd/display/dc/core/dc_stream.c > index ae13887756bf..9f568d618cd0 100644 > --- a/drivers/gpu/drm/amd/display/dc/core/dc_stream.c > +++ b/drivers/gpu/drm/amd/display/dc/core/dc_stream.c > @@ -520,9 +520,9 @@ bool dc_stream_remove_writeback(struct dc *dc, > } > > /* remove writeback info for disabled writeback pipes from stream */ > - for (i = 0, j = 0; i < stream->num_wb_info && j < MAX_DWB_PIPES; i++) { > + for (i = 0, j = 0; i < stream->num_wb_info; i++) { > if (stream->writeback_info[i].wb_enabled) { > - if (i != j) > + if (j < i) > /* trim the array */ > stream->writeback_info[j] = stream->writeback_info[i]; > j++; > -- > 2.36.2 >
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_stream.c b/drivers/gpu/drm/amd/display/dc/core/dc_stream.c index ae13887756bf..9f568d618cd0 100644 --- a/drivers/gpu/drm/amd/display/dc/core/dc_stream.c +++ b/drivers/gpu/drm/amd/display/dc/core/dc_stream.c @@ -520,9 +520,9 @@ bool dc_stream_remove_writeback(struct dc *dc, } /* remove writeback info for disabled writeback pipes from stream */ - for (i = 0, j = 0; i < stream->num_wb_info && j < MAX_DWB_PIPES; i++) { + for (i = 0, j = 0; i < stream->num_wb_info; i++) { if (stream->writeback_info[i].wb_enabled) { - if (i != j) + if (j < i) /* trim the array */ stream->writeback_info[j] = stream->writeback_info[i]; j++;
Commit 5d8c3e836fc2 ("drm/amd/display: fix array-bounds error in dc_stream_remove_writeback()") tried to fix an array bounds error seen with gcc 12.0. Unfortunately, that results in another array bounds error, seen with older versions of gcc. Building csky:allmodconfig ... failed -------------- Error log: drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_stream.c: In function 'dc_stream_remove_writeback': drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_stream.c:527:83: error: array subscript 1 is above array bounds of 'struct dc_writeback_info[1]' [-Werror=array-bounds] 527 | stream->writeback_info[j] = stream->writeback_info[i]; | ~~~~~~~~~~~~~~~~~~~~~~^~~ In file included from drivers/gpu/drm/amd/amdgpu/../display/dc/dc.h:1269, from drivers/gpu/drm/amd/amdgpu/../display/dc/inc/core_types.h:29, from drivers/gpu/drm/amd/amdgpu/../display/dc/basics/dc_common.h:29, from drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_stream.c:27: drivers/gpu/drm/amd/amdgpu/../display/dc/dc_stream.h:241:34: note: while referencing 'writeback_info' 241 | struct dc_writeback_info writeback_info[MAX_DWB_PIPES]; We could check both i and j for overflow to fix the problem. That would, however, be not make much sense since it is known and provable that j <= i. Also, the check introduced with commit 5d8c3e836fc2 does not really add value since it checks if j < MAX_DWB_PIPES. Since it is known that j <= i, it would make more sense to check if i < MAX_DWB_PIPES. Unfortunately, that does not help to solve the problem observed here: gcc still complains. To solve the problem, replace the subsequent check for 'i != j' with 'j < i'. This is identical to the original check since we know that j <= i, and it makes all versions of gcc happy. Drop the check introduced with commit 5d8c3e836fc2 since it is not really useful and does not solve the problem. Cc: Aurabindo Pillai <aurabindo.pillai@amd.com> Cc: Hamza Mahfooz <hamza.mahfooz@amd.com> Fixes: 5d8c3e836fc2 ("drm/amd/display: fix array-bounds error in dc_stream_remove_writeback()") Signed-off-by: Guenter Roeck <linux@roeck-us.net> --- drivers/gpu/drm/amd/display/dc/core/dc_stream.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)