From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755998AbcB0CUo (ORCPT ); Fri, 26 Feb 2016 21:20:44 -0500 Received: from mail-wm0-f53.google.com ([74.125.82.53]:34195 "EHLO mail-wm0-f53.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752617AbcB0CUn (ORCPT ); Fri, 26 Feb 2016 21:20:43 -0500 MIME-Version: 1.0 In-Reply-To: <1456511507-2534-5-git-send-email-gustavo@padovan.org> References: <1456511507-2534-1-git-send-email-gustavo@padovan.org> <1456511507-2534-5-git-send-email-gustavo@padovan.org> Date: Sat, 27 Feb 2016 02:20:41 +0000 Message-ID: Subject: Re: [PATCH v4 5/5] staging/android: add flags member to sync ioctl structs From: Emil Velikov To: Gustavo Padovan Cc: Greg Kroah-Hartman , devel@driverdev.osuosl.org, Daniel Stone , Daniel Vetter , Riley Andrews , ML dri-devel , "Linux-Kernel@Vger. Kernel. Org" , =?UTF-8?B?QXJ2ZSBIasO4bm5ldsOlZw==?= , Gustavo Padovan , John Harrison Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Gustavo, On 26 February 2016 at 18:31, Gustavo Padovan wrote: > From: Gustavo Padovan > > Play safe and add flags member to all structs. So we don't need to > break API or create new IOCTL in the future if new features that requires > flags arises. > > v2: check if flags are valid (zero, in this case) > > Signed-off-by: Gustavo Padovan > --- > drivers/staging/android/sync.c | 7 ++++++- > drivers/staging/android/uapi/sync.h | 6 ++++++ > 2 files changed, 12 insertions(+), 1 deletion(-) > > diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c > index 837cff5..54fd5ab 100644 > --- a/drivers/staging/android/sync.c > +++ b/drivers/staging/android/sync.c > @@ -445,6 +445,11 @@ static long sync_file_ioctl_merge(struct sync_file *sync_file, > goto err_put_fd; > } > > + if (data.flags) { > + err = -EFAULT; -EINVAL ? > + goto err_put_fd; > + } > + > fence2 = sync_file_fdget(data.fd2); > if (!fence2) { > err = -ENOENT; > @@ -511,7 +516,7 @@ static long sync_file_ioctl_fence_info(struct sync_file *sync_file, > if (copy_from_user(&in, (void __user *)arg, sizeof(*info))) > return -EFAULT; > > - if (in.status || strcmp(in.name, "\0")) > + if (in.status || in.flags || strcmp(in.name, "\0")) > return -EFAULT; -EINVAL ? > > if (in.num_fences && !in.sync_fence_info) > diff --git a/drivers/staging/android/uapi/sync.h b/drivers/staging/android/uapi/sync.h > index 9aad623..f56a6c2 100644 > --- a/drivers/staging/android/uapi/sync.h > +++ b/drivers/staging/android/uapi/sync.h > @@ -19,11 +19,13 @@ > * @fd2: file descriptor of second fence > * @name: name of new fence > * @fence: returns the fd of the new fence to userspace > + * @flags: merge_data flags > */ > struct sync_merge_data { > __s32 fd2; > char name[32]; > __s32 fence; > + __u32 flags; The overall size of the struct is not multiple of 64bit, so things will end up badly if we decide to extend it in the future. Even if there's a small chance that update will be needed, we might as well pad it now (and check the padding for zero, returning -EINVAL). > }; > > /** > @@ -31,12 +33,14 @@ struct sync_merge_data { > * @obj_name: name of parent sync_timeline > * @driver_name: name of driver implementing the parent > * @status: status of the fence 0:active 1:signaled <0:error > + * @flags: fence_info flags > * @timestamp_ns: timestamp of status change in nanoseconds > */ > struct sync_fence_info { > char obj_name[32]; > char driver_name[32]; > __s32 status; > + __u32 flags; > __u64 timestamp_ns; Should we be doing some form of validation in sync_fill_fence_info() of 'flags' ? Regards, Emil From mboxrd@z Thu Jan 1 00:00:00 1970 From: Emil Velikov Subject: Re: [PATCH v4 5/5] staging/android: add flags member to sync ioctl structs Date: Sat, 27 Feb 2016 02:20:41 +0000 Message-ID: References: <1456511507-2534-1-git-send-email-gustavo@padovan.org> <1456511507-2534-5-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 mail-wm0-x22d.google.com (mail-wm0-x22d.google.com [IPv6:2a00:1450:400c:c09::22d]) by gabe.freedesktop.org (Postfix) with ESMTPS id D85EC6E019 for ; Sat, 27 Feb 2016 02:20:42 +0000 (UTC) Received: by mail-wm0-x22d.google.com with SMTP id g62so93301315wme.0 for ; Fri, 26 Feb 2016 18:20:42 -0800 (PST) In-Reply-To: <1456511507-2534-5-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: devel@driverdev.osuosl.org, Daniel Stone , Greg Kroah-Hartman , "Linux-Kernel@Vger. Kernel. Org" , ML dri-devel , Riley Andrews , =?UTF-8?B?QXJ2ZSBIasO4bm5ldsOlZw==?= , Daniel Vetter , Gustavo Padovan , John Harrison List-Id: dri-devel@lists.freedesktop.org SGkgR3VzdGF2bywKCk9uIDI2IEZlYnJ1YXJ5IDIwMTYgYXQgMTg6MzEsIEd1c3Rhdm8gUGFkb3Zh biA8Z3VzdGF2b0BwYWRvdmFuLm9yZz4gd3JvdGU6Cj4gRnJvbTogR3VzdGF2byBQYWRvdmFuIDxn dXN0YXZvLnBhZG92YW5AY29sbGFib3JhLmNvLnVrPgo+Cj4gUGxheSBzYWZlIGFuZCBhZGQgZmxh Z3MgbWVtYmVyIHRvIGFsbCBzdHJ1Y3RzLiBTbyB3ZSBkb24ndCBuZWVkIHRvCj4gYnJlYWsgQVBJ IG9yIGNyZWF0ZSBuZXcgSU9DVEwgaW4gdGhlIGZ1dHVyZSBpZiBuZXcgZmVhdHVyZXMgdGhhdCBy ZXF1aXJlcwo+IGZsYWdzIGFyaXNlcy4KPgo+IHYyOiBjaGVjayBpZiBmbGFncyBhcmUgdmFsaWQg KHplcm8sIGluIHRoaXMgY2FzZSkKPgo+IFNpZ25lZC1vZmYtYnk6IEd1c3Rhdm8gUGFkb3ZhbiA8 Z3VzdGF2by5wYWRvdmFuQGNvbGxhYm9yYS5jby51az4KPiAtLS0KPiAgZHJpdmVycy9zdGFnaW5n L2FuZHJvaWQvc3luYy5jICAgICAgfCA3ICsrKysrKy0KPiAgZHJpdmVycy9zdGFnaW5nL2FuZHJv aWQvdWFwaS9zeW5jLmggfCA2ICsrKysrKwo+ICAyIGZpbGVzIGNoYW5nZWQsIDEyIGluc2VydGlv bnMoKyksIDEgZGVsZXRpb24oLSkKPgo+IGRpZmYgLS1naXQgYS9kcml2ZXJzL3N0YWdpbmcvYW5k cm9pZC9zeW5jLmMgYi9kcml2ZXJzL3N0YWdpbmcvYW5kcm9pZC9zeW5jLmMKPiBpbmRleCA4Mzdj ZmY1Li41NGZkNWFiIDEwMDY0NAo+IC0tLSBhL2RyaXZlcnMvc3RhZ2luZy9hbmRyb2lkL3N5bmMu Ywo+ICsrKyBiL2RyaXZlcnMvc3RhZ2luZy9hbmRyb2lkL3N5bmMuYwo+IEBAIC00NDUsNiArNDQ1 LDExIEBAIHN0YXRpYyBsb25nIHN5bmNfZmlsZV9pb2N0bF9tZXJnZShzdHJ1Y3Qgc3luY19maWxl ICpzeW5jX2ZpbGUsCj4gICAgICAgICAgICAgICAgIGdvdG8gZXJyX3B1dF9mZDsKPiAgICAgICAg IH0KPgo+ICsgICAgICAgaWYgKGRhdGEuZmxhZ3MpIHsKPiArICAgICAgICAgICAgICAgZXJyID0g LUVGQVVMVDsKLUVJTlZBTCA/Cgo+ICsgICAgICAgICAgICAgICBnb3RvIGVycl9wdXRfZmQ7Cj4g KyAgICAgICB9Cj4gKwo+ICAgICAgICAgZmVuY2UyID0gc3luY19maWxlX2ZkZ2V0KGRhdGEuZmQy KTsKPiAgICAgICAgIGlmICghZmVuY2UyKSB7Cj4gICAgICAgICAgICAgICAgIGVyciA9IC1FTk9F TlQ7Cj4gQEAgLTUxMSw3ICs1MTYsNyBAQCBzdGF0aWMgbG9uZyBzeW5jX2ZpbGVfaW9jdGxfZmVu Y2VfaW5mbyhzdHJ1Y3Qgc3luY19maWxlICpzeW5jX2ZpbGUsCj4gICAgICAgICBpZiAoY29weV9m cm9tX3VzZXIoJmluLCAodm9pZCBfX3VzZXIgKilhcmcsIHNpemVvZigqaW5mbykpKQo+ICAgICAg ICAgICAgICAgICByZXR1cm4gLUVGQVVMVDsKPgo+IC0gICAgICAgaWYgKGluLnN0YXR1cyB8fCBz dHJjbXAoaW4ubmFtZSwgIlwwIikpCj4gKyAgICAgICBpZiAoaW4uc3RhdHVzIHx8IGluLmZsYWdz IHx8IHN0cmNtcChpbi5uYW1lLCAiXDAiKSkKPiAgICAgICAgICAgICAgICAgcmV0dXJuIC1FRkFV TFQ7Ci1FSU5WQUwgPwoKPgo+ICAgICAgICAgaWYgKGluLm51bV9mZW5jZXMgJiYgIWluLnN5bmNf ZmVuY2VfaW5mbykKPiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9zdGFnaW5nL2FuZHJvaWQvdWFwaS9z eW5jLmggYi9kcml2ZXJzL3N0YWdpbmcvYW5kcm9pZC91YXBpL3N5bmMuaAo+IGluZGV4IDlhYWQ2 MjMuLmY1NmE2YzIgMTAwNjQ0Cj4gLS0tIGEvZHJpdmVycy9zdGFnaW5nL2FuZHJvaWQvdWFwaS9z eW5jLmgKPiArKysgYi9kcml2ZXJzL3N0YWdpbmcvYW5kcm9pZC91YXBpL3N5bmMuaAo+IEBAIC0x OSwxMSArMTksMTMgQEAKPiAgICogQGZkMjogICAgICAgZmlsZSBkZXNjcmlwdG9yIG9mIHNlY29u ZCBmZW5jZQo+ICAgKiBAbmFtZTogICAgICBuYW1lIG9mIG5ldyBmZW5jZQo+ICAgKiBAZmVuY2U6 ICAgICByZXR1cm5zIHRoZSBmZCBvZiB0aGUgbmV3IGZlbmNlIHRvIHVzZXJzcGFjZQo+ICsgKiBA ZmxhZ3M6ICAgICBtZXJnZV9kYXRhIGZsYWdzCj4gICAqLwo+ICBzdHJ1Y3Qgc3luY19tZXJnZV9k YXRhIHsKPiAgICAgICAgIF9fczMyICAgZmQyOwo+ICAgICAgICAgY2hhciAgICBuYW1lWzMyXTsK PiAgICAgICAgIF9fczMyICAgZmVuY2U7Cj4gKyAgICAgICBfX3UzMiAgIGZsYWdzOwpUaGUgb3Zl cmFsbCBzaXplIG9mIHRoZSBzdHJ1Y3QgaXMgbm90IG11bHRpcGxlIG9mIDY0Yml0LCBzbyB0aGlu Z3MKd2lsbCBlbmQgdXAgYmFkbHkgaWYgd2UgZGVjaWRlIHRvIGV4dGVuZCBpdCBpbiB0aGUgZnV0 dXJlLiBFdmVuIGlmCnRoZXJlJ3MgYSBzbWFsbCBjaGFuY2UgdGhhdCB1cGRhdGUgd2lsbCBiZSBu ZWVkZWQsIHdlIG1pZ2h0IGFzIHdlbGwKcGFkIGl0IG5vdyAoYW5kIGNoZWNrIHRoZSBwYWRkaW5n IGZvciB6ZXJvLCByZXR1cm5pbmcgLUVJTlZBTCkuCgo+ICB9Owo+Cj4gIC8qKgo+IEBAIC0zMSwx MiArMzMsMTQgQEAgc3RydWN0IHN5bmNfbWVyZ2VfZGF0YSB7Cj4gICAqIEBvYmpfbmFtZTogICAg ICAgICAgbmFtZSBvZiBwYXJlbnQgc3luY190aW1lbGluZQo+ICAgKiBAZHJpdmVyX25hbWU6ICAg ICAgIG5hbWUgb2YgZHJpdmVyIGltcGxlbWVudGluZyB0aGUgcGFyZW50Cj4gICAqIEBzdGF0dXM6 ICAgICAgICAgICAgc3RhdHVzIG9mIHRoZSBmZW5jZSAwOmFjdGl2ZSAxOnNpZ25hbGVkIDwwOmVy cm9yCj4gKyAqIEBmbGFnczogICAgICAgICAgICAgZmVuY2VfaW5mbyBmbGFncwo+ICAgKiBAdGlt ZXN0YW1wX25zOiAgICAgIHRpbWVzdGFtcCBvZiBzdGF0dXMgY2hhbmdlIGluIG5hbm9zZWNvbmRz Cj4gICAqLwo+ICBzdHJ1Y3Qgc3luY19mZW5jZV9pbmZvIHsKPiAgICAgICAgIGNoYXIgICAgb2Jq X25hbWVbMzJdOwo+ICAgICAgICAgY2hhciAgICBkcml2ZXJfbmFtZVszMl07Cj4gICAgICAgICBf X3MzMiAgIHN0YXR1czsKPiArICAgICAgIF9fdTMyICAgZmxhZ3M7Cj4gICAgICAgICBfX3U2NCAg IHRpbWVzdGFtcF9uczsKU2hvdWxkIHdlIGJlIGRvaW5nIHNvbWUgZm9ybSBvZiB2YWxpZGF0aW9u IGluIHN5bmNfZmlsbF9mZW5jZV9pbmZvKCkKb2YgJ2ZsYWdzJyA/CgpSZWdhcmRzLApFbWlsCl9f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCmRyaS1kZXZlbCBt YWlsaW5nIGxpc3QKZHJpLWRldmVsQGxpc3RzLmZyZWVkZXNrdG9wLm9yZwpodHRwczovL2xpc3Rz LmZyZWVkZXNrdG9wLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2RyaS1kZXZlbAo=