To eliminate code duplication lets introduce may_adjust_brk
helper which we will use in brk() and prctl() syscalls.
Signed-off-by: Cyrill Gorcunov <[email protected]>
Cc: Kees Cook <[email protected]>
Cc: Tejun Heo <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: Andrew Vagin <[email protected]>
Cc: Eric W. Biederman <[email protected]>
Cc: H. Peter Anvin <[email protected]>
Cc: Serge Hallyn <[email protected]>
Cc: Pavel Emelyanov <[email protected]>
Cc: Vasiliy Kulikov <[email protected]>
Cc: KAMEZAWA Hiroyuki <[email protected]>
Cc: Michael Kerrisk <[email protected]>
Cc: Julien Tinnes <[email protected]>
---
include/linux/mm.h | 14 ++++++++++++++
1 file changed, 14 insertions(+)
Index: linux-2.6.git/include/linux/mm.h
===================================================================
--- linux-2.6.git.orig/include/linux/mm.h
+++ linux-2.6.git/include/linux/mm.h
@@ -18,6 +18,7 @@
#include <linux/pfn.h>
#include <linux/bit_spinlock.h>
#include <linux/shrinker.h>
+#include <linux/resource.h>
struct mempolicy;
struct anon_vma;
@@ -1780,6 +1781,19 @@ extern struct vm_area_struct *copy_vma(s
bool *need_rmap_locks);
extern void exit_mmap(struct mm_struct *);
+static inline int may_adjust_brk(unsigned long rlim,
+ unsigned long new_brk,
+ unsigned long start_brk,
+ unsigned long end_data,
+ unsigned long start_data)
+{
+ if (rlim < RLIMIT_DATA) {
+ if (((new_brk - start_brk) + (end_data - start_data)) > rlim)
+ return -ENOSPC;
+ }
+ return 0;
+}
+
extern int mm_take_all_locks(struct mm_struct *mm);
extern void mm_drop_all_locks(struct mm_struct *mm);
On Thu, Jul 24, 2014 at 9:46 AM, Cyrill Gorcunov <[email protected]> wrote:
> To eliminate code duplication lets introduce may_adjust_brk
> helper which we will use in brk() and prctl() syscalls.
>
> Signed-off-by: Cyrill Gorcunov <[email protected]>
> Cc: Kees Cook <[email protected]>
> Cc: Tejun Heo <[email protected]>
> Cc: Andrew Morton <[email protected]>
> Cc: Andrew Vagin <[email protected]>
> Cc: Eric W. Biederman <[email protected]>
> Cc: H. Peter Anvin <[email protected]>
> Cc: Serge Hallyn <[email protected]>
> Cc: Pavel Emelyanov <[email protected]>
> Cc: Vasiliy Kulikov <[email protected]>
> Cc: KAMEZAWA Hiroyuki <[email protected]>
> Cc: Michael Kerrisk <[email protected]>
> Cc: Julien Tinnes <[email protected]>
> ---
> include/linux/mm.h | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> Index: linux-2.6.git/include/linux/mm.h
> ===================================================================
> --- linux-2.6.git.orig/include/linux/mm.h
> +++ linux-2.6.git/include/linux/mm.h
> @@ -18,6 +18,7 @@
> #include <linux/pfn.h>
> #include <linux/bit_spinlock.h>
> #include <linux/shrinker.h>
> +#include <linux/resource.h>
>
> struct mempolicy;
> struct anon_vma;
> @@ -1780,6 +1781,19 @@ extern struct vm_area_struct *copy_vma(s
> bool *need_rmap_locks);
> extern void exit_mmap(struct mm_struct *);
>
> +static inline int may_adjust_brk(unsigned long rlim,
> + unsigned long new_brk,
> + unsigned long start_brk,
> + unsigned long end_data,
> + unsigned long start_data)
> +{
> + if (rlim < RLIMIT_DATA) {
Won't rlim always be the value from a call to rlimit(RLIMIT_DATA)? Is
there a good reason to not just put the rlimit() call in
may_adjust_brk()? This would actually be an optimization in the
prctl_set_mm case, since now it calls rlimit() unconditionally, but
doesn't need to.
-Kees
> + if (((new_brk - start_brk) + (end_data - start_data)) > rlim)
> + return -ENOSPC;
> + }
> + return 0;
> +}
> +
> extern int mm_take_all_locks(struct mm_struct *mm);
> extern void mm_drop_all_locks(struct mm_struct *mm);
>
>
--
Kees Cook
Chrome OS Security
On Thu, Jul 24, 2014 at 12:18:56PM -0700, Kees Cook wrote:
> >
> > +static inline int may_adjust_brk(unsigned long rlim,
> > + unsigned long new_brk,
> > + unsigned long start_brk,
> > + unsigned long end_data,
> > + unsigned long start_data)
> > +{
> > + if (rlim < RLIMIT_DATA) {
>
> Won't rlim always be the value from a call to rlimit(RLIMIT_DATA)? Is
> there a good reason to not just put the rlimit() call in
> may_adjust_brk()? This would actually be an optimization in the
> prctl_set_mm case, since now it calls rlimit() unconditionally, but
> doesn't need to.
Nope, we use it for rlimit(RLIMIT_STACK) when checking for
@start_stack member.
Quoting Cyrill Gorcunov ([email protected]):
> To eliminate code duplication lets introduce may_adjust_brk
> helper which we will use in brk() and prctl() syscalls.
>
> Signed-off-by: Cyrill Gorcunov <[email protected]>
> Cc: Kees Cook <[email protected]>
> Cc: Tejun Heo <[email protected]>
> Cc: Andrew Morton <[email protected]>
> Cc: Andrew Vagin <[email protected]>
> Cc: Eric W. Biederman <[email protected]>
> Cc: H. Peter Anvin <[email protected]>
> Cc: Serge Hallyn <[email protected]>
> Cc: Pavel Emelyanov <[email protected]>
> Cc: Vasiliy Kulikov <[email protected]>
> Cc: KAMEZAWA Hiroyuki <[email protected]>
> Cc: Michael Kerrisk <[email protected]>
> Cc: Julien Tinnes <[email protected]>
> ---
> include/linux/mm.h | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> Index: linux-2.6.git/include/linux/mm.h
> ===================================================================
> --- linux-2.6.git.orig/include/linux/mm.h
> +++ linux-2.6.git/include/linux/mm.h
> @@ -18,6 +18,7 @@
> #include <linux/pfn.h>
> #include <linux/bit_spinlock.h>
> #include <linux/shrinker.h>
> +#include <linux/resource.h>
>
> struct mempolicy;
> struct anon_vma;
> @@ -1780,6 +1781,19 @@ extern struct vm_area_struct *copy_vma(s
> bool *need_rmap_locks);
> extern void exit_mmap(struct mm_struct *);
>
> +static inline int may_adjust_brk(unsigned long rlim,
> + unsigned long new_brk,
> + unsigned long start_brk,
> + unsigned long end_data,
> + unsigned long start_data)
> +{
> + if (rlim < RLIMIT_DATA) {
In the code you're replacing, this was RLIM_INFINITY. Did you really
mean for this to be RLIMIT_DATA, aka 2?
> + if (((new_brk - start_brk) + (end_data - start_data)) > rlim)
> + return -ENOSPC;
> + }
> + return 0;
> +}
> +
> extern int mm_take_all_locks(struct mm_struct *mm);
> extern void mm_drop_all_locks(struct mm_struct *mm);
>
>
On Thu, Jul 24, 2014 at 07:32:25PM +0000, Serge Hallyn wrote:
> Quoting Cyrill Gorcunov ([email protected]):
> > To eliminate code duplication lets introduce may_adjust_brk
> > helper which we will use in brk() and prctl() syscalls.
> >
> > Signed-off-by: Cyrill Gorcunov <[email protected]>
> > Cc: Kees Cook <[email protected]>
> > Cc: Tejun Heo <[email protected]>
> > Cc: Andrew Morton <[email protected]>
> > Cc: Andrew Vagin <[email protected]>
> > Cc: Eric W. Biederman <[email protected]>
> > Cc: H. Peter Anvin <[email protected]>
> > Cc: Serge Hallyn <[email protected]>
> > Cc: Pavel Emelyanov <[email protected]>
> > Cc: Vasiliy Kulikov <[email protected]>
> > Cc: KAMEZAWA Hiroyuki <[email protected]>
> > Cc: Michael Kerrisk <[email protected]>
> > Cc: Julien Tinnes <[email protected]>
> > ---
> > include/linux/mm.h | 14 ++++++++++++++
> > 1 file changed, 14 insertions(+)
> >
> > Index: linux-2.6.git/include/linux/mm.h
> > ===================================================================
> > --- linux-2.6.git.orig/include/linux/mm.h
> > +++ linux-2.6.git/include/linux/mm.h
> > @@ -18,6 +18,7 @@
> > #include <linux/pfn.h>
> > #include <linux/bit_spinlock.h>
> > #include <linux/shrinker.h>
> > +#include <linux/resource.h>
> >
> > struct mempolicy;
> > struct anon_vma;
> > @@ -1780,6 +1781,19 @@ extern struct vm_area_struct *copy_vma(s
> > bool *need_rmap_locks);
> > extern void exit_mmap(struct mm_struct *);
> >
> > +static inline int may_adjust_brk(unsigned long rlim,
> > + unsigned long new_brk,
> > + unsigned long start_brk,
> > + unsigned long end_data,
> > + unsigned long start_data)
> > +{
> > + if (rlim < RLIMIT_DATA) {
>
> In the code you're replacing, this was RLIM_INFINITY. Did you really
> mean for this to be RLIMIT_DATA, aka 2?
Good catch, thanks Serge! Better would be to pass the type of resource
(as Kees suggested) here instead of @rlim itself and sure to compare
with RLIM_INFINITY.