Re: [PATCH] i915: stack address leak when failing to read registers.

[Quentin Casasnovas <quentin.casasnovas@oracle.com>]

(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 <quentin.casasnovas@oracle.com>
> ---
>  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
>