[v3,BZ,#17273] fix incorrect mount table entry parsing in __getmntent_r()
Commit Message
When mount entry contains only four fields and have more
then one space or tab at the and, mp.mnt_freq and
mp.mnt_passno will be set to some specific values as side
effect from parsing of previus mount entry. It is because
sscanf(""," %d %d ", &a, &b) returns -1, but this case
is unprocessed. Values of mp.mnt_freq and mp.mnt_passno
stays unchanged. This patch is attempt to fix described issue
by removing trailing tabs and spaces.
[BZ #17273]
* misc/mntent_r.c (__getmntent_r): Cut off trailing spaces
and tabs from buffer before parsing fstab entry.
* misc/tst-mntent.c (main): Add test for mount entry with
trailing spaces and tabs.
---
misc/mntent_r.c | 6 +++++-
misc/tst-mntent.c | 22 +++++++++++++++++++++-
2 files changed, 26 insertions(+), 2 deletions(-)
Comments
The change looks fine to me now.
I don't see copyright papers for you on file.
On 10.10.2014 05:21, Roland McGrath wrote:
> I don't see copyright papers for you on file.
>
> .
I thought this changes are trivial. But anyway
say what I should do and I will do it. It's my
first patch.
> I thought this changes are trivial. But anyway
> say what I should do and I will do it. It's my
> first patch.
They are sort of borderline for the "number of lines of code" metric. The
safe thing is to assume they do require copyright assignment. If you are
going to make any more contributions to libc, you'll need an assignment on
file anyway. I'll send you the details off-list.
Thanks,
Roland
On 04 Oct 2014 13:31, Vladimir A. Nazarenko wrote:
> --- a/misc/mntent_r.c
> +++ b/misc/mntent_r.c
> @@ -135,7 +135,11 @@ __getmntent_r (FILE *stream, struct mntent *mp, char *buffer, int bufsiz)
>
> end_ptr = strchr (buffer, '\n');
> if (end_ptr != NULL) /* chop newline */
> - *end_ptr = '\0';
> + {
> + while (end_ptr[-1] == ' ' || end_ptr[-1] == '\t')
> + end_ptr--;
> + *end_ptr = '\0';
> + }
this randomly corrupts memory when you get a blank line which is pretty
common i think in /etc/fstab. buffer = "\n" which means end_ptr will be
buffer which means end_ptr[-1] is random stack memory. if it happens to
be 0x20 or 0x09, you corrupt a single byte. happens whenever the line is
just whitespace as you walk back to the start of the buffer allocation.
the way the malloc heaps are laid out, it doesn't seem to be noticed for
most arches, but it's easily reproducible on ppc32.
https://sourceware.org/bugzilla/show_bug.cgi?id=18887
-mike
@@ -135,7 +135,11 @@ __getmntent_r (FILE *stream, struct mntent *mp, char *buffer, int bufsiz)
end_ptr = strchr (buffer, '\n');
if (end_ptr != NULL) /* chop newline */
- *end_ptr = '\0';
+ {
+ while (end_ptr[-1] == ' ' || end_ptr[-1] == '\t')
+ end_ptr--;
+ *end_ptr = '\0';
+ }
else
{
/* Not the whole line was read. Do it now but forget it. */
@@ -73,7 +73,27 @@ main (int argc, char *argv[])
puts ("Error while reading written entry back in");
result = 1;
}
- }
+
+ /* Part III: Entry with whitespaces at the end of a line. */
+ rewind (fp);
+
+ fputs ("/foo\\040dir /bar\\040dir auto bind \t \n", fp);
+
+ rewind (fp);
+
+ mnt = getmntent (fp);
+
+ if (strcmp (mnt->mnt_fsname, "/foo dir") != 0
+ || strcmp (mnt->mnt_dir, "/bar dir") != 0
+ || strcmp (mnt->mnt_type, "auto") != 0
+ || strcmp (mnt->mnt_opts, "bind") != 0
+ || mnt->mnt_freq != 0
+ || mnt->mnt_passno != 0)
+ {
+ puts ("Error while reading entry with trailing whitespaces");
+ result = 1;
+ }
+ }
return result;
}