From mboxrd@z Thu Jan 1 00:00:00 1970 From: Laurent Pinchart Subject: Re: [PATCH] drm/fence: fix memory overwrite when setting out_fence fd Date: Fri, 13 Jan 2017 17:00:36 +0200 Message-ID: <2003560.ZPyAhlVHHW@avalon> References: <1484317329-9293-1-git-send-email-gustavo@padovan.org> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from galahad.ideasonboard.com (galahad.ideasonboard.com [IPv6:2001:4b98:dc2:45:216:3eff:febb:480d]) by gabe.freedesktop.org (Postfix) with ESMTPS id 65D1D6ED85 for ; Fri, 13 Jan 2017 15:00:23 +0000 (UTC) In-Reply-To: <1484317329-9293-1-git-send-email-gustavo@padovan.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Gustavo Padovan Cc: Gustavo Padovan , stable@vger.kernel.org, dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org SGkgR3VzdGF2bywKClRoYW5rIHlvdSBmb3IgdGhlIHBhdGNoLgoKT24gRnJpZGF5IDEzIEphbiAy MDE3IDEyOjIyOjA5IEd1c3Rhdm8gUGFkb3ZhbiB3cm90ZToKPiBGcm9tOiBHdXN0YXZvIFBhZG92 YW4gPGd1c3Rhdm8ucGFkb3ZhbkBjb2xsYWJvcmEuY29tPgo+IAo+IEN1cnJlbnRseSBpZiB0aGUg dXNlcnNwYWNlIGRlY2xhcmVzIGEgaW50IHZhcmlhYmxlIHRvIHN0b3JlIHRoZSBvdXRfZmVuY2UK PiBmZCBhbmQgcGFzcyBpdCB0byBPVVRfRkVOQ0VfUFRSIHRoZSBrZXJuZWwgd2lsbCBvdmVyd3Jp dGUgdGhlIDMyIGJpdHMKPiBhYm92ZSB0aGUgaW50IHZhcmlhYmxlIG9uIDY0IGJpdHMgc3lzdGVt cy4KPiAKPiBGaXggdGhpcyBieSBtYWtpbmcgdGhlIGludGVybmFsIHN0b3JhZ2Ugb2Ygb3V0X2Zl bmNlIGluIHRoZSBrZXJuZWwgYSBzMzIKPiBwb2ludGVyLgo+IAo+IFJlcG9ydGVkLWJ5OiBDaGFk IFZlcnNhY2UgPGNoYWR2ZXJzYXJ5QGNocm9taXVtLm9yZz4KPiBTaWduZWQtb2ZmLWJ5OiBHdXN0 YXZvIFBhZG92YW4gPGd1c3Rhdm8ucGFkb3ZhbkBjb2xsYWJvcmEuY29tPgo+IENjOiBEYW5pZWwg VmV0dGVyIDxkYW5pZWxAZmZ3bGwuY2g+Cj4gQ2M6IFJhZmFlbCBBbnRvZ25vbGxpIDxyYWZhZWwu YW50b2dub2xsaUBpbnRlbC5jb20+Cj4gQ2M6IExhdXJlbnQgUGluY2hhcnQgPGxhdXJlbnQucGlu Y2hhcnRAaWRlYXNvbmJvYXJkLmNvbT4KCkFja2VkLWJ5OiBMYXVyZW50IFBpbmNoYXJ0IDxsYXVy ZW50LnBpbmNoYXJ0QGlkZWFzb25ib2FyZC5jb20+Cgo+IENjOiBzdGFibGVAdmdlci5rZXJuZWwu b3JnCgpJIGRvbid0IHRoaW5rIHRoaXMgaXMgbmVlZGVkLCBnaXZlbiB0aGF0IHRoZSBjb2RlIHdh cyBtZXJnZWQgaW4gdjQuMTAtcmMxLCBhbmQgCnRoaXMgcGF0Y2ggc2hvdWxkIGJlIG1lcmdlZCBh cyBhIHY0LjEwLXJjIGZpeC4KCj4gLS0tCj4gIGRyaXZlcnMvZ3B1L2RybS9kcm1fYXRvbWljLmMg IHwgMTIgKysrKysrLS0tLS0tCj4gIGluY2x1ZGUvZHJtL2RybV9hdG9taWMuaCAgICAgIHwgIDIg Ky0KPiAgaW5jbHVkZS9kcm0vZHJtX21vZGVfY29uZmlnLmggfCAgMiArLQo+ICAzIGZpbGVzIGNo YW5nZWQsIDggaW5zZXJ0aW9ucygrKSwgOCBkZWxldGlvbnMoLSkKPiAKPiBkaWZmIC0tZ2l0IGEv ZHJpdmVycy9ncHUvZHJtL2RybV9hdG9taWMuYyBiL2RyaXZlcnMvZ3B1L2RybS9kcm1fYXRvbWlj LmMKPiBpbmRleCA2NDE0YmNmLi43MjMzOTJmIDEwMDY0NAo+IC0tLSBhL2RyaXZlcnMvZ3B1L2Ry bS9kcm1fYXRvbWljLmMKPiArKysgYi9kcml2ZXJzL2dwdS9kcm0vZHJtX2F0b21pYy5jCj4gQEAg LTI4NiwxNSArMjg2LDE1IEBAIGRybV9hdG9taWNfZ2V0X2NydGNfc3RhdGUoc3RydWN0IGRybV9h dG9taWNfc3RhdGUKPiAqc3RhdGUsIEVYUE9SVF9TWU1CT0woZHJtX2F0b21pY19nZXRfY3J0Y19z dGF0ZSk7Cj4gCj4gIHN0YXRpYyB2b2lkIHNldF9vdXRfZmVuY2VfZm9yX2NydGMoc3RydWN0IGRy bV9hdG9taWNfc3RhdGUgKnN0YXRlLAo+IC0JCQkJICAgc3RydWN0IGRybV9jcnRjICpjcnRjLCBz NjQgX191c2VyIAoqZmVuY2VfcHRyKQo+ICsJCQkJICAgc3RydWN0IGRybV9jcnRjICpjcnRjLCBz MzIgX191c2VyIAoqZmVuY2VfcHRyKQo+ICB7Cj4gIAlzdGF0ZS0+Y3J0Y3NbZHJtX2NydGNfaW5k ZXgoY3J0YyldLm91dF9mZW5jZV9wdHIgPSBmZW5jZV9wdHI7Cj4gIH0KPiAKPiAtc3RhdGljIHM2 NCBfX3VzZXIgKmdldF9vdXRfZmVuY2VfZm9yX2NydGMoc3RydWN0IGRybV9hdG9taWNfc3RhdGUg KnN0YXRlLAo+ICtzdGF0aWMgczMyIF9fdXNlciAqZ2V0X291dF9mZW5jZV9mb3JfY3J0YyhzdHJ1 Y3QgZHJtX2F0b21pY19zdGF0ZSAqc3RhdGUsCj4gIAkJCQkJICBzdHJ1Y3QgZHJtX2NydGMgKmNy dGMpCj4gIHsKPiAtCXM2NCBfX3VzZXIgKmZlbmNlX3B0cjsKPiArCXMzMiBfX3VzZXIgKmZlbmNl X3B0cjsKPiAKPiAgCWZlbmNlX3B0ciA9IHN0YXRlLT5jcnRjc1tkcm1fY3J0Y19pbmRleChjcnRj KV0ub3V0X2ZlbmNlX3B0cjsKPiAgCXN0YXRlLT5jcnRjc1tkcm1fY3J0Y19pbmRleChjcnRjKV0u b3V0X2ZlbmNlX3B0ciA9IE5VTEw7Cj4gQEAgLTUwNyw3ICs1MDcsNyBAQCBpbnQgZHJtX2F0b21p Y19jcnRjX3NldF9wcm9wZXJ0eShzdHJ1Y3QgZHJtX2NydGMgKmNydGMsCj4gIAkJc3RhdGUtPmNv bG9yX21nbXRfY2hhbmdlZCB8PSByZXBsYWNlZDsKPiAgCQlyZXR1cm4gcmV0Owo+ICAJfSBlbHNl IGlmIChwcm9wZXJ0eSA9PSBjb25maWctPnByb3Bfb3V0X2ZlbmNlX3B0cikgewo+IC0JCXM2NCBf X3VzZXIgKmZlbmNlX3B0ciA9IHU2NF90b191c2VyX3B0cih2YWwpOwo+ICsJCXMzMiBfX3VzZXIg KmZlbmNlX3B0ciA9IHU2NF90b191c2VyX3B0cih2YWwpOwo+IAo+ICAJCWlmICghZmVuY2VfcHRy KQo+ICAJCQlyZXR1cm4gMDsKPiBAQCAtMTkxNCw3ICsxOTE0LDcgQEAgRVhQT1JUX1NZTUJPTChk cm1fYXRvbWljX2NsZWFuX29sZF9mYik7Cj4gICAqLwo+IAo+ICBzdHJ1Y3QgZHJtX291dF9mZW5j ZV9zdGF0ZSB7Cj4gLQlzNjQgX191c2VyICpvdXRfZmVuY2VfcHRyOwo+ICsJczMyIF9fdXNlciAq b3V0X2ZlbmNlX3B0cjsKPiAgCXN0cnVjdCBzeW5jX2ZpbGUgKnN5bmNfZmlsZTsKPiAgCWludCBm ZDsKPiAgfTsKPiBAQCAtMTk1MSw3ICsxOTUxLDcgQEAgc3RhdGljIGludCBwcmVwYXJlX2NydGNf c2lnbmFsaW5nKHN0cnVjdCBkcm1fZGV2aWNlCj4gKmRldiwgcmV0dXJuIDA7Cj4gCj4gIAlmb3Jf ZWFjaF9jcnRjX2luX3N0YXRlKHN0YXRlLCBjcnRjLCBjcnRjX3N0YXRlLCBpKSB7Cj4gLQkJdTY0 IF9fdXNlciAqZmVuY2VfcHRyOwo+ICsJCXMzMiBfX3VzZXIgKmZlbmNlX3B0cjsKPiAKPiAgCQlm ZW5jZV9wdHIgPSBnZXRfb3V0X2ZlbmNlX2Zvcl9jcnRjKGNydGNfc3RhdGUtPnN0YXRlLCBjcnRj KTsKPiAKPiBkaWZmIC0tZ2l0IGEvaW5jbHVkZS9kcm0vZHJtX2F0b21pYy5oIGIvaW5jbHVkZS9k cm0vZHJtX2F0b21pYy5oCj4gaW5kZXggZjk2MjIwZS4uZjFjYjJiMCAxMDA2NDQKPiAtLS0gYS9p bmNsdWRlL2RybS9kcm1fYXRvbWljLmgKPiArKysgYi9pbmNsdWRlL2RybS9kcm1fYXRvbWljLmgK PiBAQCAtMTQ0LDcgKzE0NCw3IEBAIHN0cnVjdCBfX2RybV9jcnRjc19zdGF0ZSB7Cj4gIAlzdHJ1 Y3QgZHJtX2NydGMgKnB0cjsKPiAgCXN0cnVjdCBkcm1fY3J0Y19zdGF0ZSAqc3RhdGU7Cj4gIAlz dHJ1Y3QgZHJtX2NydGNfY29tbWl0ICpjb21taXQ7Cj4gLQlzNjQgX191c2VyICpvdXRfZmVuY2Vf cHRyOwo+ICsJczMyIF9fdXNlciAqb3V0X2ZlbmNlX3B0cjsKPiAgCXVuc2lnbmVkIGxhc3RfdmJs YW5rX2NvdW50Owo+ICB9Owo+IAo+IGRpZmYgLS1naXQgYS9pbmNsdWRlL2RybS9kcm1fbW9kZV9j b25maWcuaCBiL2luY2x1ZGUvZHJtL2RybV9tb2RlX2NvbmZpZy5oCj4gaW5kZXggMTc5NDJjMC4u ZmUyMzBmMSAxMDA2NDQKPiAtLS0gYS9pbmNsdWRlL2RybS9kcm1fbW9kZV9jb25maWcuaAo+ICsr KyBiL2luY2x1ZGUvZHJtL2RybV9tb2RlX2NvbmZpZy5oCj4gQEAgLTQ5Niw3ICs0OTYsNyBAQCBz dHJ1Y3QgZHJtX21vZGVfY29uZmlnIHsKPiAgCS8qKgo+ICAJICogQHByb3Bfb3V0X2ZlbmNlX3B0 cjogU3luYyBGaWxlIGZkIHBvaW50ZXIgcmVwcmVzZW50aW5nIHRoZQo+ICAJICogb3V0Z29pbmcg ZmVuY2VzIGZvciBhIENSVEMuIFVzZXJzcGFjZSBzaG91bGQgcHJvdmlkZSBhIHBvaW50ZXIgdG8g YQo+IC0JICogdmFsdWUgb2YgdHlwZSBzNjQsIGFuZCB0aGVuIGNhc3QgdGhhdCBwb2ludGVyIHRv IHU2NC4KPiArCSAqIHZhbHVlIG9mIHR5cGUgczMyLCBhbmQgdGhlbiBjYXN0IHRoYXQgcG9pbnRl ciB0byB1NjQuCj4gIAkgKi8KPiAgCXN0cnVjdCBkcm1fcHJvcGVydHkgKnByb3Bfb3V0X2ZlbmNl X3B0cjsKPiAgCS8qKgoKLS0gClJlZ2FyZHMsCgpMYXVyZW50IFBpbmNoYXJ0CgpfX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwpkcmktZGV2ZWwgbWFpbGluZyBs aXN0CmRyaS1kZXZlbEBsaXN0cy5mcmVlZGVza3RvcC5vcmcKaHR0cHM6Ly9saXN0cy5mcmVlZGVz a3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9kcmktZGV2ZWwK From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from galahad.ideasonboard.com ([185.26.127.97]:54278 "EHLO galahad.ideasonboard.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751678AbdAMPHr (ORCPT ); Fri, 13 Jan 2017 10:07:47 -0500 From: Laurent Pinchart To: Gustavo Padovan Cc: dri-devel@lists.freedesktop.org, Gustavo Padovan , Daniel Vetter , Rafael Antognolli , stable@vger.kernel.org Subject: Re: [PATCH] drm/fence: fix memory overwrite when setting out_fence fd Date: Fri, 13 Jan 2017 17:00:36 +0200 Message-ID: <2003560.ZPyAhlVHHW@avalon> In-Reply-To: <1484317329-9293-1-git-send-email-gustavo@padovan.org> References: <1484317329-9293-1-git-send-email-gustavo@padovan.org> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Sender: stable-owner@vger.kernel.org List-ID: Hi Gustavo, Thank you for the patch. On Friday 13 Jan 2017 12:22:09 Gustavo Padovan wrote: > From: Gustavo Padovan > > Currently if the userspace declares a int variable to store the out_fence > fd and pass it to OUT_FENCE_PTR the kernel will overwrite the 32 bits > above the int variable on 64 bits systems. > > Fix this by making the internal storage of out_fence in the kernel a s32 > pointer. > > Reported-by: Chad Versace > Signed-off-by: Gustavo Padovan > Cc: Daniel Vetter > Cc: Rafael Antognolli > Cc: Laurent Pinchart Acked-by: Laurent Pinchart > Cc: stable@vger.kernel.org I don't think this is needed, given that the code was merged in v4.10-rc1, and this patch should be merged as a v4.10-rc fix. > --- > drivers/gpu/drm/drm_atomic.c | 12 ++++++------ > include/drm/drm_atomic.h | 2 +- > include/drm/drm_mode_config.h | 2 +- > 3 files changed, 8 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > index 6414bcf..723392f 100644 > --- a/drivers/gpu/drm/drm_atomic.c > +++ b/drivers/gpu/drm/drm_atomic.c > @@ -286,15 +286,15 @@ drm_atomic_get_crtc_state(struct drm_atomic_state > *state, EXPORT_SYMBOL(drm_atomic_get_crtc_state); > > static void set_out_fence_for_crtc(struct drm_atomic_state *state, > - struct drm_crtc *crtc, s64 __user *fence_ptr) > + struct drm_crtc *crtc, s32 __user *fence_ptr) > { > state->crtcs[drm_crtc_index(crtc)].out_fence_ptr = fence_ptr; > } > > -static s64 __user *get_out_fence_for_crtc(struct drm_atomic_state *state, > +static s32 __user *get_out_fence_for_crtc(struct drm_atomic_state *state, > struct drm_crtc *crtc) > { > - s64 __user *fence_ptr; > + s32 __user *fence_ptr; > > fence_ptr = state->crtcs[drm_crtc_index(crtc)].out_fence_ptr; > state->crtcs[drm_crtc_index(crtc)].out_fence_ptr = NULL; > @@ -507,7 +507,7 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc, > state->color_mgmt_changed |= replaced; > return ret; > } else if (property == config->prop_out_fence_ptr) { > - s64 __user *fence_ptr = u64_to_user_ptr(val); > + s32 __user *fence_ptr = u64_to_user_ptr(val); > > if (!fence_ptr) > return 0; > @@ -1914,7 +1914,7 @@ EXPORT_SYMBOL(drm_atomic_clean_old_fb); > */ > > struct drm_out_fence_state { > - s64 __user *out_fence_ptr; > + s32 __user *out_fence_ptr; > struct sync_file *sync_file; > int fd; > }; > @@ -1951,7 +1951,7 @@ static int prepare_crtc_signaling(struct drm_device > *dev, return 0; > > for_each_crtc_in_state(state, crtc, crtc_state, i) { > - u64 __user *fence_ptr; > + s32 __user *fence_ptr; > > fence_ptr = get_out_fence_for_crtc(crtc_state->state, crtc); > > diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h > index f96220e..f1cb2b0 100644 > --- a/include/drm/drm_atomic.h > +++ b/include/drm/drm_atomic.h > @@ -144,7 +144,7 @@ struct __drm_crtcs_state { > struct drm_crtc *ptr; > struct drm_crtc_state *state; > struct drm_crtc_commit *commit; > - s64 __user *out_fence_ptr; > + s32 __user *out_fence_ptr; > unsigned last_vblank_count; > }; > > diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h > index 17942c0..fe230f1 100644 > --- a/include/drm/drm_mode_config.h > +++ b/include/drm/drm_mode_config.h > @@ -496,7 +496,7 @@ struct drm_mode_config { > /** > * @prop_out_fence_ptr: Sync File fd pointer representing the > * outgoing fences for a CRTC. Userspace should provide a pointer to a > - * value of type s64, and then cast that pointer to u64. > + * value of type s32, and then cast that pointer to u64. > */ > struct drm_property *prop_out_fence_ptr; > /** -- Regards, Laurent Pinchart