From mboxrd@z Thu Jan 1 00:00:00 1970 From: John Keeping Subject: Re: [PATCH v3 06/24] drm/rockchip: dw-mipi-dsi: avoid out-of-bounds read on tx_buf Date: Tue, 31 Jan 2017 11:45:48 +0000 Message-ID: <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> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <20170130200955.GL20076@art_vandelay> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Sean Paul 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 T24gTW9uLCAzMCBKYW4gMjAxNyAxNTowOTo1NSAtMDUwMCwgU2VhbiBQYXVsIHdyb3RlOgoKPiBP biBNb24sIEphbiAzMCwgMjAxNyBhdCAwNjoxNjozNlBNICswMDAwLCBKb2huIEtlZXBpbmcgd3Jv dGU6Cj4gPiBPbiBNb24sIDMwIEphbiAyMDE3IDEzOjAxOjQ2IC0wNTAwLCBTZWFuIFBhdWwgd3Jv dGU6Cj4gPiAgIAo+ID4gPiBPbiBTdW4sIEphbiAyOSwgMjAxNyBhdCAwMToyNDoyNlBNICswMDAw LCBKb2huIEtlZXBpbmcgd3JvdGU6ICAKPiA+ID4gPiBBcyBhIHNpZGUtZWZmZWN0IG9mIHRoaXMs IGVuY29kZSB0aGUgZW5kaWFubmVzcyBleHBsaWNpdGx5IHJhdGhlciB0aGFuCj4gPiA+ID4gY2Fz dGluZyBhIHUxNi4KPiA+ID4gPiAKPiA+ID4gPiBTaWduZWQtb2ZmLWJ5OiBKb2huIEtlZXBpbmcg PGpvaG5AbWV0YW5hdGUuY29tPgo+ID4gPiA+IFJldmlld2VkLWJ5OiBDaHJpcyBaaG9uZyA8enl3 QHJvY2stY2hpcHMuY29tPgo+ID4gPiA+IC0tLQo+ID4gPiA+IHYzOgo+ID4gPiA+IC0gQWRkIENo cmlzJyBSZXZpZXdlZC1ieQo+ID4gPiA+IFVuY2hhbmdlZCBpbiB2Mgo+ID4gPiA+IAo+ID4gPiA+ ICBkcml2ZXJzL2dwdS9kcm0vcm9ja2NoaXAvZHctbWlwaS1kc2kuYyB8IDkgKysrKysrKy0tCj4g PiA+ID4gIDEgZmlsZSBjaGFuZ2VkLCA3IGluc2VydGlvbnMoKyksIDIgZGVsZXRpb25zKC0pCj4g PiA+ID4gCj4gPiA+ID4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvZ3B1L2RybS9yb2NrY2hpcC9kdy1t aXBpLWRzaS5jIGIvZHJpdmVycy9ncHUvZHJtL3JvY2tjaGlwL2R3LW1pcGktZHNpLmMKPiA+ID4g PiBpbmRleCA0YmUxZmYzYTQyYmIuLjJlNmFkNDU5MWViZiAxMDA2NDQKPiA+ID4gPiAtLS0gYS9k cml2ZXJzL2dwdS9kcm0vcm9ja2NoaXAvZHctbWlwaS1kc2kuYwo+ID4gPiA+ICsrKyBiL2RyaXZl cnMvZ3B1L2RybS9yb2NrY2hpcC9kdy1taXBpLWRzaS5jCj4gPiA+ID4gQEAgLTU3Miw4ICs1NzIs MTMgQEAgc3RhdGljIGludCBkd19taXBpX2RzaV9nZW5fcGt0X2hkcl93cml0ZShzdHJ1Y3QgZHdf bWlwaV9kc2kgKmRzaSwgdTMyIGhkcl92YWwpCj4gPiA+ID4gIHN0YXRpYyBpbnQgZHdfbWlwaV9k c2lfZGNzX3Nob3J0X3dyaXRlKHN0cnVjdCBkd19taXBpX2RzaSAqZHNpLAo+ID4gPiA+ICAJCQkJ ICAgICAgIGNvbnN0IHN0cnVjdCBtaXBpX2RzaV9tc2cgKm1zZykKPiA+ID4gPiAgewo+ID4gPiA+ IC0JY29uc3QgdTE2ICp0eF9idWYgPSBtc2ctPnR4X2J1ZjsKPiA+ID4gPiAtCXUzMiB2YWwgPSBH RU5fSERBVEEoKnR4X2J1ZikgfCBHRU5fSFRZUEUobXNnLT50eXBlKTsKPiA+ID4gPiArCWNvbnN0 IHU4ICp0eF9idWYgPSBtc2ctPnR4X2J1ZjsKPiA+ID4gPiArCXUzMiB2YWwgPSBHRU5fSFRZUEUo bXNnLT50eXBlKTsKPiA+ID4gPiArCj4gPiA+ID4gKwlpZiAobXNnLT50eF9sZW4gPiAwKQo+ID4g PiA+ICsJCXZhbCB8PSBHRU5fSERBVEEodHhfYnVmWzBdKTsKPiA+ID4gPiArCWlmIChtc2ctPnR4 X2xlbiA+IDEpCj4gPiA+ID4gKwkJdmFsIHw9IEdFTl9IREFUQSh0eF9idWZbMV0gPDwgOCk7ICAg IAo+ID4gPiAKPiA+ID4gWW91IHNob3VsZCBwcm9iYWJseSB1cGRhdGUgdGhlIG1hc2sgaW5zaWRl IEdFTl9IREFUQSB0byBtYXNrIG9mZiA4IGJpdHMgaW5zdGVhZCBvZgo+ID4gPiAxNi4gIAo+ID4g Cj4gPiBXb24ndCB0aGF0IG1hc2sgb2ZmIHRoZSBkYXRhIHdyaXR0ZW4gYnkgInR4X2J1ZlsxXSA8 PCA4Ij8gIAo+IAo+IEkgd291bGQgbW92ZSB0aGUgc2hpZnQgb3V0c2lkZSB0aGUgbWFzaywgaWU6 Cj4gCj4gdmFsIHw9IEdFTl9IREFUQSh0eF9idWZbMV0pIDw8IDg7CgpJIGNhbiBkbyB0aGF0LCBi dXQgdGhhdCBkb2Vzbid0IHNlZW0gdG8gbWF0Y2ggdGhlIGludGVudGlvbiBvZiB0aGUKbWFjcm9z IHdoaWNoIGFyZSBhYm91dCBlbmNvZGluZyB0aGUgcGxhY2VtZW50IGFuZCBzaXplIG9mIGZpZWxk cyB3aXRoaW4KcmVnaXN0ZXJzLgoKTWF5YmUgaXQgd291bGQgYmUgY2xlYXJlciB0byBkbzoKCiAg ICB1MTYgZGF0YSA9IDA7CgogICAgaWYgKG1zZy0+dHhfbGVuID4gMCkKICAgICAgICBkYXRhIHw9 IHR4X2J1ZlswXTsKICAgIGlmIChtc2ctPnR4X2xlbiA+IDEpCiAgICAgICAgZGF0YSB8PSB0eF9i dWZbMV07CgogICAgdmFsID0gR0VOX0hEQVRBKGRhdGEpIHwgR0VOX0hUWVBFKG1zZy0+dHlwZSk7 Cgo/Cl9fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCmRyaS1k ZXZlbCBtYWlsaW5nIGxpc3QKZHJpLWRldmVsQGxpc3RzLmZyZWVkZXNrdG9wLm9yZwpodHRwczov L2xpc3RzLmZyZWVkZXNrdG9wLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2RyaS1kZXZlbAo= From mboxrd@z Thu Jan 1 00:00:00 1970 From: john@metanate.com (John Keeping) Date: Tue, 31 Jan 2017 11:45:48 +0000 Subject: [PATCH v3 06/24] drm/rockchip: dw-mipi-dsi: avoid out-of-bounds read on tx_buf In-Reply-To: <20170130200955.GL20076@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> Message-ID: <20170131114548.667d3911.john@metanate.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org 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]; val = GEN_HDATA(data) | GEN_HTYPE(msg->type); ? From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751981AbdAaLra (ORCPT ); Tue, 31 Jan 2017 06:47:30 -0500 Received: from dougal.metanate.com ([90.155.101.14]:59458 "EHLO metanate.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751295AbdAaLqh (ORCPT ); Tue, 31 Jan 2017 06:46:37 -0500 Date: Tue, 31 Jan 2017 11:45:48 +0000 From: John Keeping To: Sean Paul Cc: linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, linux-rockchip@lists.infradead.org, Chris Zhong , 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: <20170131114548.667d3911.john@metanate.com> In-Reply-To: <20170130200955.GL20076@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> Organization: Metanate Ltd X-Mailer: Claws Mail 3.14.1 (GTK+ 2.24.31; x86_64-unknown-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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]; val = GEN_HDATA(data) | GEN_HTYPE(msg->type); ?