[PATCHv2 0/7] Staging fixes from the Android tree (for 3.14)

[PATCHv2 0/7] Staging fixes from the Android tree (for 3.14)

[John Stultz]

I recently went through the AOSP common.git android/3.10 tree to
try to pull fixes that haven't been submitted upstream. I've
cherry picked those patches and submitted them for 3.15, but
Greg requested the bugfixes to be submitted by themselves for
3.14.

So here are the subset of changes that are bugfixes. There is
one additional patch here which I missed in my last submission,
but its a build warning fix, so I think its appropriate.

If and when these are merged, I'll resubmit the rest of the
queue (which has grown further as of today) for 3.15.

Anyway, please let me know if there's any feedback or suggestions.

thanks
-john


Cc: Greg KH <gregkh@linuxfoundation.org>
Cc: Colin Cross <ccross@android.com>
Cc: Greg Hackmann <ghackmann@google.com>
Cc: Prakash Kamliya <pkamliya@codeaurora.org>
Cc: Alistair Strachan <alistair.strachan@imgtec.com>
Cc: Todd Poynor <toddpoynor@google.com>
Cc: Laura Abbott <lauraa@codeaurora.org>
Cc: Android Kernel Team <kernel-team@android.com>



Alistair Strachan (1):
  staging: sync: Fix a race condition between release_obj and print_obj

Colin Cross (1):
  staging: ion: Fix overflow and list bugs in system heap

Greg Hackmann (1):
  staging: sw_sync: Add stubs for kernels without CONFIG_SW_SYNC

John Stultz (1):
  staging: ion: Fix build warning

Laura Abbott (1):
  staging: ion: Fix ION_IOC_FREE compat ioctl

Prakash Kamliya (1):
  staging: sync: Signal pt before sync_timeline object gets destroyed

Todd Poynor (1):
  staging: ashmem: Avoid deadlock between read and mmap calls

 drivers/staging/android/ashmem.c              | 45 ++++++++++++++++-----------
 drivers/staging/android/ion/compat_ion.c      | 26 +++++++++++++---
 drivers/staging/android/ion/ion_priv.h        |  1 +
 drivers/staging/android/ion/ion_system_heap.c |  6 +++-
 drivers/staging/android/sw_sync.h             | 17 ++++++++++
 drivers/staging/android/sync.c                | 14 ++++-----
 6 files changed, 78 insertions(+), 31 deletions(-)

-- 
1.8.3.2

[PATCH 1/7] staging: sw_sync: Add stubs for kernels without CONFIG_SW_SYNC

[John Stultz]

From: Greg Hackmann <ghackmann@google.com>

Add stubs for kernels without CONFIG_SW_SYNC

Cc: Greg KH <gregkh@linuxfoundation.org>
Cc: Colin Cross <ccross@android.com>
Cc: Android Kernel Team <kernel-team@android.com>
Signed-off-by: Greg Hackmann <ghackmann@google.com>
[jstultz: resolved minor conflict, tweaked commit message]
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 drivers/staging/android/sw_sync.h | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/staging/android/sw_sync.h b/drivers/staging/android/sw_sync.h
index 585040b..5aaf71d 100644
--- a/drivers/staging/android/sw_sync.h
+++ b/drivers/staging/android/sw_sync.h
@@ -35,10 +35,27 @@ struct sw_sync_pt {
 	u32			value;
 };
 
+#if IS_ENABLED(CONFIG_SW_SYNC)
 struct sw_sync_timeline *sw_sync_timeline_create(const char *name);
 void sw_sync_timeline_inc(struct sw_sync_timeline *obj, u32 inc);
 
 struct sync_pt *sw_sync_pt_create(struct sw_sync_timeline *obj, u32 value);
+#else
+static inline struct sw_sync_timeline *sw_sync_timeline_create(const char *name)
+{
+	return NULL;
+}
+
+static inline void sw_sync_timeline_inc(struct sw_sync_timeline *obj, u32 inc)
+{
+}
+
+static inline struct sync_pt *sw_sync_pt_create(struct sw_sync_timeline *obj,
+		u32 value)
+{
+	return NULL;
+}
+#endif /* IS_ENABLED(CONFIG_SW_SYNC) */
 
 #endif /* __KERNEL __ */
 
-- 
1.8.3.2

[PATCH 2/7] staging: sync: Signal pt before sync_timeline object gets destroyed

[John Stultz]

From: Prakash Kamliya <pkamliya@codeaurora.org>

There is a race condition

Assume we have *one* sync_fence object, with *one* sync_pt
which belongs to *one* sync_timeline, given this condition,
sync_timeline->kref will have two counts, one for sync_timeline
(implicit) and another for sync_pt.

Assume following is the situation on CPU

Theead-1 : (Thread which calls sync_timeline_destroy())
  -> (some function calls)
   -> sync_timeline_destory()
    -> sync_timeline_signal() (CPU is inside this
function after putting reference to sync_timeline)

At this time Thread-2 comes and does following

Thread-2 : (fclose on fence fd)
> sync_fence_release() -> because of fclose() on fence object
 -> sync_fence_free()
  -> sync_pt_free()
   -> kref_put(&pt->parent->kref, sync_timeline_free);
    -> sync_timeline_free() (CPU is inside this because
this time kref will be zero after _put)

Thread-2 will free sync_timeline object before Thread-1
has finished its work inside sync_timeline_signal.

With this change we signals all sync_pt before putting
reference to sync_timeline object.

c: Greg KH <gregkh@linuxfoundation.org>
Cc: Colin Cross <ccross@android.com>
Cc: Android Kernel Team <kernel-team@android.com>
Signed-off-by: Prakash Kamliya <pkamliya@codeaurora.org>
[jstultz: minor commit subject tweak]
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 drivers/staging/android/sync.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c
index 38e5d3b..fec2d1c 100644
--- a/drivers/staging/android/sync.c
+++ b/drivers/staging/android/sync.c
@@ -92,14 +92,14 @@ static void sync_timeline_free(struct kref *kref)
 void sync_timeline_destroy(struct sync_timeline *obj)
 {
 	obj->destroyed = true;
+	smp_wmb();
 
 	/*
-	 * If this is not the last reference, signal any children
-	 * that their parent is going away.
+	 * signal any children that their parent is going away.
 	 */
+	sync_timeline_signal(obj);
 
-	if (!kref_put(&obj->kref, sync_timeline_free))
-		sync_timeline_signal(obj);
+	kref_put(&obj->kref, sync_timeline_free);
 }
 EXPORT_SYMBOL(sync_timeline_destroy);
 
-- 
1.8.3.2

[PATCH 3/7] staging: sync: Fix a race condition between release_obj and print_obj

[John Stultz]

From: Alistair Strachan <alistair.strachan@imgtec.com>

Before this change, a timeline would only be removed from the timeline
list *after* the sync driver had its release_obj() called. However, the
driver's release_obj() may free resources needed by print_obj().

Although the timeline list is locked when print_obj() is called, it is
not locked when release_obj() is called. If one CPU was in print_obj()
when another was in release_obj(), the print_obj() may make unsafe
accesses.

It is not actually necessary to hold the timeline list lock when calling
release_obj() if the call is made after the timeline is unlinked from
the list, since there is no possibility another thread could be in --
or enter -- print_obj() for that timeline.

This change moves the release_obj() call to after the timeline is
unlinked, preventing the above race from occurring.

Cc: Greg KH <gregkh@linuxfoundation.org>
Cc: Colin Cross <ccross@android.com>
Cc: Android Kernel Team <kernel-team@android.com>
Signed-off-by: Alistair Strachan <alistair.strachan@imgtec.com>
[jstultz: minor commit subject tweak]
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 drivers/staging/android/sync.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c
index fec2d1c..3d05f662 100644
--- a/drivers/staging/android/sync.c
+++ b/drivers/staging/android/sync.c
@@ -79,13 +79,13 @@ static void sync_timeline_free(struct kref *kref)
 		container_of(kref, struct sync_timeline, kref);
 	unsigned long flags;
 
-	if (obj->ops->release_obj)
-		obj->ops->release_obj(obj);
-
 	spin_lock_irqsave(&sync_timeline_list_lock, flags);
 	list_del(&obj->sync_timeline_list);
 	spin_unlock_irqrestore(&sync_timeline_list_lock, flags);
 
+	if (obj->ops->release_obj)
+		obj->ops->release_obj(obj);
+
 	kfree(obj);
 }
 
-- 
1.8.3.2

[PATCH 4/7] staging: ashmem: Avoid deadlock between read and mmap calls

[John Stultz]

From: Todd Poynor <toddpoynor@google.com>

Avoid holding ashmem_mutex across code that can page fault.  Page faults
grab the mmap_sem for the process, which are also held by mmap calls
prior to calling ashmem_mmap, which locks ashmem_mutex.  The reversed
order of locking between the two can deadlock.

The calls that can page fault are read() and the ASHMEM_SET_NAME and
ASHMEM_GET_NAME ioctls.  Move the code that accesses userspace pages
outside the ashmem_mutex.

Cc: Greg KH <gregkh@linuxfoundation.org>
Cc: Colin Cross <ccross@android.com>
Cc: Android Kernel Team <kernel-team@android.com>
Signed-off-by: Todd Poynor <toddpoynor@google.com>
[jstultz: minor commit message tweaks]
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 drivers/staging/android/ashmem.c | 45 +++++++++++++++++++++++-----------------
 1 file changed, 26 insertions(+), 19 deletions(-)

diff --git a/drivers/staging/android/ashmem.c b/drivers/staging/android/ashmem.c
index 23948f1..713a972 100644
--- a/drivers/staging/android/ashmem.c
+++ b/drivers/staging/android/ashmem.c
@@ -295,21 +295,29 @@ static ssize_t ashmem_read(struct file *file, char __user *buf,
 
 	/* If size is not set, or set to 0, always return EOF. */
 	if (asma->size == 0)
-		goto out;
+		goto out_unlock;
 
 	if (!asma->file) {
 		ret = -EBADF;
-		goto out;
+		goto out_unlock;
 	}
 
-	ret = asma->file->f_op->read(asma->file, buf, len, pos);
-	if (ret < 0)
-		goto out;
+	mutex_unlock(&ashmem_mutex);
 
-	/** Update backing file pos, since f_ops->read() doesn't */
-	asma->file->f_pos = *pos;
+	/*
+	 * asma and asma->file are used outside the lock here.  We assume
+	 * once asma->file is set it will never be changed, and will not
+	 * be destroyed until all references to the file are dropped and
+	 * ashmem_release is called.
+	 */
+	ret = asma->file->f_op->read(asma->file, buf, len, pos);
+	if (ret >= 0) {
+		/** Update backing file pos, since f_ops->read() doesn't */
+		asma->file->f_pos = *pos;
+	}
+	return ret;
 
-out:
+out_unlock:
 	mutex_unlock(&ashmem_mutex);
 	return ret;
 }
@@ -498,6 +506,7 @@ out:
 
 static int set_name(struct ashmem_area *asma, void __user *name)
 {
+	int len;
 	int ret = 0;
 	char local_name[ASHMEM_NAME_LEN];
 
@@ -510,21 +519,19 @@ static int set_name(struct ashmem_area *asma, void __user *name)
 	 * variable that does not need protection and later copy the local
 	 * variable to the structure member with lock held.
 	 */
-	if (copy_from_user(local_name, name, ASHMEM_NAME_LEN))
-		return -EFAULT;
-
+	len = strncpy_from_user(local_name, name, ASHMEM_NAME_LEN);
+	if (len < 0)
+		return len;
+	if (len == ASHMEM_NAME_LEN)
+		local_name[ASHMEM_NAME_LEN - 1] = '\0';
 	mutex_lock(&ashmem_mutex);
 	/* cannot change an existing mapping's name */
-	if (unlikely(asma->file)) {
+	if (unlikely(asma->file))
 		ret = -EINVAL;
-		goto out;
-	}
-	memcpy(asma->name + ASHMEM_NAME_PREFIX_LEN,
-		local_name, ASHMEM_NAME_LEN);
-	asma->name[ASHMEM_FULL_NAME_LEN-1] = '\0';
-out:
-	mutex_unlock(&ashmem_mutex);
+	else
+		strcpy(asma->name + ASHMEM_NAME_PREFIX_LEN, local_name);
 
+	mutex_unlock(&ashmem_mutex);
 	return ret;
 }
 
-- 
1.8.3.2

[PATCH 5/7] staging: ion: Fix overflow and list bugs in system heap

[John Stultz]

From: Colin Cross <ccross@android.com>

Fix a few bugs in ion_system_heap:

Initialize the list node in the info block.

Don't store size_remaining in a signed long, allocating >2GB
could overflow, resulting in a call to sg_alloc_table with
nents=0 which panics.  alloc_largest_available will never
return a block larger than size_remanining, so it can never
go negative.

Limit a single allocation to half of all memory.  Prevents a
large allocation from taking down the whole system.

Cc: Greg KH <gregkh@linuxfoundation.org>
Cc: Colin Cross <ccross@android.com>
Cc: Android Kernel Team <kernel-team@android.com>
Signed-off-by: Colin Cross <ccross@android.com>
[jstultz: Minor commit subject tweak]
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 drivers/staging/android/ion/ion_system_heap.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/android/ion/ion_system_heap.c b/drivers/staging/android/ion/ion_system_heap.c
index 7f07291..9849f39 100644
--- a/drivers/staging/android/ion/ion_system_heap.c
+++ b/drivers/staging/android/ion/ion_system_heap.c
@@ -124,6 +124,7 @@ static struct page_info *alloc_largest_available(struct ion_system_heap *heap,
 
 		info->page = page;
 		info->order = orders[i];
+		INIT_LIST_HEAD(&info->list);
 		return info;
 	}
 	kfree(info);
@@ -145,12 +146,15 @@ static int ion_system_heap_allocate(struct ion_heap *heap,
 	struct list_head pages;
 	struct page_info *info, *tmp_info;
 	int i = 0;
-	long size_remaining = PAGE_ALIGN(size);
+	unsigned long size_remaining = PAGE_ALIGN(size);
 	unsigned int max_order = orders[0];
 
 	if (align > PAGE_SIZE)
 		return -EINVAL;
 
+	if (size / PAGE_SIZE > totalram_pages / 2)
+		return -ENOMEM;
+
 	INIT_LIST_HEAD(&pages);
 	while (size_remaining > 0) {
 		info = alloc_largest_available(sys_heap, buffer, size_remaining,
-- 
1.8.3.2

[PATCH 6/7] staging: ion: Fix ION_IOC_FREE compat ioctl

[John Stultz]

From: Laura Abbott <lauraa@codeaurora.org>

The compat ioctl for ION_IOC_FREE currently passes allocation data
instead of the free data. Correct this.

Cc: Greg KH <gregkh@linuxfoundation.org>
Cc: Colin Cross <ccross@android.com>
Cc: Android Kernel Team <kernel-team@android.com>
Signed-off-by: Laura Abbott <lauraa@codeaurora.org>
[jstultz: Folded in a small build fix]
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 drivers/staging/android/ion/compat_ion.c | 26 ++++++++++++++++++++++----
 1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/android/ion/compat_ion.c b/drivers/staging/android/ion/compat_ion.c
index af6cd37..ee3a738 100644
--- a/drivers/staging/android/ion/compat_ion.c
+++ b/drivers/staging/android/ion/compat_ion.c
@@ -35,9 +35,14 @@ struct compat_ion_custom_data {
 	compat_ulong_t arg;
 };
 
+struct compat_ion_handle_data {
+	compat_int_t handle;
+};
+
 #define COMPAT_ION_IOC_ALLOC	_IOWR(ION_IOC_MAGIC, 0, \
 				      struct compat_ion_allocation_data)
-#define COMPAT_ION_IOC_FREE	_IOWR(ION_IOC_MAGIC, 1, struct ion_handle_data)
+#define COMPAT_ION_IOC_FREE	_IOWR(ION_IOC_MAGIC, 1, \
+				      struct compat_ion_handle_data)
 #define COMPAT_ION_IOC_CUSTOM	_IOWR(ION_IOC_MAGIC, 6, \
 				      struct compat_ion_custom_data)
 
@@ -64,6 +69,19 @@ static int compat_get_ion_allocation_data(
 	return err;
 }
 
+static int compat_get_ion_handle_data(
+			struct compat_ion_handle_data __user *data32,
+			struct ion_handle_data __user *data)
+{
+	compat_int_t i;
+	int err;
+
+	err = get_user(i, &data32->handle);
+	err |= put_user(i, &data->handle);
+
+	return err;
+}
+
 static int compat_put_ion_allocation_data(
 			struct compat_ion_allocation_data __user *data32,
 			struct ion_allocation_data __user *data)
@@ -132,8 +150,8 @@ long compat_ion_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 	}
 	case COMPAT_ION_IOC_FREE:
 	{
-		struct compat_ion_allocation_data __user *data32;
-		struct ion_allocation_data __user *data;
+		struct compat_ion_handle_data __user *data32;
+		struct ion_handle_data __user *data;
 		int err;
 
 		data32 = compat_ptr(arg);
@@ -141,7 +159,7 @@ long compat_ion_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 		if (data == NULL)
 			return -EFAULT;
 
-		err = compat_get_ion_allocation_data(data32, data);
+		err = compat_get_ion_handle_data(data32, data);
 		if (err)
 			return err;
 
-- 
1.8.3.2

[PATCH 7/7] staging: ion: Fix build warning

[John Stultz]

Add #include <linux/device.h> to fix the following warning seen
with gcc 4.7.3:

In file included from drivers/staging/android/ion/ion_heap.c:26:0:
drivers/staging/android/ion/ion_priv.h:358:21: warning: ‘struct device’ declared inside parameter list [enabled by default]
drivers/staging/android/ion/ion_priv.h:358:21: warning: its scope is only this definition or declaration, which is probably not what you want [enabled by default]

Cc: Greg KH <gregkh@linuxfoundation.org>
Cc: Colin Cross <ccross@android.com>
Cc: Android Kernel Team <kernel-team@android.com>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 drivers/staging/android/ion/ion_priv.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/staging/android/ion/ion_priv.h b/drivers/staging/android/ion/ion_priv.h
index d986739..fc2e4fc 100644
--- a/drivers/staging/android/ion/ion_priv.h
+++ b/drivers/staging/android/ion/ion_priv.h
@@ -17,6 +17,7 @@
 #ifndef _ION_PRIV_H
 #define _ION_PRIV_H
 
+#include <linux/device.h>
 #include <linux/dma-direction.h>
 #include <linux/kref.h>
 #include <linux/mm_types.h>
-- 
1.8.3.2

Re: [PATCHv2 0/7] Staging fixes from the Android tree (for 3.14)

[Greg KH]

On Tue, Feb 04, 2014 at 04:08:33PM -0800, John Stultz wrote:
> I recently went through the AOSP common.git android/3.10 tree to
> try to pull fixes that haven't been submitted upstream. I've
> cherry picked those patches and submitted them for 3.15, but
> Greg requested the bugfixes to be submitted by themselves for
> 3.14.
> 
> So here are the subset of changes that are bugfixes. There is
> one additional patch here which I missed in my last submission,
> but its a build warning fix, so I think its appropriate.
> 
> If and when these are merged, I'll resubmit the rest of the
> queue (which has grown further as of today) for 3.15.
> 
> Anyway, please let me know if there's any feedback or suggestions.

I seem to be missing patch 2/7 here.  Any idea where it went?

I've applied the other 6 to my tree now, thanks.

greg k-h

Re: [PATCHv2 0/7] Staging fixes from the Android tree (for 3.14)

[John Stultz]

On 02/07/2014 09:03 AM, Greg KH wrote:
> On Tue, Feb 04, 2014 at 04:08:33PM -0800, John Stultz wrote:
>> I recently went through the AOSP common.git android/3.10 tree to
>> try to pull fixes that haven't been submitted upstream. I've
>> cherry picked those patches and submitted them for 3.15, but
>> Greg requested the bugfixes to be submitted by themselves for
>> 3.14.
>>
>> So here are the subset of changes that are bugfixes. There is
>> one additional patch here which I missed in my last submission,
>> but its a build warning fix, so I think its appropriate.
>>
>> If and when these are merged, I'll resubmit the rest of the
>> queue (which has grown further as of today) for 3.15.
>>
>> Anyway, please let me know if there's any feedback or suggestions.
> I seem to be missing patch 2/7 here.  Any idea where it went?

Huh. I see it in my lkml archive. The subject is:
[PATCH 2/7] staging: sync: Signal pt before sync_timeline object gets
destroyed

https://lkml.org/lkml/2014/2/4/738

Maybe check your spam folder? If you need I can bounce it to you again.

thanks
-john

Re: [PATCHv2 0/7] Staging fixes from the Android tree (for 3.14)

[Greg KH]

On Fri, Feb 07, 2014 at 09:16:46AM -0800, John Stultz wrote:
> On 02/07/2014 09:03 AM, Greg KH wrote:
> > On Tue, Feb 04, 2014 at 04:08:33PM -0800, John Stultz wrote:
> >> I recently went through the AOSP common.git android/3.10 tree to
> >> try to pull fixes that haven't been submitted upstream. I've
> >> cherry picked those patches and submitted them for 3.15, but
> >> Greg requested the bugfixes to be submitted by themselves for
> >> 3.14.
> >>
> >> So here are the subset of changes that are bugfixes. There is
> >> one additional patch here which I missed in my last submission,
> >> but its a build warning fix, so I think its appropriate.
> >>
> >> If and when these are merged, I'll resubmit the rest of the
> >> queue (which has grown further as of today) for 3.15.
> >>
> >> Anyway, please let me know if there's any feedback or suggestions.
> > I seem to be missing patch 2/7 here.  Any idea where it went?
> 
> Huh. I see it in my lkml archive. The subject is:
> [PATCH 2/7] staging: sync: Signal pt before sync_timeline object gets
> destroyed
> 
> https://lkml.org/lkml/2014/2/4/738
> 
> Maybe check your spam folder? If you need I can bounce it to you again.

Ah, you forgot to cc: me, that's why I didn't get it :)

Got it now, thanks.

greg k-h