Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754580AbbBZQBA (ORCPT ); Thu, 26 Feb 2015 11:01:00 -0500 Received: from userp1040.oracle.com ([156.151.31.81]:49902 "EHLO userp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754554AbbBZQA6 (ORCPT ); Thu, 26 Feb 2015 11:00:58 -0500 Date: Thu, 26 Feb 2015 17:03:00 +0100 From: Quentin Casasnovas To: David Airlie Cc: linux-kernel@vger.kernel.org, Quentin Casasnovas Subject: Re: [PATCH] i915: stack address leak when failing to read registers. Message-ID: <20150226160300.GE30434@chrystal.uk.oracle.com> References: <1422885516-533-1-git-send-email-quentin.casasnovas@oracle.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1422885516-533-1-git-send-email-quentin.casasnovas@oracle.com> User-Agent: Mutt/1.5.22 (2013-10-16) X-Source-IP: aserv0022.oracle.com [141.146.126.234] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4175 Lines: 109 (Removing stable from CC...) Ping on this? On Mon, Feb 02, 2015 at 02:58:36PM +0100, Quentin Casasnovas wrote: > It is possible for the *_read*() functions to fail, in which case it'll > leave its third argument untouched. Most of the code do not check the > return value of *_read*() functions, and will happily use garbage from the > stack to test various things. For example, ch7xxx_dump_regs() will leak 1 > byte from the stack in case of failure, ivch_dump_regs() 2 bytes, etc. > > Instead of fixing every caller to check the return value, just clear the > output variable so this patch can easily go to -stable. A proper fix would > probably to check the return value everywhere though, since it does not > make much sense to carry on talking to the ship after some error. > > This issue was found by code review while preparing Ksplice updates. > > Signed-off-by: Quentin Casasnovas > --- > drivers/gpu/drm/i915/dvo_ch7017.c | 1 + > drivers/gpu/drm/i915/dvo_ch7xxx.c | 1 + > drivers/gpu/drm/i915/dvo_ivch.c | 1 + > drivers/gpu/drm/i915/dvo_ns2501.c | 2 +- > drivers/gpu/drm/i915/dvo_sil164.c | 1 + > drivers/gpu/drm/i915/dvo_tfp410.c | 1 + > 6 files changed, 6 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/dvo_ch7017.c b/drivers/gpu/drm/i915/dvo_ch7017.c > index 86b27d1..826ac6c 100644 > --- a/drivers/gpu/drm/i915/dvo_ch7017.c > +++ b/drivers/gpu/drm/i915/dvo_ch7017.c > @@ -181,6 +181,7 @@ static bool ch7017_read(struct intel_dvo_device *dvo, u8 addr, u8 *val) > .buf = val, > } > }; > + *val = 0; > return i2c_transfer(dvo->i2c_bus, msgs, 2) == 2; > } > > diff --git a/drivers/gpu/drm/i915/dvo_ch7xxx.c b/drivers/gpu/drm/i915/dvo_ch7xxx.c > index 80449f4..4ebbeef 100644 > --- a/drivers/gpu/drm/i915/dvo_ch7xxx.c > +++ b/drivers/gpu/drm/i915/dvo_ch7xxx.c > @@ -166,6 +166,7 @@ static bool ch7xxx_readb(struct intel_dvo_device *dvo, int addr, uint8_t *ch) > DRM_DEBUG_KMS("Unable to read register 0x%02x from %s:%02x.\n", > addr, adapter->name, dvo->slave_addr); > } > + *ch = 0; > return false; > } > > diff --git a/drivers/gpu/drm/i915/dvo_ivch.c b/drivers/gpu/drm/i915/dvo_ivch.c > index 0f2587f..43604f0a 100644 > --- a/drivers/gpu/drm/i915/dvo_ivch.c > +++ b/drivers/gpu/drm/i915/dvo_ivch.c > @@ -202,6 +202,7 @@ static bool ivch_read(struct intel_dvo_device *dvo, int addr, uint16_t *data) > "%s:%02x.\n", > addr, adapter->name, dvo->slave_addr); > } > + *data = 0; > return false; > } > > diff --git a/drivers/gpu/drm/i915/dvo_ns2501.c b/drivers/gpu/drm/i915/dvo_ns2501.c > index 44163043..b314f64 100644 > --- a/drivers/gpu/drm/i915/dvo_ns2501.c > +++ b/drivers/gpu/drm/i915/dvo_ns2501.c > @@ -409,7 +409,7 @@ static bool ns2501_readb(struct intel_dvo_device *dvo, int addr, uint8_t * ch) > ("Unable to read register 0x%02x from %s:0x%02x.\n", addr, > adapter->name, dvo->slave_addr); > } > - > + *ch = 0; > return false; > } > > diff --git a/drivers/gpu/drm/i915/dvo_sil164.c b/drivers/gpu/drm/i915/dvo_sil164.c > index fa01149..7d5a440 100644 > --- a/drivers/gpu/drm/i915/dvo_sil164.c > +++ b/drivers/gpu/drm/i915/dvo_sil164.c > @@ -99,6 +99,7 @@ static bool sil164_readb(struct intel_dvo_device *dvo, int addr, uint8_t *ch) > DRM_DEBUG_KMS("Unable to read register 0x%02x from %s:%02x.\n", > addr, adapter->name, dvo->slave_addr); > } > + *ch = 0; > return false; > } > > diff --git a/drivers/gpu/drm/i915/dvo_tfp410.c b/drivers/gpu/drm/i915/dvo_tfp410.c > index 7853719..2b66881 100644 > --- a/drivers/gpu/drm/i915/dvo_tfp410.c > +++ b/drivers/gpu/drm/i915/dvo_tfp410.c > @@ -124,6 +124,7 @@ static bool tfp410_readb(struct intel_dvo_device *dvo, int addr, uint8_t *ch) > DRM_DEBUG_KMS("Unable to read register 0x%02x from %s:%02x.\n", > addr, adapter->name, dvo->slave_addr); > } > + *ch = 0; > return false; > } > > -- > 2.0.5 > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/