From: Alex Deucher <alexdeucher@gmail.com>
To: "Christian König" <christian.koenig@amd.com>
Cc: Erick Archer <erick.archer@outlook.com>,
Alex Deucher <alexander.deucher@amd.com>,
"Pan, Xinhui" <Xinhui.Pan@amd.com>,
David Airlie <airlied@gmail.com>, Daniel Vetter <daniel@ffwll.ch>,
"Gustavo A. R. Silva" <gustavoars@kernel.org>,
Kees Cook <keescook@chromium.org>,
amd-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
linux-kernel@vger.kernel.org, linux-hardening@vger.kernel.org
Subject: Re: [PATCH v2] drm/radeon/radeon_display: Decrease the size of allocated memory
Date: Mon, 8 Apr 2024 16:05:43 -0400 [thread overview]
Message-ID: <CADnq5_OsJ+VxY9z_iqOscvWAoRqK2hZ0thUOHtBzhfo3De4EoA@mail.gmail.com> (raw)
In-Reply-To: <c2182ac1-368c-441b-b6ed-b5d15a8f9f38@amd.com>
On Mon, Apr 1, 2024 at 8:35 AM Christian König <christian.koenig@amd.com> wrote:
>
> Am 30.03.24 um 17:34 schrieb Erick Archer:
> > This is an effort to get rid of all multiplications from allocation
> > functions in order to prevent integer overflows [1] [2].
> >
> > In this case, the memory allocated to store RADEONFB_CONN_LIMIT pointers
> > to "drm_connector" structures can be avoided. This is because this
> > memory area is never accessed.
> >
> > Also, in the kzalloc function, it is preferred to use sizeof(*pointer)
> > instead of sizeof(type) due to the type of the variable can change and
> > one needs not change the former (unlike the latter).
> >
> > At the same time take advantage to remove the "#if 0" block, the code
> > where the removed memory area was accessed, and the RADEONFB_CONN_LIMIT
> > constant due to now is never used.
> >
> > Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#open-coded-arithmetic-in-allocator-arguments [1]
> > Link: https://github.com/KSPP/linux/issues/160 [2]
> > Signed-off-by: Erick Archer <erick.archer@outlook.com>
>
> Well in general we don't do any new feature development any more for the
> radeon driver.
>
> But this cleanup looks so straight forward that the risk of breaking
> something is probably very low.
>
> Acked-by from my side, but Alex should probably take a look as well.
I can't remember why that was done that way. Maybe some leftover from
the early KMS days before we finalized the fbdev interactions?
Anyway, patch applied. Thanks.
Alex
>
> Regards,
> Christian.
>
> > ---
> > Changes in v2:
> > - Rebase against linux-next.
> >
> > Previous versions:
> > v1 -> https://lore.kernel.org/linux-hardening/20240222180431.7451-1-erick.archer@gmx.com/
> >
> > Hi everyone,
> >
> > Any comments would be greatly appreciated. The first version was
> > not commented.
> >
> > Thanks,
> > Erick
> > ---
> > drivers/gpu/drm/radeon/radeon.h | 1 -
> > drivers/gpu/drm/radeon/radeon_display.c | 8 +-------
> > 2 files changed, 1 insertion(+), 8 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
> > index 3e5ff17e3caf..0999c8eaae94 100644
> > --- a/drivers/gpu/drm/radeon/radeon.h
> > +++ b/drivers/gpu/drm/radeon/radeon.h
> > @@ -132,7 +132,6 @@ extern int radeon_cik_support;
> > /* RADEON_IB_POOL_SIZE must be a power of 2 */
> > #define RADEON_IB_POOL_SIZE 16
> > #define RADEON_DEBUGFS_MAX_COMPONENTS 32
> > -#define RADEONFB_CONN_LIMIT 4
> > #define RADEON_BIOS_NUM_SCRATCH 8
> >
> > /* internal ring indices */
> > diff --git a/drivers/gpu/drm/radeon/radeon_display.c b/drivers/gpu/drm/radeon/radeon_display.c
> > index efd18c8d84c8..5f1d24d3120c 100644
> > --- a/drivers/gpu/drm/radeon/radeon_display.c
> > +++ b/drivers/gpu/drm/radeon/radeon_display.c
> > @@ -683,7 +683,7 @@ static void radeon_crtc_init(struct drm_device *dev, int index)
> > struct radeon_device *rdev = dev->dev_private;
> > struct radeon_crtc *radeon_crtc;
> >
> > - radeon_crtc = kzalloc(sizeof(struct radeon_crtc) + (RADEONFB_CONN_LIMIT * sizeof(struct drm_connector *)), GFP_KERNEL);
> > + radeon_crtc = kzalloc(sizeof(*radeon_crtc), GFP_KERNEL);
> > if (radeon_crtc == NULL)
> > return;
> >
> > @@ -709,12 +709,6 @@ static void radeon_crtc_init(struct drm_device *dev, int index)
> > dev->mode_config.cursor_width = radeon_crtc->max_cursor_width;
> > dev->mode_config.cursor_height = radeon_crtc->max_cursor_height;
> >
> > -#if 0
> > - radeon_crtc->mode_set.crtc = &radeon_crtc->base;
> > - radeon_crtc->mode_set.connectors = (struct drm_connector **)(radeon_crtc + 1);
> > - radeon_crtc->mode_set.num_connectors = 0;
> > -#endif
> > -
> > if (rdev->is_atom_bios && (ASIC_IS_AVIVO(rdev) || radeon_r4xx_atom))
> > radeon_atombios_init_crtc(dev, radeon_crtc);
> > else
>
prev parent reply other threads:[~2024-04-08 20:05 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-30 16:34 [PATCH v2] drm/radeon/radeon_display: Decrease the size of allocated memory Erick Archer
2024-04-01 12:35 ` Christian König
2024-04-08 20:05 ` Alex Deucher [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CADnq5_OsJ+VxY9z_iqOscvWAoRqK2hZ0thUOHtBzhfo3De4EoA@mail.gmail.com \
--to=alexdeucher@gmail.com \
--cc=Xinhui.Pan@amd.com \
--cc=airlied@gmail.com \
--cc=alexander.deucher@amd.com \
--cc=amd-gfx@lists.freedesktop.org \
--cc=christian.koenig@amd.com \
--cc=daniel@ffwll.ch \
--cc=dri-devel@lists.freedesktop.org \
--cc=erick.archer@outlook.com \
--cc=gustavoars@kernel.org \
--cc=keescook@chromium.org \
--cc=linux-hardening@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).