* [GIT PULL] Cacheflush updates for 3.12
@ 2013-08-12 17:31 Will Deacon
2013-12-04 15:37 ` Christian Gmeiner
0 siblings, 1 reply; 9+ messages in thread
From: Will Deacon @ 2013-08-12 17:31 UTC (permalink / raw)
To: linux-arm-kernel
Hello again,
Please pull the following user-cacheflush updates for 3.12. This series both
improves performance of cacheflush-heavy workloads (i.e. browser benchmarks)
and also addresses a DoS issue on non-preemptible systems.
Will
--->8
The following changes since commit 3b2f64d00c46e1e4e9bd0bb9bb12619adac27a4b:
Linux 3.11-rc2 (2013-07-21 12:05:29 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/will/linux.git for-rmk/cacheflush
for you to fetch changes up to d3065541b5a95e5d0ef37e0dc8ada80fd1fba963:
ARM: cacheflush: don't bother rounding to nearest vma (2013-07-22 10:49:00 +0100)
----------------------------------------------------------------
Will Deacon (4):
ARM: entry: allow ARM-private syscalls to be restarted
ARM: cacheflush: split user cache-flushing into interruptible chunks
ARM: cacheflush: don't round address range up to nearest page
ARM: cacheflush: don't bother rounding to nearest vma
arch/arm/include/asm/cacheflush.h | 3 +--
arch/arm/include/asm/thread_info.h | 11 +++++++++++
arch/arm/kernel/entry-common.S | 4 ++--
arch/arm/kernel/traps.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++++---------------
4 files changed, 63 insertions(+), 19 deletions(-)
^ permalink raw reply [flat|nested] 9+ messages in thread* [GIT PULL] Cacheflush updates for 3.12 2013-08-12 17:31 [GIT PULL] Cacheflush updates for 3.12 Will Deacon @ 2013-12-04 15:37 ` Christian Gmeiner 2013-12-04 16:13 ` Will Deacon 0 siblings, 1 reply; 9+ messages in thread From: Christian Gmeiner @ 2013-12-04 15:37 UTC (permalink / raw) To: linux-arm-kernel 2013/8/12 Will Deacon <will.deacon@arm.com>: > Hello again, > > Please pull the following user-cacheflush updates for 3.12. This series both > improves performance of cacheflush-heavy workloads (i.e. browser benchmarks) > and also addresses a DoS issue on non-preemptible systems. > > Will > > --->8 > > The following changes since commit 3b2f64d00c46e1e4e9bd0bb9bb12619adac27a4b: > > Linux 3.11-rc2 (2013-07-21 12:05:29 -0700) > > are available in the git repository at: > > git://git.kernel.org/pub/scm/linux/kernel/git/will/linux.git for-rmk/cacheflush > > for you to fetch changes up to d3065541b5a95e5d0ef37e0dc8ada80fd1fba963: > > ARM: cacheflush: don't bother rounding to nearest vma (2013-07-22 10:49:00 +0100) > > ---------------------------------------------------------------- > Will Deacon (4): > ARM: entry: allow ARM-private syscalls to be restarted > ARM: cacheflush: split user cache-flushing into interruptible chunks > ARM: cacheflush: don't round address range up to nearest page > ARM: cacheflush: don't bother rounding to nearest vma > Hi all. I spend the last day running a bisect and I think I have found a problem :) I have a simple automated test case running, which looks like this: imx6d based device running X, chromium and x11vnc <----> windows pc connected via VNC to the device. With this patchset applyed the browser tab crashed after about 5 minutes hitting the F5/refresh button every 1-3 seconds. The last good kernel is 3.11.10 and the first bad one is 3.12. This is the result of my bisect: root at arm:~/kernel/linux-3.12.2# cat ~/bisect.log Bisecting: a merge base must be tested [6e4664525b1db28f8c4e1130957f70a94c19213e] Linux 3.11 Bisecting: 5700 revisions left to test after this (roughly 13 steps) [cc998ff8811530be521f6b316f37ab7676a07938] Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next Bisecting: 2850 revisions left to test after this (roughly 12 steps) [f9121153fdfbfaa930bf65077a5597e20d3ac608] mm/hwpoison: don't need to hold compound lock for hugetlbfs page Bisecting: 1428 revisions left to test after this (roughly 11 steps) [327fff3e1391a27dcc89de6e0481689a865361c9] Merge branch 'misc' of git://git.kernel.org/pub/scm/linux/kernel/git/mmarek/kbuild Bisecting: 775 revisions left to test after this (roughly 10 steps) [8e73e367f7dc50f1d1bc22a63e5764bb4eea9b48] Merge tag 'cleanup-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc Bisecting: 247 revisions left to test after this (roughly 8 steps) [39eda2aba6be642b71f2e0ad623dcb09fd9d79cf] Merge branch 'next' of git://git.kernel.org/pub/scm/linux/kernel/git/benh/powerpc Bisecting: 205 revisions left to test after this (roughly 8 steps) [22e04f6b4b04a8afe9af9239224591d06ba3b24d] Merge branch 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/jikos/hid Bisecting: 110 revisions left to test after this (roughly 7 steps) [2e032852245b3dcfe5461d7353e34eb6da095ccf] Merge branch 'for-linus' of git://git.linaro.org/people/rmk/linux-arm Bisecting: 55 revisions left to test after this (roughly 5 steps) [5cc91e0460889c8461620904968e193dddb1beb3] Merge branch 'for-rmk/cacheflush-v2' of git://git.kernel.org/pub/scm/linux/kernel/git/will/linux into devel-stable Bisecting: 11 revisions left to test after this (roughly 4 steps) [6af396a6b6c698eb3834184518fc9a59bc22c817] ARM: cacheflush: use -ishst dsb variant for ensuring flush completion Bisecting: 5 revisions left to test after this (roughly 3 steps) [b4f656eea63376da79b0b5a17660c4ce14b71b74] Pull branch 'for-rmk' of git://git.linaro.org/people/ardbiesheuvel/linux-arm into devel-stable Bisecting: 2 revisions left to test after this (roughly 2 steps) [d9524dc32cab52714dee0c8e59c7437ee33a239a] ARM: cacheflush: don't round address range up to nearest page Bisecting: 0 revisions left to test after this (roughly 1 step) [28256d612726a28a8b9d3c49f2b74198c4423d6a] ARM: cacheflush: split user cache-flushing into interruptible chunks Bisecting: 0 revisions left to test after this (roughly 0 steps) [377747c40657eb35ad98a56439606d96a928425a] ARM: entry: allow ARM-private syscalls to be restarted 28256d612726a28a8b9d3c49f2b74198c4423d6a is the first bad commit commit 28256d612726a28a8b9d3c49f2b74198c4423d6a Author: Will Deacon <will.deacon@arm.com> Date: Mon May 13 15:21:49 2013 +0100 ARM: cacheflush: split user cache-flushing into interruptible chunks Flushing a large, non-faulting VMA from userspace can potentially result in a long time spent flushing the cache line-by-line without preemption occurring (in the case of CONFIG_PREEMPT=n). Whilst this doesn't affect the stability of the system, it can certainly affect the responsiveness and CPU availability for other tasks. This patch splits up the user cacheflush code so that it flushes in chunks of a page. After each chunk has been flushed, we may reschedule if appropriate and, before processing the next chunk, we allow any pending signals to be handled before resuming from where we left off. Signed-off-by: Will Deacon <will.deacon@arm.com> :040000 040000 33ebf747dde46884ce4e7d4ce922fef3cd5b580e 22cdb8a0bc6dc72cb92d93c13ed1a45081269f77 M arch If I revert 28256d612726a28a8b9d3c49f2b74198c4423d6a and 97c72d89ce0ec8c73f19d5e35ec1f90f7a14bed7 my "test" runs hours. What debug options should I enable to get meaningful output from the kernel? greets -- Christian Gmeiner, MSc ^ permalink raw reply [flat|nested] 9+ messages in thread
* [GIT PULL] Cacheflush updates for 3.12 2013-12-04 15:37 ` Christian Gmeiner @ 2013-12-04 16:13 ` Will Deacon 2013-12-05 9:55 ` Jon Medhurst (Tixy) 2013-12-05 14:23 ` Jon Medhurst (Tixy) 0 siblings, 2 replies; 9+ messages in thread From: Will Deacon @ 2013-12-04 16:13 UTC (permalink / raw) To: linux-arm-kernel On Wed, Dec 04, 2013 at 03:37:36PM +0000, Christian Gmeiner wrote: > 2013/8/12 Will Deacon <will.deacon@arm.com>: > > Please pull the following user-cacheflush updates for 3.12. This series both > > improves performance of cacheflush-heavy workloads (i.e. browser benchmarks) > > and also addresses a DoS issue on non-preemptible systems. [...] > Hi all. Hello, > I spend the last day running a bisect and I think I have found a problem :) > > I have a simple automated test case running, which looks like this: > > imx6d based device running X, chromium and x11vnc <----> windows pc connected > via VNC to the device. With this patchset applyed the browser tab > crashed after about > 5 minutes hitting the F5/refresh button every 1-3 seconds. Hmm... it would be great if we had a simpler way to reproduce this, but ok. How many cores do you have on your IMX6? Also, how does the browser tab crash? Does it receive a SIGILL? > 28256d612726a28a8b9d3c49f2b74198c4423d6a is the first bad commit > commit 28256d612726a28a8b9d3c49f2b74198c4423d6a > Author: Will Deacon <will.deacon@arm.com> > Date: Mon May 13 15:21:49 2013 +0100 > > ARM: cacheflush: split user cache-flushing into interruptible chunks > > Flushing a large, non-faulting VMA from userspace can potentially result > in a long time spent flushing the cache line-by-line without preemption > occurring (in the case of CONFIG_PREEMPT=n). > > Whilst this doesn't affect the stability of the system, it can certainly > affect the responsiveness and CPU availability for other tasks. > > This patch splits up the user cacheflush code so that it flushes in > chunks of a page. After each chunk has been flushed, we may reschedule > if appropriate and, before processing the next chunk, we allow any > pending signals to be handled before resuming from where we left off. > > Signed-off-by: Will Deacon <will.deacon@arm.com> I took another look at that patch and can't see anything obviously wrong with it. It may, however, be exposing bugs in userspace that you would struggle to hit before. > :040000 040000 33ebf747dde46884ce4e7d4ce922fef3cd5b580e > 22cdb8a0bc6dc72cb92d93c13ed1a45081269f77 M arch > > > If I revert 28256d612726a28a8b9d3c49f2b74198c4423d6a and > 97c72d89ce0ec8c73f19d5e35ec1f90f7a14bed7 my "test" runs hours. > > > What debug options should I enable to get meaningful output from the kernel? An strace log of the failing case would be good. Another thing you could try is commenting out the cond_resched in __do_cache_op and see if that helps. Will ^ permalink raw reply [flat|nested] 9+ messages in thread
* [GIT PULL] Cacheflush updates for 3.12 2013-12-04 16:13 ` Will Deacon @ 2013-12-05 9:55 ` Jon Medhurst (Tixy) 2013-12-05 14:23 ` Jon Medhurst (Tixy) 1 sibling, 0 replies; 9+ messages in thread From: Jon Medhurst (Tixy) @ 2013-12-05 9:55 UTC (permalink / raw) To: linux-arm-kernel On Wed, 2013-12-04 at 16:13 +0000, Will Deacon wrote: > On Wed, Dec 04, 2013 at 03:37:36PM +0000, Christian Gmeiner wrote: > > 2013/8/12 Will Deacon <will.deacon@arm.com>: > > > Please pull the following user-cacheflush updates for 3.12. This series both > > > improves performance of cacheflush-heavy workloads (i.e. browser benchmarks) > > > and also addresses a DoS issue on non-preemptible systems. > > [...] > > > Hi all. > > Hello, > > > I spend the last day running a bisect and I think I have found a problem :) > > > > I have a simple automated test case running, which looks like this: > > > > imx6d based device running X, chromium and x11vnc <----> windows pc connected > > via VNC to the device. With this patchset applyed the browser tab > > crashed after about > > 5 minutes hitting the F5/refresh button every 1-3 seconds. > > Hmm... it would be great if we had a simpler way to reproduce this, but ok. > How many cores do you have on your IMX6? Also, how does the browser tab crash? > Does it receive a SIGILL? I think I'm also seeing this problem with Linaro Android on vexpress. The latest Android version (KitKat) has moved to using the Chrome browser and it crashes very easily after just a few seconds use (with SIGSEGV's because execution jumped into kernel virtual memory range). The reason I think it's the same issue as talked about in this email is that after reading this I check a 3.10 kernel with the same Android image and that was fine. Then I tried a previously crashing 3.13-rc2 kernel with the hack below to undo $subject, and that stopped the crashes: diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c index dbf0923..ff58932 100644 --- a/arch/arm/kernel/traps.c +++ b/arch/arm/kernel/traps.c @@ -560,7 +560,7 @@ do_cache_op(unsigned long start, unsigned long end, int flags) if (!access_ok(VERIFY_READ, start, end - start)) return -EFAULT; - return __do_cache_op(start, end); + return flush_cache_user_range(start, end); } A Linaro Android vexpress build which shows the bug can de found at https://android-build.linaro.org/builds/~linaro-android/vexpress-linaro/#build=172 And the bug is being tracked at https://bugs.launchpad.net/linaro-android/+bug/1254750 (ignore comments on that report about serial console issues, they should have been on a different bug report) -- Tixy > > 28256d612726a28a8b9d3c49f2b74198c4423d6a is the first bad commit > > commit 28256d612726a28a8b9d3c49f2b74198c4423d6a > > Author: Will Deacon <will.deacon@arm.com> > > Date: Mon May 13 15:21:49 2013 +0100 > > > > ARM: cacheflush: split user cache-flushing into interruptible chunks > > > > Flushing a large, non-faulting VMA from userspace can potentially result > > in a long time spent flushing the cache line-by-line without preemption > > occurring (in the case of CONFIG_PREEMPT=n). > > > > Whilst this doesn't affect the stability of the system, it can certainly > > affect the responsiveness and CPU availability for other tasks. > > > > This patch splits up the user cacheflush code so that it flushes in > > chunks of a page. After each chunk has been flushed, we may reschedule > > if appropriate and, before processing the next chunk, we allow any > > pending signals to be handled before resuming from where we left off. > > > > Signed-off-by: Will Deacon <will.deacon@arm.com> > > I took another look at that patch and can't see anything obviously wrong > with it. It may, however, be exposing bugs in userspace that you would > struggle to hit before. > > > :040000 040000 33ebf747dde46884ce4e7d4ce922fef3cd5b580e > > 22cdb8a0bc6dc72cb92d93c13ed1a45081269f77 M arch > > > > > > If I revert 28256d612726a28a8b9d3c49f2b74198c4423d6a and > > 97c72d89ce0ec8c73f19d5e35ec1f90f7a14bed7 my "test" runs hours. > > > > > > What debug options should I enable to get meaningful output from the kernel? > > An strace log of the failing case would be good. Another thing you could try > is commenting out the cond_resched in __do_cache_op and see if that helps. > > Will > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel at lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [GIT PULL] Cacheflush updates for 3.12 2013-12-04 16:13 ` Will Deacon 2013-12-05 9:55 ` Jon Medhurst (Tixy) @ 2013-12-05 14:23 ` Jon Medhurst (Tixy) 2013-12-05 15:14 ` Jon Medhurst (Tixy) 1 sibling, 1 reply; 9+ messages in thread From: Jon Medhurst (Tixy) @ 2013-12-05 14:23 UTC (permalink / raw) To: linux-arm-kernel On Wed, 2013-12-04 at 16:13 +0000, Will Deacon wrote: > took another look at that patch and can't see anything obviously wrong > with it. If the memory region isn't guaranteed to be page aligned then doesn't it flush up to PAGE_SIZE-1 more bytes than requested and so exceed the bounds check in do_cache_op? Fixing this as below _appears_ to stop the Browser crashes I'm seeing (still doing some more testing)... diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c index dbf0923..cbc9dae 100644 --- a/arch/arm/kernel/traps.c +++ b/arch/arm/kernel/traps.c @@ -512,6 +512,10 @@ __do_cache_op(unsigned long start, unsigned long end) unsigned long chunk = PAGE_SIZE; do { + unsigned long chunk_end = start + chunk; + if (chunk_end > end) + chunk_end = end; + if (signal_pending(current)) { struct thread_info *ti = current_thread_info(); @@ -531,7 +535,7 @@ __do_cache_op(unsigned long start, unsigned long end) return -ERESTART_RESTARTBLOCK; } - ret = flush_cache_user_range(start, start + chunk); + ret = flush_cache_user_range(start, chunk_end); if (ret) return ret; -- Tixy ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [GIT PULL] Cacheflush updates for 3.12 2013-12-05 14:23 ` Jon Medhurst (Tixy) @ 2013-12-05 15:14 ` Jon Medhurst (Tixy) 2013-12-05 17:16 ` Christian Gmeiner 2013-12-06 11:50 ` Will Deacon 0 siblings, 2 replies; 9+ messages in thread From: Jon Medhurst (Tixy) @ 2013-12-05 15:14 UTC (permalink / raw) To: linux-arm-kernel On Thu, 2013-12-05 at 14:23 +0000, Jon Medhurst (Tixy) wrote: > On Wed, 2013-12-04 at 16:13 +0000, Will Deacon wrote: > > took another look at that patch and can't see anything obviously wrong > > with it. > > If the memory region isn't guaranteed to be page aligned then doesn't it > flush up to PAGE_SIZE-1 more bytes than requested and so exceed the > bounds check in do_cache_op? Fixing this as below _appears_ to stop the > Browser crashes I'm seeing (still doing some more testing)... Actually, a smaller patch and one which avoids the possibility of arithmetic owerflow is... From: Jon Medhurst <tixy@linaro.org> Date: Thu, 5 Dec 2013 14:29:24 +0000 Subject: [PATCH] ARM: cacheflush: correctly limit range of memory region being flushed The __do_cache_op function operates with a 'chunk' size of one page but fails to limit the size of the final chunk so as to not exceed the specified memory region. Fix this. Signed-off-by: Jon Medhurst <tixy@linaro.org> --- arch/arm/kernel/traps.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c index dbf0923..7940241 100644 --- a/arch/arm/kernel/traps.c +++ b/arch/arm/kernel/traps.c @@ -509,9 +509,10 @@ static inline int __do_cache_op(unsigned long start, unsigned long end) { int ret; - unsigned long chunk = PAGE_SIZE; do { + unsigned long chunk = min(PAGE_SIZE, end - start); + if (signal_pending(current)) { struct thread_info *ti = current_thread_info(); -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [GIT PULL] Cacheflush updates for 3.12 2013-12-05 15:14 ` Jon Medhurst (Tixy) @ 2013-12-05 17:16 ` Christian Gmeiner 2013-12-06 8:05 ` Christian Gmeiner 2013-12-06 11:50 ` Will Deacon 1 sibling, 1 reply; 9+ messages in thread From: Christian Gmeiner @ 2013-12-05 17:16 UTC (permalink / raw) To: linux-arm-kernel 2013/12/5 Jon Medhurst (Tixy) <tixy@linaro.org>: > On Thu, 2013-12-05 at 14:23 +0000, Jon Medhurst (Tixy) wrote: >> On Wed, 2013-12-04 at 16:13 +0000, Will Deacon wrote: >> > took another look at that patch and can't see anything obviously wrong >> > with it. >> >> If the memory region isn't guaranteed to be page aligned then doesn't it >> flush up to PAGE_SIZE-1 more bytes than requested and so exceed the >> bounds check in do_cache_op? Fixing this as below _appears_ to stop the >> Browser crashes I'm seeing (still doing some more testing)... > > Actually, a smaller patch and one which avoids the possibility of > arithmetic owerflow is... > > From: Jon Medhurst <tixy@linaro.org> > Date: Thu, 5 Dec 2013 14:29:24 +0000 > Subject: [PATCH] ARM: cacheflush: correctly limit range of memory region > being flushed > > The __do_cache_op function operates with a 'chunk' size of one page > but fails to limit the size of the final chunk so as to not exceed > the specified memory region. Fix this. > > Signed-off-by: Jon Medhurst <tixy@linaro.org> > --- > arch/arm/kernel/traps.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c > index dbf0923..7940241 100644 > --- a/arch/arm/kernel/traps.c > +++ b/arch/arm/kernel/traps.c > @@ -509,9 +509,10 @@ static inline int > __do_cache_op(unsigned long start, unsigned long end) > { > int ret; > - unsigned long chunk = PAGE_SIZE; > > do { > + unsigned long chunk = min(PAGE_SIZE, end - start); > + > if (signal_pending(current)) { > struct thread_info *ti = current_thread_info(); > > -- > 1.7.10.4 > > I am running my test overnight with this patch... stay tuned. greets -- Christian Gmeiner, MSc ^ permalink raw reply [flat|nested] 9+ messages in thread
* [GIT PULL] Cacheflush updates for 3.12 2013-12-05 17:16 ` Christian Gmeiner @ 2013-12-06 8:05 ` Christian Gmeiner 0 siblings, 0 replies; 9+ messages in thread From: Christian Gmeiner @ 2013-12-06 8:05 UTC (permalink / raw) To: linux-arm-kernel 2013/12/5 Christian Gmeiner <christian.gmeiner@gmail.com>: > 2013/12/5 Jon Medhurst (Tixy) <tixy@linaro.org>: >> On Thu, 2013-12-05 at 14:23 +0000, Jon Medhurst (Tixy) wrote: >>> On Wed, 2013-12-04 at 16:13 +0000, Will Deacon wrote: >>> > took another look at that patch and can't see anything obviously wrong >>> > with it. >>> >>> If the memory region isn't guaranteed to be page aligned then doesn't it >>> flush up to PAGE_SIZE-1 more bytes than requested and so exceed the >>> bounds check in do_cache_op? Fixing this as below _appears_ to stop the >>> Browser crashes I'm seeing (still doing some more testing)... >> >> Actually, a smaller patch and one which avoids the possibility of >> arithmetic owerflow is... >> >> From: Jon Medhurst <tixy@linaro.org> >> Date: Thu, 5 Dec 2013 14:29:24 +0000 >> Subject: [PATCH] ARM: cacheflush: correctly limit range of memory region >> being flushed >> >> The __do_cache_op function operates with a 'chunk' size of one page >> but fails to limit the size of the final chunk so as to not exceed >> the specified memory region. Fix this. >> >> Signed-off-by: Jon Medhurst <tixy@linaro.org> >> --- >> arch/arm/kernel/traps.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c >> index dbf0923..7940241 100644 >> --- a/arch/arm/kernel/traps.c >> +++ b/arch/arm/kernel/traps.c >> @@ -509,9 +509,10 @@ static inline int >> __do_cache_op(unsigned long start, unsigned long end) >> { >> int ret; >> - unsigned long chunk = PAGE_SIZE; >> >> do { >> + unsigned long chunk = min(PAGE_SIZE, end - start); >> + >> if (signal_pending(current)) { >> struct thread_info *ti = current_thread_info(); >> >> -- >> 1.7.10.4 >> >> > > I am running my test overnight with this patch... stay tuned. > My test is still running, which is good. Tested-by: Christian Gmeiner <christian.gmeiner@gmail.com> ^ permalink raw reply [flat|nested] 9+ messages in thread
* [GIT PULL] Cacheflush updates for 3.12 2013-12-05 15:14 ` Jon Medhurst (Tixy) 2013-12-05 17:16 ` Christian Gmeiner @ 2013-12-06 11:50 ` Will Deacon 1 sibling, 0 replies; 9+ messages in thread From: Will Deacon @ 2013-12-06 11:50 UTC (permalink / raw) To: linux-arm-kernel Hi guys, I was out-of-office yesterday, so sorry for the radio silence. On Thu, Dec 05, 2013 at 03:14:31PM +0000, Jon Medhurst (Tixy) wrote: > On Thu, 2013-12-05 at 14:23 +0000, Jon Medhurst (Tixy) wrote: > > On Wed, 2013-12-04 at 16:13 +0000, Will Deacon wrote: > > > took another look at that patch and can't see anything obviously wrong > > > with it. > > > > If the memory region isn't guaranteed to be page aligned then doesn't it > > flush up to PAGE_SIZE-1 more bytes than requested and so exceed the > > bounds check in do_cache_op? Fixing this as below _appears_ to stop the > > Browser crashes I'm seeing (still doing some more testing)... Well spotted! We used to round down to a page, but I fixed that in the same series and didn't notice I'd introduce a bug here. Thanks for digging. Interesting that we've not heard more reports of this... > Actually, a smaller patch and one which avoids the possibility of > arithmetic owerflow is... > > From: Jon Medhurst <tixy@linaro.org> > Date: Thu, 5 Dec 2013 14:29:24 +0000 > Subject: [PATCH] ARM: cacheflush: correctly limit range of memory region > being flushed > > The __do_cache_op function operates with a 'chunk' size of one page > but fails to limit the size of the final chunk so as to not exceed > the specified memory region. Fix this. > > Signed-off-by: Jon Medhurst <tixy@linaro.org> > --- > arch/arm/kernel/traps.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c > index dbf0923..7940241 100644 > --- a/arch/arm/kernel/traps.c > +++ b/arch/arm/kernel/traps.c > @@ -509,9 +509,10 @@ static inline int > __do_cache_op(unsigned long start, unsigned long end) > { > int ret; > - unsigned long chunk = PAGE_SIZE; > > do { > + unsigned long chunk = min(PAGE_SIZE, end - start); > + > if (signal_pending(current)) { > struct thread_info *ti = current_thread_info(); Acked-by: Will Deacon <will.deacon@arm.com> Please can you put this into the patch system, along with a CC stable? Cheers, Will ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2013-12-06 11:50 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-08-12 17:31 [GIT PULL] Cacheflush updates for 3.12 Will Deacon 2013-12-04 15:37 ` Christian Gmeiner 2013-12-04 16:13 ` Will Deacon 2013-12-05 9:55 ` Jon Medhurst (Tixy) 2013-12-05 14:23 ` Jon Medhurst (Tixy) 2013-12-05 15:14 ` Jon Medhurst (Tixy) 2013-12-05 17:16 ` Christian Gmeiner 2013-12-06 8:05 ` Christian Gmeiner 2013-12-06 11:50 ` Will Deacon
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).