| Message ID | 20220223153613.835563-2-surenb@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,1/3] mm: refactor vm_area_struct::anon_vma_name usage code | expand |
On Wed 23-02-22 07:36:12, Suren Baghdasaryan wrote: > A deep process chain with many vmas could grow really high. > With default sysctl_max_map_count (64k) and default pid_max (32k) > the max number of vmas in the system is 2147450880 and the > refcounter has headroom of 1073774592 before it reaches > REFCOUNT_SATURATED (3221225472). Therefore it's unlikely that > an anonymous name refcounter will overflow with these defaults. > Currently the max for pid_max is PID_MAX_LIMIT (4194304) and > for sysctl_max_map_count it's INT_MAX (2147483647). In this > configuration anon_vma_name refcount overflow becomes > theoretically possible (that still require heavy sharing of > that anon_vma_name between processes). > kref refcounting interface used in anon_vma_name structure will > detect a counter overflow when it reaches REFCOUNT_SATURATED value > but will only generate a warning about broken refcounter. If I am reading the refcounter code properly the "overflow" will simply make the ref counter frozen and the object will never be freed. A determined attacker could leak memory like that but it would be rather expensive and inefficient way to do so. Still good to have it covered. > To ensure anon_vma_name refcount does not overflow, stop anon_vma_name > sharing when the refcount reaches REFCOUNT_MAX (2147483647), which > still leaves INT_MAX/2 (1073741823) values before the counter reaches > REFCOUNT_SATURATED. This should provide enough headroom for raising > the refcounts temporarily. I am not sure this is the intended way refcounter users should avoid overflows but I do not see other interface that would be usable. Maybe somebody else can come up with a better suggestion but this approach makes sense to me. > > Suggested-by: Michal Hocko <mhocko@suse.com> > Signed-off-by: Suren Baghdasaryan <surenb@google.com> Acked-by: Michal Hocko <mhocko@suse.com> Thanks! > --- > changes in v2: > - Updated description to include calculation details, per Michal Hocko > > include/linux/mm_inline.h | 18 ++++++++++++++---- > mm/madvise.c | 3 +-- > 2 files changed, 15 insertions(+), 6 deletions(-) > > diff --git a/include/linux/mm_inline.h b/include/linux/mm_inline.h > index 4bad32507570..f82085ff8a6b 100644 > --- a/include/linux/mm_inline.h > +++ b/include/linux/mm_inline.h > @@ -161,15 +161,25 @@ static inline void anon_vma_name_put(struct anon_vma_name *anon_name) > kref_put(&anon_name->kref, anon_vma_name_free); > } > > +static inline > +struct anon_vma_name *anon_vma_name_reuse(struct anon_vma_name *anon_name) > +{ > + /* Prevent anon_name refcount saturation early on */ > + if (kref_read(&anon_name->kref) < REFCOUNT_MAX) { > + anon_vma_name_get(anon_name); > + return anon_name; > + > + } > + return anon_vma_name_alloc(anon_name->name); > +} > + > static inline void dup_anon_vma_name(struct vm_area_struct *orig_vma, > struct vm_area_struct *new_vma) > { > struct anon_vma_name *anon_name = anon_vma_name(orig_vma); > > - if (anon_name) { > - anon_vma_name_get(anon_name); > - new_vma->anon_name = anon_name; > - } > + if (anon_name) > + new_vma->anon_name = anon_vma_name_reuse(anon_name); > } > > static inline void free_anon_vma_name(struct vm_area_struct *vma) > diff --git a/mm/madvise.c b/mm/madvise.c > index 081b1cded21e..1f2693dccf7b 100644 > --- a/mm/madvise.c > +++ b/mm/madvise.c > @@ -113,8 +113,7 @@ static int replace_anon_vma_name(struct vm_area_struct *vma, > if (anon_vma_name_eq(orig_name, anon_name)) > return 0; > > - anon_vma_name_get(anon_name); > - vma->anon_name = anon_name; > + vma->anon_name = anon_vma_name_reuse(anon_name); > anon_vma_name_put(orig_name); > > return 0; > -- > 2.35.1.473.g83b2b277ed-goog
diff --git a/include/linux/mm_inline.h b/include/linux/mm_inline.h index 4bad32507570..f82085ff8a6b 100644 --- a/include/linux/mm_inline.h +++ b/include/linux/mm_inline.h @@ -161,15 +161,25 @@ static inline void anon_vma_name_put(struct anon_vma_name *anon_name) kref_put(&anon_name->kref, anon_vma_name_free); } +static inline +struct anon_vma_name *anon_vma_name_reuse(struct anon_vma_name *anon_name) +{ + /* Prevent anon_name refcount saturation early on */ + if (kref_read(&anon_name->kref) < REFCOUNT_MAX) { + anon_vma_name_get(anon_name); + return anon_name; + + } + return anon_vma_name_alloc(anon_name->name); +} + static inline void dup_anon_vma_name(struct vm_area_struct *orig_vma, struct vm_area_struct *new_vma) { struct anon_vma_name *anon_name = anon_vma_name(orig_vma); - if (anon_name) { - anon_vma_name_get(anon_name); - new_vma->anon_name = anon_name; - } + if (anon_name) + new_vma->anon_name = anon_vma_name_reuse(anon_name); } static inline void free_anon_vma_name(struct vm_area_struct *vma) diff --git a/mm/madvise.c b/mm/madvise.c index 081b1cded21e..1f2693dccf7b 100644 --- a/mm/madvise.c +++ b/mm/madvise.c @@ -113,8 +113,7 @@ static int replace_anon_vma_name(struct vm_area_struct *vma, if (anon_vma_name_eq(orig_name, anon_name)) return 0; - anon_vma_name_get(anon_name); - vma->anon_name = anon_name; + vma->anon_name = anon_vma_name_reuse(anon_name); anon_vma_name_put(orig_name); return 0;
A deep process chain with many vmas could grow really high. With default sysctl_max_map_count (64k) and default pid_max (32k) the max number of vmas in the system is 2147450880 and the refcounter has headroom of 1073774592 before it reaches REFCOUNT_SATURATED (3221225472). Therefore it's unlikely that an anonymous name refcounter will overflow with these defaults. Currently the max for pid_max is PID_MAX_LIMIT (4194304) and for sysctl_max_map_count it's INT_MAX (2147483647). In this configuration anon_vma_name refcount overflow becomes theoretically possible (that still require heavy sharing of that anon_vma_name between processes). kref refcounting interface used in anon_vma_name structure will detect a counter overflow when it reaches REFCOUNT_SATURATED value but will only generate a warning about broken refcounter. To ensure anon_vma_name refcount does not overflow, stop anon_vma_name sharing when the refcount reaches REFCOUNT_MAX (2147483647), which still leaves INT_MAX/2 (1073741823) values before the counter reaches REFCOUNT_SATURATED. This should provide enough headroom for raising the refcounts temporarily. Suggested-by: Michal Hocko <mhocko@suse.com> Signed-off-by: Suren Baghdasaryan <surenb@google.com> --- changes in v2: - Updated description to include calculation details, per Michal Hocko include/linux/mm_inline.h | 18 ++++++++++++++---- mm/madvise.c | 3 +-- 2 files changed, 15 insertions(+), 6 deletions(-)