From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sean Paul Subject: Re: [PATCH v3 06/24] drm/rockchip: dw-mipi-dsi: avoid out-of-bounds read on tx_buf Date: Tue, 31 Jan 2017 09:48:18 -0500 Message-ID: <20170131144818.GX20076@art_vandelay> References: <20170129132444.25251-1-john@metanate.com> <20170129132444.25251-7-john@metanate.com> <20170130180146.GG20076@art_vandelay> <20170130181636.1bc81e86.john@metanate.com> <20170130200955.GL20076@art_vandelay> <20170131114548.667d3911.john@metanate.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Content-Disposition: inline In-Reply-To: <20170131114548.667d3911.john@metanate.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: John Keeping Cc: Chris Zhong , linux-rockchip@lists.infradead.org, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, linux-arm-kernel@lists.infradead.org List-Id: linux-rockchip.vger.kernel.org T24gVHVlLCBKYW4gMzEsIDIwMTcgYXQgMTE6NDU6NDhBTSArMDAwMCwgSm9obiBLZWVwaW5nIHdy b3RlOgo+IE9uIE1vbiwgMzAgSmFuIDIwMTcgMTU6MDk6NTUgLTA1MDAsIFNlYW4gUGF1bCB3cm90 ZToKPiAKPiA+IE9uIE1vbiwgSmFuIDMwLCAyMDE3IGF0IDA2OjE2OjM2UE0gKzAwMDAsIEpvaG4g S2VlcGluZyB3cm90ZToKPiA+ID4gT24gTW9uLCAzMCBKYW4gMjAxNyAxMzowMTo0NiAtMDUwMCwg U2VhbiBQYXVsIHdyb3RlOgo+ID4gPiAgIAo+ID4gPiA+IE9uIFN1biwgSmFuIDI5LCAyMDE3IGF0 IDAxOjI0OjI2UE0gKzAwMDAsIEpvaG4gS2VlcGluZyB3cm90ZTogIAo+ID4gPiA+ID4gQXMgYSBz aWRlLWVmZmVjdCBvZiB0aGlzLCBlbmNvZGUgdGhlIGVuZGlhbm5lc3MgZXhwbGljaXRseSByYXRo ZXIgdGhhbgo+ID4gPiA+ID4gY2FzdGluZyBhIHUxNi4KPiA+ID4gPiA+IAo+ID4gPiA+ID4gU2ln bmVkLW9mZi1ieTogSm9obiBLZWVwaW5nIDxqb2huQG1ldGFuYXRlLmNvbT4KPiA+ID4gPiA+IFJl dmlld2VkLWJ5OiBDaHJpcyBaaG9uZyA8enl3QHJvY2stY2hpcHMuY29tPgo+ID4gPiA+ID4gLS0t Cj4gPiA+ID4gPiB2MzoKPiA+ID4gPiA+IC0gQWRkIENocmlzJyBSZXZpZXdlZC1ieQo+ID4gPiA+ ID4gVW5jaGFuZ2VkIGluIHYyCj4gPiA+ID4gPiAKPiA+ID4gPiA+ICBkcml2ZXJzL2dwdS9kcm0v cm9ja2NoaXAvZHctbWlwaS1kc2kuYyB8IDkgKysrKysrKy0tCj4gPiA+ID4gPiAgMSBmaWxlIGNo YW5nZWQsIDcgaW5zZXJ0aW9ucygrKSwgMiBkZWxldGlvbnMoLSkKPiA+ID4gPiA+IAo+ID4gPiA+ ID4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvZ3B1L2RybS9yb2NrY2hpcC9kdy1taXBpLWRzaS5jIGIv ZHJpdmVycy9ncHUvZHJtL3JvY2tjaGlwL2R3LW1pcGktZHNpLmMKPiA+ID4gPiA+IGluZGV4IDRi ZTFmZjNhNDJiYi4uMmU2YWQ0NTkxZWJmIDEwMDY0NAo+ID4gPiA+ID4gLS0tIGEvZHJpdmVycy9n cHUvZHJtL3JvY2tjaGlwL2R3LW1pcGktZHNpLmMKPiA+ID4gPiA+ICsrKyBiL2RyaXZlcnMvZ3B1 L2RybS9yb2NrY2hpcC9kdy1taXBpLWRzaS5jCj4gPiA+ID4gPiBAQCAtNTcyLDggKzU3MiwxMyBA QCBzdGF0aWMgaW50IGR3X21pcGlfZHNpX2dlbl9wa3RfaGRyX3dyaXRlKHN0cnVjdCBkd19taXBp X2RzaSAqZHNpLCB1MzIgaGRyX3ZhbCkKPiA+ID4gPiA+ICBzdGF0aWMgaW50IGR3X21pcGlfZHNp X2Rjc19zaG9ydF93cml0ZShzdHJ1Y3QgZHdfbWlwaV9kc2kgKmRzaSwKPiA+ID4gPiA+ICAJCQkJ ICAgICAgIGNvbnN0IHN0cnVjdCBtaXBpX2RzaV9tc2cgKm1zZykKPiA+ID4gPiA+ICB7Cj4gPiA+ ID4gPiAtCWNvbnN0IHUxNiAqdHhfYnVmID0gbXNnLT50eF9idWY7Cj4gPiA+ID4gPiAtCXUzMiB2 YWwgPSBHRU5fSERBVEEoKnR4X2J1ZikgfCBHRU5fSFRZUEUobXNnLT50eXBlKTsKPiA+ID4gPiA+ ICsJY29uc3QgdTggKnR4X2J1ZiA9IG1zZy0+dHhfYnVmOwo+ID4gPiA+ID4gKwl1MzIgdmFsID0g R0VOX0hUWVBFKG1zZy0+dHlwZSk7Cj4gPiA+ID4gPiArCj4gPiA+ID4gPiArCWlmIChtc2ctPnR4 X2xlbiA+IDApCj4gPiA+ID4gPiArCQl2YWwgfD0gR0VOX0hEQVRBKHR4X2J1ZlswXSk7Cj4gPiA+ ID4gPiArCWlmIChtc2ctPnR4X2xlbiA+IDEpCj4gPiA+ID4gPiArCQl2YWwgfD0gR0VOX0hEQVRB KHR4X2J1ZlsxXSA8PCA4KTsgICAgCj4gPiA+ID4gCj4gPiA+ID4gWW91IHNob3VsZCBwcm9iYWJs eSB1cGRhdGUgdGhlIG1hc2sgaW5zaWRlIEdFTl9IREFUQSB0byBtYXNrIG9mZiA4IGJpdHMgaW5z dGVhZCBvZgo+ID4gPiA+IDE2LiAgCj4gPiA+IAo+ID4gPiBXb24ndCB0aGF0IG1hc2sgb2ZmIHRo ZSBkYXRhIHdyaXR0ZW4gYnkgInR4X2J1ZlsxXSA8PCA4Ij8gIAo+ID4gCj4gPiBJIHdvdWxkIG1v dmUgdGhlIHNoaWZ0IG91dHNpZGUgdGhlIG1hc2ssIGllOgo+ID4gCj4gPiB2YWwgfD0gR0VOX0hE QVRBKHR4X2J1ZlsxXSkgPDwgODsKPiAKPiBJIGNhbiBkbyB0aGF0LCBidXQgdGhhdCBkb2Vzbid0 IHNlZW0gdG8gbWF0Y2ggdGhlIGludGVudGlvbiBvZiB0aGUKPiBtYWNyb3Mgd2hpY2ggYXJlIGFi b3V0IGVuY29kaW5nIHRoZSBwbGFjZW1lbnQgYW5kIHNpemUgb2YgZmllbGRzIHdpdGhpbgo+IHJl Z2lzdGVycy4KPiAKPiBNYXliZSBpdCB3b3VsZCBiZSBjbGVhcmVyIHRvIGRvOgo+IAo+ICAgICB1 MTYgZGF0YSA9IDA7Cj4gCj4gICAgIGlmIChtc2ctPnR4X2xlbiA+IDApCj4gICAgICAgICBkYXRh IHw9IHR4X2J1ZlswXTsKPiAgICAgaWYgKG1zZy0+dHhfbGVuID4gMSkKPiAgICAgICAgIGRhdGEg fD0gdHhfYnVmWzFdOwoKWWVwLCB0aGlzIGxvb2tzIGdvb2QgdG8gbWUsIGJ1dCBJIHRoaW5rIHlv dSBuZWVkOgoKZGF0YSB8PSB0eF9idWZbMV0gPDwgODsKClNlYW4KCj4gCj4gICAgIHZhbCA9IEdF Tl9IREFUQShkYXRhKSB8IEdFTl9IVFlQRShtc2ctPnR5cGUpOwo+IAo+ID8KPiBfX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwo+IGRyaS1kZXZlbCBtYWlsaW5n IGxpc3QKPiBkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCj4gaHR0cHM6Ly9saXN0cy5m cmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9kcmktZGV2ZWwKCi0tIApTZWFuIFBhdWws IFNvZnR3YXJlIEVuZ2luZWVyLCBHb29nbGUgLyBDaHJvbWl1bSBPUwpfX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwpkcmktZGV2ZWwgbWFpbGluZyBsaXN0CmRy aS1kZXZlbEBsaXN0cy5mcmVlZGVza3RvcC5vcmcKaHR0cHM6Ly9saXN0cy5mcmVlZGVza3RvcC5v cmcvbWFpbG1hbi9saXN0aW5mby9kcmktZGV2ZWwK From mboxrd@z Thu Jan 1 00:00:00 1970 From: seanpaul@chromium.org (Sean Paul) Date: Tue, 31 Jan 2017 09:48:18 -0500 Subject: [PATCH v3 06/24] drm/rockchip: dw-mipi-dsi: avoid out-of-bounds read on tx_buf In-Reply-To: <20170131114548.667d3911.john@metanate.com> References: <20170129132444.25251-1-john@metanate.com> <20170129132444.25251-7-john@metanate.com> <20170130180146.GG20076@art_vandelay> <20170130181636.1bc81e86.john@metanate.com> <20170130200955.GL20076@art_vandelay> <20170131114548.667d3911.john@metanate.com> Message-ID: <20170131144818.GX20076@art_vandelay> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Tue, Jan 31, 2017 at 11:45:48AM +0000, John Keeping wrote: > On Mon, 30 Jan 2017 15:09:55 -0500, Sean Paul wrote: > > > On Mon, Jan 30, 2017 at 06:16:36PM +0000, John Keeping wrote: > > > On Mon, 30 Jan 2017 13:01:46 -0500, Sean Paul wrote: > > > > > > > On Sun, Jan 29, 2017 at 01:24:26PM +0000, John Keeping wrote: > > > > > As a side-effect of this, encode the endianness explicitly rather than > > > > > casting a u16. > > > > > > > > > > Signed-off-by: John Keeping > > > > > Reviewed-by: Chris Zhong > > > > > --- > > > > > v3: > > > > > - Add Chris' Reviewed-by > > > > > Unchanged in v2 > > > > > > > > > > drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 9 +++++++-- > > > > > 1 file changed, 7 insertions(+), 2 deletions(-) > > > > > > > > > > diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c > > > > > index 4be1ff3a42bb..2e6ad4591ebf 100644 > > > > > --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c > > > > > +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c > > > > > @@ -572,8 +572,13 @@ static int dw_mipi_dsi_gen_pkt_hdr_write(struct dw_mipi_dsi *dsi, u32 hdr_val) > > > > > static int dw_mipi_dsi_dcs_short_write(struct dw_mipi_dsi *dsi, > > > > > const struct mipi_dsi_msg *msg) > > > > > { > > > > > - const u16 *tx_buf = msg->tx_buf; > > > > > - u32 val = GEN_HDATA(*tx_buf) | GEN_HTYPE(msg->type); > > > > > + const u8 *tx_buf = msg->tx_buf; > > > > > + u32 val = GEN_HTYPE(msg->type); > > > > > + > > > > > + if (msg->tx_len > 0) > > > > > + val |= GEN_HDATA(tx_buf[0]); > > > > > + if (msg->tx_len > 1) > > > > > + val |= GEN_HDATA(tx_buf[1] << 8); > > > > > > > > You should probably update the mask inside GEN_HDATA to mask off 8 bits instead of > > > > 16. > > > > > > Won't that mask off the data written by "tx_buf[1] << 8"? > > > > I would move the shift outside the mask, ie: > > > > val |= GEN_HDATA(tx_buf[1]) << 8; > > I can do that, but that doesn't seem to match the intention of the > macros which are about encoding the placement and size of fields within > registers. > > Maybe it would be clearer to do: > > u16 data = 0; > > if (msg->tx_len > 0) > data |= tx_buf[0]; > if (msg->tx_len > 1) > data |= tx_buf[1]; Yep, this looks good to me, but I think you need: data |= tx_buf[1] << 8; Sean > > val = GEN_HDATA(data) | GEN_HTYPE(msg->type); > > ? > _______________________________________________ > dri-devel mailing list > dri-devel at lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Sean Paul, Software Engineer, Google / Chromium OS From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751157AbdAaOsb (ORCPT ); Tue, 31 Jan 2017 09:48:31 -0500 Received: from mail-qk0-f177.google.com ([209.85.220.177]:33223 "EHLO mail-qk0-f177.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750822AbdAaOsV (ORCPT ); Tue, 31 Jan 2017 09:48:21 -0500 Date: Tue, 31 Jan 2017 09:48:18 -0500 From: Sean Paul To: John Keeping Cc: Chris Zhong , linux-rockchip@lists.infradead.org, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v3 06/24] drm/rockchip: dw-mipi-dsi: avoid out-of-bounds read on tx_buf Message-ID: <20170131144818.GX20076@art_vandelay> References: <20170129132444.25251-1-john@metanate.com> <20170129132444.25251-7-john@metanate.com> <20170130180146.GG20076@art_vandelay> <20170130181636.1bc81e86.john@metanate.com> <20170130200955.GL20076@art_vandelay> <20170131114548.667d3911.john@metanate.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170131114548.667d3911.john@metanate.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Jan 31, 2017 at 11:45:48AM +0000, John Keeping wrote: > On Mon, 30 Jan 2017 15:09:55 -0500, Sean Paul wrote: > > > On Mon, Jan 30, 2017 at 06:16:36PM +0000, John Keeping wrote: > > > On Mon, 30 Jan 2017 13:01:46 -0500, Sean Paul wrote: > > > > > > > On Sun, Jan 29, 2017 at 01:24:26PM +0000, John Keeping wrote: > > > > > As a side-effect of this, encode the endianness explicitly rather than > > > > > casting a u16. > > > > > > > > > > Signed-off-by: John Keeping > > > > > Reviewed-by: Chris Zhong > > > > > --- > > > > > v3: > > > > > - Add Chris' Reviewed-by > > > > > Unchanged in v2 > > > > > > > > > > drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 9 +++++++-- > > > > > 1 file changed, 7 insertions(+), 2 deletions(-) > > > > > > > > > > diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c > > > > > index 4be1ff3a42bb..2e6ad4591ebf 100644 > > > > > --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c > > > > > +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c > > > > > @@ -572,8 +572,13 @@ static int dw_mipi_dsi_gen_pkt_hdr_write(struct dw_mipi_dsi *dsi, u32 hdr_val) > > > > > static int dw_mipi_dsi_dcs_short_write(struct dw_mipi_dsi *dsi, > > > > > const struct mipi_dsi_msg *msg) > > > > > { > > > > > - const u16 *tx_buf = msg->tx_buf; > > > > > - u32 val = GEN_HDATA(*tx_buf) | GEN_HTYPE(msg->type); > > > > > + const u8 *tx_buf = msg->tx_buf; > > > > > + u32 val = GEN_HTYPE(msg->type); > > > > > + > > > > > + if (msg->tx_len > 0) > > > > > + val |= GEN_HDATA(tx_buf[0]); > > > > > + if (msg->tx_len > 1) > > > > > + val |= GEN_HDATA(tx_buf[1] << 8); > > > > > > > > You should probably update the mask inside GEN_HDATA to mask off 8 bits instead of > > > > 16. > > > > > > Won't that mask off the data written by "tx_buf[1] << 8"? > > > > I would move the shift outside the mask, ie: > > > > val |= GEN_HDATA(tx_buf[1]) << 8; > > I can do that, but that doesn't seem to match the intention of the > macros which are about encoding the placement and size of fields within > registers. > > Maybe it would be clearer to do: > > u16 data = 0; > > if (msg->tx_len > 0) > data |= tx_buf[0]; > if (msg->tx_len > 1) > data |= tx_buf[1]; Yep, this looks good to me, but I think you need: data |= tx_buf[1] << 8; Sean > > val = GEN_HDATA(data) | GEN_HTYPE(msg->type); > > ? > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Sean Paul, Software Engineer, Google / Chromium OS