| Message ID | 20180109062027.4570-1-zlang@redhat.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Tue, Jan 09, 2018 at 02:20:27PM +0800, Zorro Lang wrote: > xfs_db can print specified field values, likes: > # xfs_db -c "inode $inum" -c "print core.nblocks" $dev > > But when we give it some bad chararcters, the 'print' command will > crash: > > # xfs_db -r -c "inode 67211167" -c "print core.*" /dev/mapper/rhel-root > bad character in field * > *** Error in `xfs_db': free(): invalid pointer: 0x00007fa116e937b8 *** > ... > (crash messages) > ... > # xfs_db -r -c "inode 67211167" -c "print core.\"" /dev/mapper/rhel-root > missing closing quote > *** Error in `xfs_db': free(): invalid pointer: 0x00007f6e8e3c37b8 *** > ... > (crash messages) > ... > > The reason is xfs_db call ftok_free() to free ftok_t list, but > flist_split() forgets to set the tail to NULL. That cause ftok_free() > trys to free illegal memory address. > > Signed-off-by: Zorro Lang <zlang@redhat.com> > --- > db/flist.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/db/flist.c b/db/flist.c > index e11acbfc..1a875b95 100644 > --- a/db/flist.c > +++ b/db/flist.c > @@ -371,11 +371,14 @@ flist_split( > v = xmalloc(sizeof(*v)); > v->tok = NULL; > while (*s) { > + v = xrealloc(v, (nv + 1) * sizeof(*v)); > /* need to add string handling */ > if (*s == '\"') { > s++; /* skip first quote */ > if ((a = strrchr(s, '\"')) == NULL) { > dbprintf(_("missing closing quote %s\n"), s); > + v[nv].tok = NULL; > + v[nv].tokty = TT_END; > ftok_free(v); > return NULL; > } > @@ -393,19 +396,21 @@ flist_split( > t = puncttypes[a - punctchars]; > } else { > dbprintf(_("bad character in field %s\n"), s); > + v[nv].tok = NULL; > + v[nv].tokty = TT_END; > ftok_free(v); > return NULL; > } > a = xmalloc(l + 1); > strncpy(a, s, l); > a[l] = '\0'; > - v = xrealloc(v, (nv + 2) * sizeof(*v)); > v[nv].tok = a; > v[nv].tokty = t; > nv++; > s += l + tailskip; > tailskip = 0; > } > + v = xrealloc(v, (nv + 1) * sizeof(*v)); > v[nv].tok = NULL; Could we just move the above line into the loop (right after nv is bumped) to initialize .tok similar to the initial allocation before the loop? Brian > v[nv].tokty = TT_END; > return v; > -- > 2.13.6 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jan 09, 2018 at 10:55:37AM -0500, Brian Foster wrote: > On Tue, Jan 09, 2018 at 02:20:27PM +0800, Zorro Lang wrote: > > xfs_db can print specified field values, likes: > > # xfs_db -c "inode $inum" -c "print core.nblocks" $dev > > > > But when we give it some bad chararcters, the 'print' command will > > crash: > > > > # xfs_db -r -c "inode 67211167" -c "print core.*" /dev/mapper/rhel-root > > bad character in field * > > *** Error in `xfs_db': free(): invalid pointer: 0x00007fa116e937b8 *** > > ... > > (crash messages) > > ... > > # xfs_db -r -c "inode 67211167" -c "print core.\"" /dev/mapper/rhel-root > > missing closing quote > > *** Error in `xfs_db': free(): invalid pointer: 0x00007f6e8e3c37b8 *** > > ... > > (crash messages) > > ... > > > > The reason is xfs_db call ftok_free() to free ftok_t list, but > > flist_split() forgets to set the tail to NULL. That cause ftok_free() > > trys to free illegal memory address. > > > > Signed-off-by: Zorro Lang <zlang@redhat.com> > > --- > > db/flist.c | 7 ++++++- > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > diff --git a/db/flist.c b/db/flist.c > > index e11acbfc..1a875b95 100644 > > --- a/db/flist.c > > +++ b/db/flist.c > > @@ -371,11 +371,14 @@ flist_split( > > v = xmalloc(sizeof(*v)); > > v->tok = NULL; > > while (*s) { > > + v = xrealloc(v, (nv + 1) * sizeof(*v)); > > /* need to add string handling */ > > if (*s == '\"') { > > s++; /* skip first quote */ > > if ((a = strrchr(s, '\"')) == NULL) { > > dbprintf(_("missing closing quote %s\n"), s); > > + v[nv].tok = NULL; > > + v[nv].tokty = TT_END; > > ftok_free(v); > > return NULL; > > } > > @@ -393,19 +396,21 @@ flist_split( > > t = puncttypes[a - punctchars]; > > } else { > > dbprintf(_("bad character in field %s\n"), s); > > + v[nv].tok = NULL; > > + v[nv].tokty = TT_END; > > ftok_free(v); > > return NULL; > > } > > a = xmalloc(l + 1); > > strncpy(a, s, l); > > a[l] = '\0'; > > - v = xrealloc(v, (nv + 2) * sizeof(*v)); > > v[nv].tok = a; > > v[nv].tokty = t; > > nv++; > > s += l + tailskip; > > tailskip = 0; > > } > > + v = xrealloc(v, (nv + 1) * sizeof(*v)); > > v[nv].tok = NULL; > > Could we just move the above line into the loop (right after nv is > bumped) to initialize .tok similar to the initial allocation before the > loop? Thanks for you review, Brian. I found djwong@ has fixed this bug on xfsprogs for-next branch by: commit 945e47e2fcc5d1cec693122286da06d8ab829c52 Author: Darrick J. Wong <darrick.wong@oracle.com> Date: Thu Jan 4 13:58:29 2018 -0600 xfs_db: fix crash when field list selector string has trailing slash Thanks for you help, Zorro > > Brian > > > v[nv].tokty = TT_END; > > return v; > > -- > > 2.13.6 > > > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/db/flist.c b/db/flist.c index e11acbfc..1a875b95 100644 --- a/db/flist.c +++ b/db/flist.c @@ -371,11 +371,14 @@ flist_split( v = xmalloc(sizeof(*v)); v->tok = NULL; while (*s) { + v = xrealloc(v, (nv + 1) * sizeof(*v)); /* need to add string handling */ if (*s == '\"') { s++; /* skip first quote */ if ((a = strrchr(s, '\"')) == NULL) { dbprintf(_("missing closing quote %s\n"), s); + v[nv].tok = NULL; + v[nv].tokty = TT_END; ftok_free(v); return NULL; } @@ -393,19 +396,21 @@ flist_split( t = puncttypes[a - punctchars]; } else { dbprintf(_("bad character in field %s\n"), s); + v[nv].tok = NULL; + v[nv].tokty = TT_END; ftok_free(v); return NULL; } a = xmalloc(l + 1); strncpy(a, s, l); a[l] = '\0'; - v = xrealloc(v, (nv + 2) * sizeof(*v)); v[nv].tok = a; v[nv].tokty = t; nv++; s += l + tailskip; tailskip = 0; } + v = xrealloc(v, (nv + 1) * sizeof(*v)); v[nv].tok = NULL; v[nv].tokty = TT_END; return v;
xfs_db can print specified field values, likes: # xfs_db -c "inode $inum" -c "print core.nblocks" $dev But when we give it some bad chararcters, the 'print' command will crash: # xfs_db -r -c "inode 67211167" -c "print core.*" /dev/mapper/rhel-root bad character in field * *** Error in `xfs_db': free(): invalid pointer: 0x00007fa116e937b8 *** ... (crash messages) ... # xfs_db -r -c "inode 67211167" -c "print core.\"" /dev/mapper/rhel-root missing closing quote *** Error in `xfs_db': free(): invalid pointer: 0x00007f6e8e3c37b8 *** ... (crash messages) ... The reason is xfs_db call ftok_free() to free ftok_t list, but flist_split() forgets to set the tail to NULL. That cause ftok_free() trys to free illegal memory address. Signed-off-by: Zorro Lang <zlang@redhat.com> --- db/flist.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)