* [PATCH] db/print: bad character cause illegal memory access
@ 2018-01-09 6:20 Zorro Lang
2018-01-09 15:55 ` Brian Foster
0 siblings, 1 reply; 3+ messages in thread
From: Zorro Lang @ 2018-01-09 6:20 UTC (permalink / raw)
To: linux-xfs
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;
v[nv].tokty = TT_END;
return v;
--
2.13.6
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] db/print: bad character cause illegal memory access
2018-01-09 6:20 [PATCH] db/print: bad character cause illegal memory access Zorro Lang
@ 2018-01-09 15:55 ` Brian Foster
2018-01-10 7:00 ` Zorro Lang
0 siblings, 1 reply; 3+ messages in thread
From: Brian Foster @ 2018-01-09 15:55 UTC (permalink / raw)
To: Zorro Lang; +Cc: linux-xfs
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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] db/print: bad character cause illegal memory access
2018-01-09 15:55 ` Brian Foster
@ 2018-01-10 7:00 ` Zorro Lang
0 siblings, 0 replies; 3+ messages in thread
From: Zorro Lang @ 2018-01-10 7:00 UTC (permalink / raw)
To: Brian Foster; +Cc: linux-xfs
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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-01-10 7:00 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-09 6:20 [PATCH] db/print: bad character cause illegal memory access Zorro Lang
2018-01-09 15:55 ` Brian Foster
2018-01-10 7:00 ` Zorro Lang
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.