| Message ID | 20181001143138.95119-3-jannh@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2,1/3] mm/vmstat: fix outdated vmstat_text | expand |
On Mon, Oct 1, 2018 at 7:31 AM, Jann Horn <jannh@google.com> wrote: > As evidenced by the previous two patches, having two gigantic arrays that > must manually be kept in sync, including ifdefs, isn't exactly robust. > To make it easier to catch such issues in the future, add a BUILD_BUG_ON(). > > Signed-off-by: Jann Horn <jannh@google.com> Reviewed-by: Kees Cook <keescook@chromium.org> -Kees > --- > mm/vmstat.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/mm/vmstat.c b/mm/vmstat.c > index 7878da76abf2..b678c607e490 100644 > --- a/mm/vmstat.c > +++ b/mm/vmstat.c > @@ -1663,6 +1663,8 @@ static void *vmstat_start(struct seq_file *m, loff_t *pos) > stat_items_size += sizeof(struct vm_event_state); > #endif > > + BUILD_BUG_ON(stat_items_size != > + ARRAY_SIZE(vmstat_text) * sizeof(unsigned long)); > v = kmalloc(stat_items_size, GFP_KERNEL); > m->private = v; > if (!v) > -- > 2.19.0.605.g01d371f741-goog >
On Mon 01-10-18 16:31:38, Jann Horn wrote: > As evidenced by the previous two patches, having two gigantic arrays that > must manually be kept in sync, including ifdefs, isn't exactly robust. > To make it easier to catch such issues in the future, add a BUILD_BUG_ON(). > > Signed-off-by: Jann Horn <jannh@google.com> We should have done that looong ago. Thanks! Acked-by: Michal Hocko <mhocko@suse.com> > --- > mm/vmstat.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/mm/vmstat.c b/mm/vmstat.c > index 7878da76abf2..b678c607e490 100644 > --- a/mm/vmstat.c > +++ b/mm/vmstat.c > @@ -1663,6 +1663,8 @@ static void *vmstat_start(struct seq_file *m, loff_t *pos) > stat_items_size += sizeof(struct vm_event_state); > #endif > > + BUILD_BUG_ON(stat_items_size != > + ARRAY_SIZE(vmstat_text) * sizeof(unsigned long)); > v = kmalloc(stat_items_size, GFP_KERNEL); > m->private = v; > if (!v) > -- > 2.19.0.605.g01d371f741-goog
On Mon, Oct 01, 2018 at 04:31:38PM +0200, Jann Horn wrote: > As evidenced by the previous two patches, having two gigantic arrays that > must manually be kept in sync, including ifdefs, isn't exactly robust. > To make it easier to catch such issues in the future, add a BUILD_BUG_ON(). > > Signed-off-by: Jann Horn <jannh@google.com> > --- > mm/vmstat.c | 2 ++ > 1 file changed, 2 insertions(+) I agree with Michal here, we had to do this long time ago. For patches 1-3: Acked-by: Roman Gushchin <guro@fb.com> BTW, don't we want to split this huge array into smaller parts? This will make the code more clear and easier to modify. Thank you!
diff --git a/mm/vmstat.c b/mm/vmstat.c index 7878da76abf2..b678c607e490 100644 --- a/mm/vmstat.c +++ b/mm/vmstat.c @@ -1663,6 +1663,8 @@ static void *vmstat_start(struct seq_file *m, loff_t *pos) stat_items_size += sizeof(struct vm_event_state); #endif + BUILD_BUG_ON(stat_items_size != + ARRAY_SIZE(vmstat_text) * sizeof(unsigned long)); v = kmalloc(stat_items_size, GFP_KERNEL); m->private = v; if (!v)
As evidenced by the previous two patches, having two gigantic arrays that must manually be kept in sync, including ifdefs, isn't exactly robust. To make it easier to catch such issues in the future, add a BUILD_BUG_ON(). Signed-off-by: Jann Horn <jannh@google.com> --- mm/vmstat.c | 2 ++ 1 file changed, 2 insertions(+)