linux-kernel - RE: [PATCH] drm/radeon: remove code that can never get executed

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 13 Oct 2014 18:44:22 +0200 (CEST)
From:	Henning Schild <henning@...nsch.de>
To:	"Deucher, Alexander" <alexander.deucher@....com>,
	linux-kernel@...r.kernel.org
Cc:	Rafał Miłecki <zajec5@...il.com>
Subject: RE: [PATCH] drm/radeon: remove code that can never get executed

Well i am not sure what the code actually is supposed to do. That is up to you
because you know the hardware. I just wanted to point out that right now some
code can never be reached.

I ran into the whole thing because the pointer (sadp) that gets kfreed was never
initialized (in one of the three cases). My first fix was to initialize it to
NULL, that way the kfree did not fail even if the allocation did not do
anything. Then i found the other two copies and decided to follow their scheme.

In your patch you have to make kfree depend on (sad_count > 0) or sadp needs to
be initialized with NULL. Otherwise you will get the dangling pointer kfree that
lead me to read this code.

Henning

> On October 13, 2014 at 6:08 PM "Deucher, Alexander"
> <Alexander.Deucher@....com> wrote:
> 
> > -----Original Message-----
> > From: Henning Schild [mailto:henning@...nsch.de]
> > Sent: Saturday, October 11, 2014 8:51 AM
> > To: linux-kernel@...r.kernel.org
> > Cc: Henning Schild; Deucher, Alexander; Rafał Miłecki
> > Subject: [PATCH] drm/radeon: remove code that can never get executed
> > 
> > Removing a code-path that can never be executed ... and its copies. If
> > drm_edid_to_speaker_allocation returns 0 the callers return. There is no
> > need to check that condition again.
> 
> I think we actually want to set the speaker allocation setup to stereo if the
> speaker allocation block is not present so I think the attached patch is
> probably the proper fix.
> 
> Alex
> 
> > 
> > Signed-off-by: Henning Schild <henning@...nsch.de>
> > ---
> > drivers/gpu/drm/radeon/dce3_1_afmt.c | 5 +----
> > drivers/gpu/drm/radeon/dce6_afmt.c | 5 +----
> > drivers/gpu/drm/radeon/evergreen_hdmi.c | 5 +----
> > 3 files changed, 3 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/radeon/dce3_1_afmt.c
> > b/drivers/gpu/drm/radeon/dce3_1_afmt.c
> > index cb76074..6d31ed8 100644
> > --- a/drivers/gpu/drm/radeon/dce3_1_afmt.c
> > +++ b/drivers/gpu/drm/radeon/dce3_1_afmt.c
> > @@ -58,10 +58,7 @@ static void
> > dce3_2_afmt_write_speaker_allocation(struct drm_encoder *encoder)
> > tmp &= ~(DP_CONNECTION | SPEAKER_ALLOCATION_MASK);
> > /* set HDMI mode */
> > tmp |= HDMI_CONNECTION;
> > - if (sad_count)
> > - tmp |= SPEAKER_ALLOCATION(sadb[0]);
> > - else
> > - tmp |= SPEAKER_ALLOCATION(5); /* stereo */
> > + tmp |= SPEAKER_ALLOCATION(sadb[0]);
> > WREG32(AZ_F0_CODEC_PIN0_CONTROL_CHANNEL_SPEAKER, tmp);
> > 
> > kfree(sadb);
> > diff --git a/drivers/gpu/drm/radeon/dce6_afmt.c
> > b/drivers/gpu/drm/radeon/dce6_afmt.c
> > index ab29f95..e6b2750 100644
> > --- a/drivers/gpu/drm/radeon/dce6_afmt.c
> > +++ b/drivers/gpu/drm/radeon/dce6_afmt.c
> > @@ -186,10 +186,7 @@ void dce6_afmt_write_speaker_allocation(struct
> > drm_encoder *encoder)
> > tmp &= ~(DP_CONNECTION | SPEAKER_ALLOCATION_MASK);
> > /* set HDMI mode */
> > tmp |= HDMI_CONNECTION;
> > - if (sad_count)
> > - tmp |= SPEAKER_ALLOCATION(sadb[0]);
> > - else
> > - tmp |= SPEAKER_ALLOCATION(5); /* stereo */
> > + tmp |= SPEAKER_ALLOCATION(sadb[0]);
> > WREG32_ENDPOINT(offset,
> > AZ_F0_CODEC_PIN_CONTROL_CHANNEL_SPEAKER, tmp);
> > 
> > kfree(sadb);
> > diff --git a/drivers/gpu/drm/radeon/evergreen_hdmi.c
> > b/drivers/gpu/drm/radeon/evergreen_hdmi.c
> > index 278c7a1..11a6b65 100644
> > --- a/drivers/gpu/drm/radeon/evergreen_hdmi.c
> > +++ b/drivers/gpu/drm/radeon/evergreen_hdmi.c
> > @@ -128,10 +128,7 @@ static void
> > dce4_afmt_write_speaker_allocation(struct drm_encoder *encoder)
> > tmp &= ~(DP_CONNECTION | SPEAKER_ALLOCATION_MASK);
> > /* set HDMI mode */
> > tmp |= HDMI_CONNECTION;
> > - if (sad_count)
> > - tmp |= SPEAKER_ALLOCATION(sadb[0]);
> > - else
> > - tmp |= SPEAKER_ALLOCATION(5); /* stereo */
> > + tmp |= SPEAKER_ALLOCATION(sadb[0]);
> > WREG32(AZ_F0_CODEC_PIN0_CONTROL_CHANNEL_SPEAKER, tmp);
> > 
> > kfree(sadb);
> > --
> > 2.0.4

>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/