From mboxrd@z Thu Jan 1 00:00:00 1970 From: ville.syrjala@linux.intel.com (Ville =?iso-8859-1?Q?Syrj=E4l=E4?=) Date: Fri, 30 Sep 2016 19:22:11 +0300 Subject: [PATCH] drm/sun4i: Check that the plane coordinates are not negative In-Reply-To: <20160930180826.169e3daf@bbrezillon> References: <20160930143320.26241-1-maxime.ripard@free-electrons.com> <20160930180826.169e3daf@bbrezillon> Message-ID: <20160930162211.GQ4329@intel.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Fri, Sep 30, 2016 at 06:08:26PM +0200, Boris Brezillon wrote: > On Fri, 30 Sep 2016 16:33:20 +0200 > Maxime Ripard wrote: > > > Our planes cannot be set at negative coordinates. Make sure we reject such > > configuration. > > > > Reported-by: Boris Brezillon > > Signed-off-by: Maxime Ripard > > --- > > drivers/gpu/drm/sun4i/sun4i_layer.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/gpu/drm/sun4i/sun4i_layer.c b/drivers/gpu/drm/sun4i/sun4i_layer.c > > index f0035bf5efea..f5463c4c2cde 100644 > > --- a/drivers/gpu/drm/sun4i/sun4i_layer.c > > +++ b/drivers/gpu/drm/sun4i/sun4i_layer.c > > @@ -29,6 +29,9 @@ struct sun4i_plane_desc { > > static int sun4i_backend_layer_atomic_check(struct drm_plane *plane, > > struct drm_plane_state *state) > > { > > + if ((state->crtc_x < 0) || (state->crtc_y < 0)) > > + return -EINVAL; > > + > > Hm, I think it's a perfectly valid use case from the DRM framework and > DRM user PoV: you may want to place your plane at a negative CRTC > offset (which means part of the plane will be hidden). > > Maybe I'm wrong, but it seems you can support that by adapting the > start address of your framebuffer pointer and the layer size. > > Have you tried doing something like that? You shouldn't even be looking at the user provided coordinates. Also you can't return an error from the atomic update hook. The void return value should have been a decent hint ;) The right fix would be to move all the error handling into the atomic check hook, which probably should just call the helper to clip the coordinates and whatnot. Then the update hook can just use at the clipped coordinates when programming the hw registers. > > --->8--- > diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c b/drivers/gpu/drm/sun4i/sun4i_backend.c > index 3ab560450a82..6b68804f3035 100644 > --- a/drivers/gpu/drm/sun4i/sun4i_backend.c > +++ b/drivers/gpu/drm/sun4i/sun4i_backend.c > @@ -110,15 +110,30 @@ int sun4i_backend_update_layer_coord(struct sun4i_backend *backend, > { > struct drm_plane_state *state = plane->state; > struct drm_framebuffer *fb = state->fb; > + int crtc_w, crtc_h, crtc_x, crtc_y; > > DRM_DEBUG_DRIVER("Updating layer %d\n", layer); > > + crtc_x = state->crtc_x; > + crtc_y = state->crtc_y; > + crtc_w = state->crtc_w; > + crtc_h = state->crtc_h; > + > + if (crtc_x < 0) { > + crtc_w += crtx_x; > + crtc_x = 0; > + } > + > + if (crtc_y < 0) { > + crtc_h += crtx_y; > + crtc_y = 0; > + } > + > if (plane->type == DRM_PLANE_TYPE_PRIMARY) { > DRM_DEBUG_DRIVER("Primary layer, updating global size W: %u H: %u\n", > state->crtc_w, state->crtc_h); > regmap_write(backend->regs, SUN4I_BACKEND_DISSIZE_REG, > - SUN4I_BACKEND_DISSIZE(state->crtc_w, > - state->crtc_h)); > + SUN4I_BACKEND_DISSIZE(crtc_w, crtc_h)); > } > > /* Set the line width */ > @@ -130,15 +145,13 @@ int sun4i_backend_update_layer_coord(struct sun4i_backend *backend, > DRM_DEBUG_DRIVER("Layer size W: %u H: %u\n", > state->crtc_w, state->crtc_h); > regmap_write(backend->regs, SUN4I_BACKEND_LAYSIZE_REG(layer), > - SUN4I_BACKEND_LAYSIZE(state->crtc_w, > - state->crtc_h)); > + SUN4I_BACKEND_LAYSIZE(crtc_w, crtc_h)); > > /* Set base coordinates */ > DRM_DEBUG_DRIVER("Layer coordinates X: %d Y: %d\n", > state->crtc_x, state->crtc_y); > regmap_write(backend->regs, SUN4I_BACKEND_LAYCOOR_REG(layer), > - SUN4I_BACKEND_LAYCOOR(state->crtc_x, > - state->crtc_y)); > + SUN4I_BACKEND_LAYCOOR(crtc_x, crtc_y)); > > return 0; > } > @@ -198,6 +211,12 @@ int sun4i_backend_update_layer_buffer(struct sun4i_backend *backend, > paddr += (state->src_x >> 16) * bpp; > paddr += (state->src_y >> 16) * fb->pitches[0]; > > + if (state->crtc_x < 0) > + paddr -= bpp * state->crtc_x; > + > + if (state->crtc_y < 0) > + paddr -= fb->pitches[0] * state->crtc_y; > + > DRM_DEBUG_DRIVER("Setting buffer address to %pad\n", &paddr); > > /* Write the 32 lower bits of the address (in bits) */ > _______________________________________________ > dri-devel mailing list > dri-devel at lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Ville Syrj?l? Intel OTC From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH] drm/sun4i: Check that the plane coordinates are not negative Date: Fri, 30 Sep 2016 19:22:11 +0300 Message-ID: <20160930162211.GQ4329@intel.com> References: <20160930143320.26241-1-maxime.ripard@free-electrons.com> <20160930180826.169e3daf@bbrezillon> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by gabe.freedesktop.org (Postfix) with ESMTPS id 23AF36E171 for ; Fri, 30 Sep 2016 16:22:16 +0000 (UTC) Content-Disposition: inline In-Reply-To: <20160930180826.169e3daf@bbrezillon> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Boris Brezillon Cc: linux-arm-kernel@lists.infradead.org, Daniel Vetter , Maxime Ripard , Chen-Yu Tsai , dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org T24gRnJpLCBTZXAgMzAsIDIwMTYgYXQgMDY6MDg6MjZQTSArMDIwMCwgQm9yaXMgQnJlemlsbG9u IHdyb3RlOgo+IE9uIEZyaSwgMzAgU2VwIDIwMTYgMTY6MzM6MjAgKzAyMDAKPiBNYXhpbWUgUmlw YXJkIDxtYXhpbWUucmlwYXJkQGZyZWUtZWxlY3Ryb25zLmNvbT4gd3JvdGU6Cj4gCj4gPiBPdXIg cGxhbmVzIGNhbm5vdCBiZSBzZXQgYXQgbmVnYXRpdmUgY29vcmRpbmF0ZXMuIE1ha2Ugc3VyZSB3 ZSByZWplY3Qgc3VjaAo+ID4gY29uZmlndXJhdGlvbi4KPiA+IAo+ID4gUmVwb3J0ZWQtYnk6IEJv cmlzIEJyZXppbGxvbiA8Ym9yaXMuYnJlemlsbG9uQGZyZWUtZWxlY3Ryb25zLmNvbT4KPiA+IFNp Z25lZC1vZmYtYnk6IE1heGltZSBSaXBhcmQgPG1heGltZS5yaXBhcmRAZnJlZS1lbGVjdHJvbnMu Y29tPgo+ID4gLS0tCj4gPiAgZHJpdmVycy9ncHUvZHJtL3N1bjRpL3N1bjRpX2xheWVyLmMgfCAz ICsrKwo+ID4gIDEgZmlsZSBjaGFuZ2VkLCAzIGluc2VydGlvbnMoKykKPiA+IAo+ID4gZGlmZiAt LWdpdCBhL2RyaXZlcnMvZ3B1L2RybS9zdW40aS9zdW40aV9sYXllci5jIGIvZHJpdmVycy9ncHUv ZHJtL3N1bjRpL3N1bjRpX2xheWVyLmMKPiA+IGluZGV4IGYwMDM1YmY1ZWZlYS4uZjU0NjNjNGMy Y2RlIDEwMDY0NAo+ID4gLS0tIGEvZHJpdmVycy9ncHUvZHJtL3N1bjRpL3N1bjRpX2xheWVyLmMK PiA+ICsrKyBiL2RyaXZlcnMvZ3B1L2RybS9zdW40aS9zdW40aV9sYXllci5jCj4gPiBAQCAtMjks NiArMjksOSBAQCBzdHJ1Y3Qgc3VuNGlfcGxhbmVfZGVzYyB7Cj4gPiAgc3RhdGljIGludCBzdW40 aV9iYWNrZW5kX2xheWVyX2F0b21pY19jaGVjayhzdHJ1Y3QgZHJtX3BsYW5lICpwbGFuZSwKPiA+ ICAJCQkJCSAgICBzdHJ1Y3QgZHJtX3BsYW5lX3N0YXRlICpzdGF0ZSkKPiA+ICB7Cj4gPiArCWlm ICgoc3RhdGUtPmNydGNfeCA8IDApIHx8IChzdGF0ZS0+Y3J0Y195IDwgMCkpCj4gPiArCQlyZXR1 cm4gLUVJTlZBTDsKPiA+ICsKPiAKPiBIbSwgSSB0aGluayBpdCdzIGEgcGVyZmVjdGx5IHZhbGlk IHVzZSBjYXNlIGZyb20gdGhlIERSTSBmcmFtZXdvcmsgYW5kCj4gRFJNIHVzZXIgUG9WOiB5b3Ug bWF5IHdhbnQgdG8gcGxhY2UgeW91ciBwbGFuZSBhdCBhIG5lZ2F0aXZlIENSVEMKPiBvZmZzZXQg KHdoaWNoIG1lYW5zIHBhcnQgb2YgdGhlIHBsYW5lIHdpbGwgYmUgaGlkZGVuKS4KPiAKPiBNYXli ZSBJJ20gd3JvbmcsIGJ1dCBpdCBzZWVtcyB5b3UgY2FuIHN1cHBvcnQgdGhhdCBieSBhZGFwdGlu ZyB0aGUKPiBzdGFydCBhZGRyZXNzIG9mIHlvdXIgZnJhbWVidWZmZXIgcG9pbnRlciBhbmQgdGhl IGxheWVyIHNpemUuCj4gCj4gSGF2ZSB5b3UgdHJpZWQgZG9pbmcgc29tZXRoaW5nIGxpa2UgdGhh dD8KCllvdSBzaG91bGRuJ3QgZXZlbiBiZSBsb29raW5nIGF0IHRoZSB1c2VyIHByb3ZpZGVkIGNv b3JkaW5hdGVzLiBBbHNvCnlvdSBjYW4ndCByZXR1cm4gYW4gZXJyb3IgZnJvbSB0aGUgYXRvbWlj IHVwZGF0ZSBob29rLiBUaGUgdm9pZCByZXR1cm4KdmFsdWUgc2hvdWxkIGhhdmUgYmVlbiBhIGRl Y2VudCBoaW50IDspIFRoZSByaWdodCBmaXggd291bGQgYmUKdG8gbW92ZSBhbGwgdGhlIGVycm9y IGhhbmRsaW5nIGludG8gdGhlIGF0b21pYyBjaGVjayBob29rLCB3aGljaApwcm9iYWJseSBzaG91 bGQganVzdCBjYWxsIHRoZSBoZWxwZXIgdG8gY2xpcCB0aGUgY29vcmRpbmF0ZXMgYW5kCndoYXRu b3QuIFRoZW4gdGhlIHVwZGF0ZSBob29rIGNhbiBqdXN0IHVzZSBhdCB0aGUgY2xpcHBlZCAKY29v cmRpbmF0ZXMgd2hlbiBwcm9ncmFtbWluZyB0aGUgaHcgcmVnaXN0ZXJzLgoKPiAKPiAtLS0+OC0t LQo+IGRpZmYgLS1naXQgYS9kcml2ZXJzL2dwdS9kcm0vc3VuNGkvc3VuNGlfYmFja2VuZC5jIGIv ZHJpdmVycy9ncHUvZHJtL3N1bjRpL3N1bjRpX2JhY2tlbmQuYwo+IGluZGV4IDNhYjU2MDQ1MGE4 Mi4uNmI2ODgwNGYzMDM1IDEwMDY0NAo+IC0tLSBhL2RyaXZlcnMvZ3B1L2RybS9zdW40aS9zdW40 aV9iYWNrZW5kLmMKPiArKysgYi9kcml2ZXJzL2dwdS9kcm0vc3VuNGkvc3VuNGlfYmFja2VuZC5j Cj4gQEAgLTExMCwxNSArMTEwLDMwIEBAIGludCBzdW40aV9iYWNrZW5kX3VwZGF0ZV9sYXllcl9j b29yZChzdHJ1Y3Qgc3VuNGlfYmFja2VuZCAqYmFja2VuZCwKPiAgewo+ICAgICAgICAgc3RydWN0 IGRybV9wbGFuZV9zdGF0ZSAqc3RhdGUgPSBwbGFuZS0+c3RhdGU7Cj4gICAgICAgICBzdHJ1Y3Qg ZHJtX2ZyYW1lYnVmZmVyICpmYiA9IHN0YXRlLT5mYjsKPiArICAgICAgIGludCBjcnRjX3csIGNy dGNfaCwgY3J0Y194LCBjcnRjX3k7Cj4gIAo+ICAgICAgICAgRFJNX0RFQlVHX0RSSVZFUigiVXBk YXRpbmcgbGF5ZXIgJWRcbiIsIGxheWVyKTsKPiAgCj4gKyAgICAgICBjcnRjX3ggPSBzdGF0ZS0+ Y3J0Y194Owo+ICsgICAgICAgY3J0Y195ID0gc3RhdGUtPmNydGNfeTsKPiArICAgICAgIGNydGNf dyA9IHN0YXRlLT5jcnRjX3c7Cj4gKyAgICAgICBjcnRjX2ggPSBzdGF0ZS0+Y3J0Y19oOwo+ICsK PiArICAgICAgIGlmIChjcnRjX3ggPCAwKSB7Cj4gKyAgICAgICAgICAgICAgIGNydGNfdyArPSBj cnR4X3g7Cj4gKyAgICAgICAgICAgICAgIGNydGNfeCA9IDA7Cj4gKyAgICAgICB9Cj4gKwo+ICsg ICAgICAgaWYgKGNydGNfeSA8IDApIHsKPiArICAgICAgICAgICAgICAgY3J0Y19oICs9IGNydHhf eTsKPiArICAgICAgICAgICAgICAgY3J0Y195ID0gMDsKPiArICAgICAgIH0KPiArCj4gICAgICAg ICBpZiAocGxhbmUtPnR5cGUgPT0gRFJNX1BMQU5FX1RZUEVfUFJJTUFSWSkgewo+ICAgICAgICAg ICAgICAgICBEUk1fREVCVUdfRFJJVkVSKCJQcmltYXJ5IGxheWVyLCB1cGRhdGluZyBnbG9iYWwg c2l6ZSBXOiAldSBIOiAldVxuIiwKPiAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBz dGF0ZS0+Y3J0Y193LCBzdGF0ZS0+Y3J0Y19oKTsKPiAgICAgICAgICAgICAgICAgcmVnbWFwX3dy aXRlKGJhY2tlbmQtPnJlZ3MsIFNVTjRJX0JBQ0tFTkRfRElTU0laRV9SRUcsCj4gLSAgICAgICAg ICAgICAgICAgICAgICAgICAgICBTVU40SV9CQUNLRU5EX0RJU1NJWkUoc3RhdGUtPmNydGNfdywK PiAtICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBzdGF0 ZS0+Y3J0Y19oKSk7Cj4gKyAgICAgICAgICAgICAgICAgICAgICAgICAgICBTVU40SV9CQUNLRU5E X0RJU1NJWkUoY3J0Y193LCBjcnRjX2gpKTsKPiAgICAgICAgIH0KPiAgCj4gICAgICAgICAvKiBT ZXQgdGhlIGxpbmUgd2lkdGggKi8KPiBAQCAtMTMwLDE1ICsxNDUsMTMgQEAgaW50IHN1bjRpX2Jh Y2tlbmRfdXBkYXRlX2xheWVyX2Nvb3JkKHN0cnVjdCBzdW40aV9iYWNrZW5kICpiYWNrZW5kLAo+ ICAgICAgICAgRFJNX0RFQlVHX0RSSVZFUigiTGF5ZXIgc2l6ZSBXOiAldSBIOiAldVxuIiwKPiAg ICAgICAgICAgICAgICAgICAgICAgICAgc3RhdGUtPmNydGNfdywgc3RhdGUtPmNydGNfaCk7Cj4g ICAgICAgICByZWdtYXBfd3JpdGUoYmFja2VuZC0+cmVncywgU1VONElfQkFDS0VORF9MQVlTSVpF X1JFRyhsYXllciksCj4gLSAgICAgICAgICAgICAgICAgICAgU1VONElfQkFDS0VORF9MQVlTSVpF KHN0YXRlLT5jcnRjX3csCj4gLSAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgIHN0YXRlLT5jcnRjX2gpKTsKPiArICAgICAgICAgICAgICAgICAgICBTVU40SV9CQUNLRU5E X0xBWVNJWkUoY3J0Y193LCBjcnRjX2gpKTsKPiAgCj4gICAgICAgICAvKiBTZXQgYmFzZSBjb29y ZGluYXRlcyAqLwo+ICAgICAgICAgRFJNX0RFQlVHX0RSSVZFUigiTGF5ZXIgY29vcmRpbmF0ZXMg WDogJWQgWTogJWRcbiIsCj4gICAgICAgICAgICAgICAgICAgICAgICAgIHN0YXRlLT5jcnRjX3gs IHN0YXRlLT5jcnRjX3kpOwo+ICAgICAgICAgcmVnbWFwX3dyaXRlKGJhY2tlbmQtPnJlZ3MsIFNV TjRJX0JBQ0tFTkRfTEFZQ09PUl9SRUcobGF5ZXIpLAo+IC0gICAgICAgICAgICAgICAgICAgIFNV TjRJX0JBQ0tFTkRfTEFZQ09PUihzdGF0ZS0+Y3J0Y194LAo+IC0gICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgICBzdGF0ZS0+Y3J0Y195KSk7Cj4gKyAgICAgICAgICAgICAg ICAgICAgU1VONElfQkFDS0VORF9MQVlDT09SKGNydGNfeCwgY3J0Y195KSk7Cj4gIAo+ICAgICAg ICAgcmV0dXJuIDA7Cj4gIH0KPiBAQCAtMTk4LDYgKzIxMSwxMiBAQCBpbnQgc3VuNGlfYmFja2Vu ZF91cGRhdGVfbGF5ZXJfYnVmZmVyKHN0cnVjdCBzdW40aV9iYWNrZW5kICpiYWNrZW5kLAo+ICAg ICAgICAgcGFkZHIgKz0gKHN0YXRlLT5zcmNfeCA+PiAxNikgKiBicHA7Cj4gICAgICAgICBwYWRk ciArPSAoc3RhdGUtPnNyY195ID4+IDE2KSAqIGZiLT5waXRjaGVzWzBdOwo+ICAKPiArICAgICAg IGlmIChzdGF0ZS0+Y3J0Y194IDwgMCkKPiArICAgICAgICAgICAgICAgcGFkZHIgLT0gYnBwICog c3RhdGUtPmNydGNfeDsKPiArCj4gKyAgICAgICBpZiAoc3RhdGUtPmNydGNfeSA8IDApCj4gKyAg ICAgICAgICAgICAgIHBhZGRyIC09IGZiLT5waXRjaGVzWzBdICogc3RhdGUtPmNydGNfeTsKPiAr Cj4gICAgICAgICBEUk1fREVCVUdfRFJJVkVSKCJTZXR0aW5nIGJ1ZmZlciBhZGRyZXNzIHRvICVw YWRcbiIsICZwYWRkcik7Cj4gIAo+ICAgICAgICAgLyogV3JpdGUgdGhlIDMyIGxvd2VyIGJpdHMg b2YgdGhlIGFkZHJlc3MgKGluIGJpdHMpICovCj4gX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX18KPiBkcmktZGV2ZWwgbWFpbGluZyBsaXN0Cj4gZHJpLWRldmVs QGxpc3RzLmZyZWVkZXNrdG9wLm9yZwo+IGh0dHBzOi8vbGlzdHMuZnJlZWRlc2t0b3Aub3JnL21h aWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCgotLSAKVmlsbGUgU3lyasOkbMOkCkludGVsIE9UQwpf X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwpkcmktZGV2ZWwg bWFpbGluZyBsaXN0CmRyaS1kZXZlbEBsaXN0cy5mcmVlZGVza3RvcC5vcmcKaHR0cHM6Ly9saXN0 cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9kcmktZGV2ZWwK