[CRIU] Re: [PATCH cr] proc: add a static buffer to prevent segv

[Andrew Vagin]

On Wed, May 02, 2012 at 02:48:39PM +0400, Cyrill Gorcunov wrote:
> On Wed, May 02, 2012 at 02:46:10PM +0400, Cyrill Gorcunov wrote:
> > On Wed, May 02, 2012 at 02:37:57PM +0400, Andrey Vagin wrote:
> > > A few of our functions use buffer and string functions.
> > > All these functions require that a string contains '\0' at the end.
> > > Before this patch we didn't guarantee that.
> > > 
> > > I've seen segmentation fault in parse_pid_stat_small.
> > > 
> > > Signed-off-by: Andrey Vagin <avagin at openvz.org>
> > > ---
> > >  proc_parse.c |   31 +++++++++++++++++++------------
> > >  1 files changed, 19 insertions(+), 12 deletions(-)
> > > 
> > > diff --git a/proc_parse.c b/proc_parse.c
> > > index cd1e7d6..cc67fdc 100644
> > > --- a/proc_parse.c
> > > +++ b/proc_parse.c
> > > @@ -15,11 +15,20 @@
> > >  
> > >  #include "proc_parse.h"
> > >  
> > > +struct buffer {
> > > +	char buf[PAGE_SIZE];
> > > +	char end; /* '\0' */
> > > +};
> > > +
> > 
> > No need to define one more struct, just make it simple
> > 
> > 	static char buf[PAGE_SIZE + 1];
> > 
> > and never read more than PAGE_SIZE. No?

Never read more than the last argument of read() ;)

> 
> Actually I've seen sigsegv as well, but it was at
> 
> parse_pid_stat
> 		...
> 		strncpy(s->comm, tok + 1, sizeof(s->comm) - 1);
> 
> 	Cyrill