* [musl] [PATCH] mntent: deal with escaped whitespace in mtab and fstab
@ 2023-03-29 15:17 Matthias Goergens
2023-03-29 17:04 ` Szabolcs Nagy
0 siblings, 1 reply; 9+ messages in thread
From: Matthias Goergens @ 2023-03-29 15:17 UTC (permalink / raw)
To: musl; +Cc: Matthias Goergens
From glibc's documentation:
> Since fields in the mtab and fstab files are separated by whitespace,
> octal escapes are used to represent the characters space (\040),
> tab (\011), newline (\012), and backslash (\\) in those files when they
> occur in one of the four strings in a mntent structure. The
> routines addmntent() and getmntent() will convert from string
> representation to escaped representation and back. When converting
> from escaped representation, the sequence \134 is also converted to a
> backslash.
This fixes the issue reported in https://www.openwall.com/lists/musl/2021/12/14/1
Please pardon the previous broken patch that I tried to send directly
via gmail.
This is my first time contributing to musl. Please point out any ways
to improve. I probably got a few things wrong?
Thanks!
Addendum: this is a new version. The first one did not copy the final
null-byte.
---
src/misc/mntent.c | 75 ++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 68 insertions(+), 7 deletions(-)
diff --git a/src/misc/mntent.c b/src/misc/mntent.c
index d404fbe3..49f4e386 100644
--- a/src/misc/mntent.c
+++ b/src/misc/mntent.c
@@ -1,3 +1,4 @@
+#include <assert.h>
#include <stdio.h>
#include <string.h>
#include <mntent.h>
@@ -20,6 +21,37 @@ int endmntent(FILE *f)
return 1;
}
+static inline int decode1(char** in_buf, char** out_buf, const char* from, const char to) {
+ if(strncmp(from, *in_buf, strlen(from)) == 0) {
+ *(*out_buf)++ = to;
+ *in_buf += strlen(from);
+ return 1;
+ }
+ return 0;
+}
+
+static inline char* decode(char* buf) {
+ assert(buf != NULL);
+ char* read_cursor = buf;
+ char* write_cursor = buf;
+ while(*read_cursor) {
+ // space
+ decode1(&read_cursor, &write_cursor, "\\040", '\040')
+ // tab
+ || decode1(&read_cursor, &write_cursor, "\\011", '\011')
+ // newline
+ || decode1(&read_cursor, &write_cursor, "\\012", '\012')
+ // backslash
+ || decode1(&read_cursor, &write_cursor, "\\134", '\134')
+ || decode1(&read_cursor, &write_cursor, "\\\\", '\\')
+ // default: copy char as is.
+ || (*write_cursor++ = *read_cursor++);
+ }
+ *write_cursor = *read_cursor;
+
+ return buf;
+}
+
struct mntent *getmntent_r(FILE *f, struct mntent *mnt, char *linebuf, int buflen)
{
int n[8], use_internal = (linebuf == SENTINEL);
@@ -55,10 +87,10 @@ struct mntent *getmntent_r(FILE *f, struct mntent *mnt, char *linebuf, int bufle
linebuf[n[5]] = 0;
linebuf[n[7]] = 0;
- mnt->mnt_fsname = linebuf+n[0];
- mnt->mnt_dir = linebuf+n[2];
- mnt->mnt_type = linebuf+n[4];
- mnt->mnt_opts = linebuf+n[6];
+ mnt->mnt_fsname = decode(linebuf+n[0]);
+ mnt->mnt_dir = decode(linebuf+n[2]);
+ mnt->mnt_type = decode(linebuf+n[4]);
+ mnt->mnt_opts = decode(linebuf+n[6]);
return mnt;
}
@@ -69,12 +101,41 @@ struct mntent *getmntent(FILE *f)
return getmntent_r(f, &mnt, SENTINEL, 0);
}
+static inline int write_string(FILE *f, const char* str)
+{
+ char c;
+ int error_occured = 0;
+ while(str && !error_occured && (c = *str++) != 0) {
+ if(c == '\040') // space
+ error_occured = fprintf(f, "%s", "\\040") < 0;
+ else if (c == '\011') // tab
+ error_occured = fprintf(f, "%s", "\\011") < 0;
+ else if (c == '\012') // newline
+ error_occured = fprintf(f, "%s", "\\012") < 0;
+ else if (c == '\\')
+ error_occured = fprintf(f, "%s", "\\\\") < 0;
+ else
+ error_occured = fprintf(f, "%c", c) < 0;
+ }
+ return error_occured;
+}
+
int addmntent(FILE *f, const struct mntent *mnt)
{
if (fseek(f, 0, SEEK_END)) return 1;
- return fprintf(f, "%s\t%s\t%s\t%s\t%d\t%d\n",
- mnt->mnt_fsname, mnt->mnt_dir, mnt->mnt_type, mnt->mnt_opts,
- mnt->mnt_freq, mnt->mnt_passno) < 0;
+ flockfile(f);
+ int result =
+ write_string(f, mnt->mnt_fsname)
+ || (fprintf(f, "\t") < 0)
+ || write_string(f, mnt->mnt_dir)
+ || (fprintf(f, "\t") < 0)
+ || write_string(f, mnt->mnt_type)
+ || (fprintf(f, "\t") < 0)
+ || write_string(f, mnt->mnt_opts)
+ || (fprintf(f, "\t%d\t%d\n",
+ mnt->mnt_freq, mnt->mnt_passno) < 0);
+ funlockfile(f);
+ return result;
}
char *hasmntopt(const struct mntent *mnt, const char *opt)
--
2.40.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [musl] [PATCH] mntent: deal with escaped whitespace in mtab and fstab
2023-03-29 15:17 [musl] [PATCH] mntent: deal with escaped whitespace in mtab and fstab Matthias Goergens
@ 2023-03-29 17:04 ` Szabolcs Nagy
2023-03-29 17:16 ` Szabolcs Nagy
0 siblings, 1 reply; 9+ messages in thread
From: Szabolcs Nagy @ 2023-03-29 17:04 UTC (permalink / raw)
To: Matthias Goergens; +Cc: musl
* Matthias Goergens <matthias.goergens@gmail.com> [2023-03-29 23:17:51 +0800]:
> >From glibc's documentation:
>
> > Since fields in the mtab and fstab files are separated by whitespace,
> > octal escapes are used to represent the characters space (\040),
> > tab (\011), newline (\012), and backslash (\\) in those files when they
> > occur in one of the four strings in a mntent structure. The
> > routines addmntent() and getmntent() will convert from string
> > representation to escaped representation and back. When converting
> > from escaped representation, the sequence \134 is also converted to a
> > backslash.
>
> This fixes the issue reported in https://www.openwall.com/lists/musl/2021/12/14/1
>
> Please pardon the previous broken patch that I tried to send directly
> via gmail.
>
> This is my first time contributing to musl. Please point out any ways
> to improve. I probably got a few things wrong?
>
> Thanks!
>
> Addendum: this is a new version. The first one did not copy the final
> null-byte.
> ---
> src/misc/mntent.c | 75 ++++++++++++++++++++++++++++++++++++++++++-----
> 1 file changed, 68 insertions(+), 7 deletions(-)
>
> diff --git a/src/misc/mntent.c b/src/misc/mntent.c
> index d404fbe3..49f4e386 100644
> --- a/src/misc/mntent.c
> +++ b/src/misc/mntent.c
> @@ -1,3 +1,4 @@
> +#include <assert.h>
> #include <stdio.h>
> #include <string.h>
> #include <mntent.h>
> @@ -20,6 +21,37 @@ int endmntent(FILE *f)
> return 1;
> }
>
> +static inline int decode1(char** in_buf, char** out_buf, const char* from, const char to) {
> + if(strncmp(from, *in_buf, strlen(from)) == 0) {
> + *(*out_buf)++ = to;
> + *in_buf += strlen(from);
> + return 1;
> + }
> + return 0;
> +}
> +
> +static inline char* decode(char* buf) {
> + assert(buf != NULL);
> + char* read_cursor = buf;
> + char* write_cursor = buf;
> + while(*read_cursor) {
> + // space
> + decode1(&read_cursor, &write_cursor, "\\040", '\040')
> + // tab
> + || decode1(&read_cursor, &write_cursor, "\\011", '\011')
> + // newline
> + || decode1(&read_cursor, &write_cursor, "\\012", '\012')
> + // backslash
> + || decode1(&read_cursor, &write_cursor, "\\134", '\134')
> + || decode1(&read_cursor, &write_cursor, "\\\\", '\\')
> + // default: copy char as is.
> + || (*write_cursor++ = *read_cursor++);
> + }
this will try matching at every position, but
we know that each escape starts with \ so this
can be optimized.
i expect inlining decode1 will increase code size
unnecessarily. you can avoid that e.g. like
for (;;) {
...
const char *replace =
"\040" "040" "\0"
"\011" "011" "\0"
...
"\\" "\\" "\0"
"\\" "";
for (;;) {
char c = *replace++;
size_t n = strlen(replace);
if (strncmp(pr, replace, n) == 0) {
*pw++ = c;
pr += n;
break;
}
replace += n+1;
}
...
char *next = __strchrnul(pr, '\\');
memmove(pw, pr, next-pr);
...
}
> + *write_cursor = *read_cursor;
> +
> + return buf;
> +}
> +
> struct mntent *getmntent_r(FILE *f, struct mntent *mnt, char *linebuf, int buflen)
> {
> int n[8], use_internal = (linebuf == SENTINEL);
> @@ -55,10 +87,10 @@ struct mntent *getmntent_r(FILE *f, struct mntent *mnt, char *linebuf, int bufle
> linebuf[n[5]] = 0;
> linebuf[n[7]] = 0;
>
> - mnt->mnt_fsname = linebuf+n[0];
> - mnt->mnt_dir = linebuf+n[2];
> - mnt->mnt_type = linebuf+n[4];
> - mnt->mnt_opts = linebuf+n[6];
> + mnt->mnt_fsname = decode(linebuf+n[0]);
> + mnt->mnt_dir = decode(linebuf+n[2]);
> + mnt->mnt_type = decode(linebuf+n[4]);
> + mnt->mnt_opts = decode(linebuf+n[6]);
>
> return mnt;
> }
> @@ -69,12 +101,41 @@ struct mntent *getmntent(FILE *f)
> return getmntent_r(f, &mnt, SENTINEL, 0);
> }
>
> +static inline int write_string(FILE *f, const char* str)
> +{
> + char c;
> + int error_occured = 0;
> + while(str && !error_occured && (c = *str++) != 0) {
> + if(c == '\040') // space
> + error_occured = fprintf(f, "%s", "\\040") < 0;
> + else if (c == '\011') // tab
> + error_occured = fprintf(f, "%s", "\\011") < 0;
> + else if (c == '\012') // newline
> + error_occured = fprintf(f, "%s", "\\012") < 0;
> + else if (c == '\\')
> + error_occured = fprintf(f, "%s", "\\\\") < 0;
> + else
> + error_occured = fprintf(f, "%c", c) < 0;
> + }
> + return error_occured;
> +}
> +
> int addmntent(FILE *f, const struct mntent *mnt)
> {
> if (fseek(f, 0, SEEK_END)) return 1;
> - return fprintf(f, "%s\t%s\t%s\t%s\t%d\t%d\n",
> - mnt->mnt_fsname, mnt->mnt_dir, mnt->mnt_type, mnt->mnt_opts,
> - mnt->mnt_freq, mnt->mnt_passno) < 0;
the original code has a bug here: fprintf returns number of
chars printed but addmntent returns 0 to indicate success.
i think this should be fixed independently of the new feature.
(separate patch).
> + flockfile(f);
> + int result =
> + write_string(f, mnt->mnt_fsname)
> + || (fprintf(f, "\t") < 0)
> + || write_string(f, mnt->mnt_dir)
> + || (fprintf(f, "\t") < 0)
> + || write_string(f, mnt->mnt_type)
> + || (fprintf(f, "\t") < 0)
> + || write_string(f, mnt->mnt_opts)
> + || (fprintf(f, "\t%d\t%d\n",
> + mnt->mnt_freq, mnt->mnt_passno) < 0);
> + funlockfile(f);
this looks a bit ugly (recursive locks and char-by-char
printf) but i don't see anything wrong (again i would not
use inline).
> + return result;
> }
>
> char *hasmntopt(const struct mntent *mnt, const char *opt)
> --
> 2.40.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [musl] [PATCH] mntent: deal with escaped whitespace in mtab and fstab
2023-03-29 17:04 ` Szabolcs Nagy
@ 2023-03-29 17:16 ` Szabolcs Nagy
2023-03-30 8:14 ` Matthias Görgens
0 siblings, 1 reply; 9+ messages in thread
From: Szabolcs Nagy @ 2023-03-29 17:16 UTC (permalink / raw)
To: Matthias Goergens, musl
* Szabolcs Nagy <nsz@port70.net> [2023-03-29 19:04:18 +0200]:
> > - return fprintf(f, "%s\t%s\t%s\t%s\t%d\t%d\n",
> > - mnt->mnt_fsname, mnt->mnt_dir, mnt->mnt_type, mnt->mnt_opts,
> > - mnt->mnt_freq, mnt->mnt_passno) < 0;
>
> the original code has a bug here: fprintf returns number of
> chars printed but addmntent returns 0 to indicate success.
sorry missed the < 0.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [musl] [PATCH] mntent: deal with escaped whitespace in mtab and fstab
2023-03-29 17:16 ` Szabolcs Nagy
@ 2023-03-30 8:14 ` Matthias Görgens
2023-03-30 9:29 ` Pascal Cuoq
0 siblings, 1 reply; 9+ messages in thread
From: Matthias Görgens @ 2023-03-30 8:14 UTC (permalink / raw)
To: Matthias Goergens, musl
On Thu, 30 Mar 2023, 01:16 Szabolcs Nagy, <nsz@port70.net> wrote:
>
> * Szabolcs Nagy <nsz@port70.net> [2023-03-29 19:04:18 +0200]:
> > > - return fprintf(f, "%s\t%s\t%s\t%s\t%d\t%d\n",
> > > - mnt->mnt_fsname, mnt->mnt_dir, mnt->mnt_type, mnt->mnt_opts,
> > > - mnt->mnt_freq, mnt->mnt_passno) < 0;
> >
> > the original code has a bug here: fprintf returns number of
> > chars printed but addmntent returns 0 to indicate success.
>
> sorry missed the < 0.
I made that exact same error when first reading the code. I wonder whether we
should put the zero in front, to make it clearer to readers? Like `0 >
fprintf(...`
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [musl] [PATCH] mntent: deal with escaped whitespace in mtab and fstab
2023-03-30 8:14 ` Matthias Görgens
@ 2023-03-30 9:29 ` Pascal Cuoq
2023-03-30 13:53 ` Rich Felker
0 siblings, 1 reply; 9+ messages in thread
From: Pascal Cuoq @ 2023-03-30 9:29 UTC (permalink / raw)
To: musl; +Cc: Matthias Goergens
>> sorry missed the < 0.
>
> I made that exact same error when first reading the code. I wonder whether we
> should put the zero in front, to make it clearer to readers? Like `0 >
> fprintf(...`
As someone who reviews tons of C code from different authors and application domains, I expect the result of fprintf to be assigned to a variable and that variable to be used in the return statement. Anything else will give me pause or be confused for something that works when it doesn't.
Pascal
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [musl] [PATCH] mntent: deal with escaped whitespace in mtab and fstab
2023-03-30 9:29 ` Pascal Cuoq
@ 2023-03-30 13:53 ` Rich Felker
0 siblings, 0 replies; 9+ messages in thread
From: Rich Felker @ 2023-03-30 13:53 UTC (permalink / raw)
To: Pascal Cuoq; +Cc: musl, Matthias Goergens
On Thu, Mar 30, 2023 at 09:29:12AM +0000, Pascal Cuoq wrote:
>
>
> >> sorry missed the < 0.
> >
> > I made that exact same error when first reading the code. I wonder whether we
> > should put the zero in front, to make it clearer to readers? Like `0 >
> > fprintf(...`
>
> As someone who reviews tons of C code from different authors and
> application domains, I expect the result of fprintf to be assigned
> to a variable and that variable to be used in the return statement.
> Anything else will give me pause or be confused for something that
> works when it doesn't.
OK, but that's not generally a style we use in musl (introducing vars
to save a value that will only be used once immediately after it's
stored).
^ permalink raw reply [flat|nested] 9+ messages in thread
* [musl] [PATCH] mntent: deal with escaped whitespace in mtab and fstab
@ 2023-03-30 9:23 Matthias Goergens
0 siblings, 0 replies; 9+ messages in thread
From: Matthias Goergens @ 2023-03-30 9:23 UTC (permalink / raw)
To: musl; +Cc: Matthias Goergens
From glibc's documentation:
> Since fields in the mtab and fstab files are separated by whitespace,
> octal escapes are used to represent the characters space (\040),
> tab (\011), newline (\012), and backslash (\\) in those files when they
> occur in one of the four strings in a mntent structure. The
> routines addmntent() and getmntent() will convert from string
> representation to escaped representation and back. When converting
> from escaped representation, the sequence \134 is also converted to a
> backslash.
This fixes the issue reported in https://www.openwall.com/lists/musl/2021/12/14/1
--
This is a new version that incorporates suggestions by Szabolcs Nagy and rofl0r
The previous version had a helper function named `decode1` that decoded
a single escape. I implemented Szabolcs Nagy's extremely clever manual
inlining. However, I'm not completely sure that it's not too clever for
me. (It took me a while to understand why it works.)
---
src/misc/mntent.c | 85 +++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 78 insertions(+), 7 deletions(-)
diff --git a/src/misc/mntent.c b/src/misc/mntent.c
index d404fbe3..1c129579 100644
--- a/src/misc/mntent.c
+++ b/src/misc/mntent.c
@@ -1,8 +1,10 @@
+#include <assert.h>
#include <stdio.h>
#include <string.h>
#include <mntent.h>
#include <errno.h>
#include <limits.h>
+#include "stdio_impl.h"
static char *internal_buf;
static size_t internal_bufsize;
@@ -20,6 +22,46 @@ int endmntent(FILE *f)
return 1;
}
+static char* decode(char* buf) {
+ assert(buf != NULL);
+ char* src = buf;
+ char* dest = buf;
+ while (1) {
+ char* next_src = __strchrnul(src, '\\');
+ int offset = next_src - src;
+ memmove(dest, src, offset);
+ src = next_src;
+ dest += offset;
+
+ if(*src == '\0') {
+ *dest = *src;
+ return buf;
+ }
+ assert (*src == '\\');
+ src++;
+
+ const char *replacements =
+ "\040" "040" "\0" // space
+ "\011" "011" "\0" // tab
+ "\012" "012" "\0" // newline
+ "\134" "134" "\0" // backslash
+ "\\" "\\" "\0"
+ // Fallback for unrecognized escape sequence,
+ // copy literally:
+ "\\" "";
+ while(1) {
+ char c = *replacements++;
+ size_t n = strlen(replacements);
+ if (strncmp(src, replacements, n) == 0) {
+ *dest++ = c;
+ src += n;
+ break;
+ }
+ replacements += n+1;
+ }
+ }
+}
+
struct mntent *getmntent_r(FILE *f, struct mntent *mnt, char *linebuf, int buflen)
{
int n[8], use_internal = (linebuf == SENTINEL);
@@ -55,10 +97,10 @@ struct mntent *getmntent_r(FILE *f, struct mntent *mnt, char *linebuf, int bufle
linebuf[n[5]] = 0;
linebuf[n[7]] = 0;
- mnt->mnt_fsname = linebuf+n[0];
- mnt->mnt_dir = linebuf+n[2];
- mnt->mnt_type = linebuf+n[4];
- mnt->mnt_opts = linebuf+n[6];
+ mnt->mnt_fsname = decode(linebuf+n[0]);
+ mnt->mnt_dir = decode(linebuf+n[2]);
+ mnt->mnt_type = decode(linebuf+n[4]);
+ mnt->mnt_opts = decode(linebuf+n[6]);
return mnt;
}
@@ -69,12 +111,41 @@ struct mntent *getmntent(FILE *f)
return getmntent_r(f, &mnt, SENTINEL, 0);
}
+static int escape_and_write_string(FILE *f, const char* str)
+{
+ char c;
+ int error_occured = 0;
+ while(str && !error_occured && (c = *str++) != 0) {
+ if(c == '\040') // space
+ error_occured = fputs("\\040", f) < 0;
+ else if (c == '\011') // tab
+ error_occured = fputs("\\011", f) < 0;
+ else if (c == '\012') // newline
+ error_occured = fputs("\\012", f) < 0;
+ else if (c == '\\')
+ error_occured = fputs("\\\\", f) < 0;
+ else
+ error_occured = putc_unlocked(c, f) < 0;
+ }
+ return error_occured;
+}
+
int addmntent(FILE *f, const struct mntent *mnt)
{
if (fseek(f, 0, SEEK_END)) return 1;
- return fprintf(f, "%s\t%s\t%s\t%s\t%d\t%d\n",
- mnt->mnt_fsname, mnt->mnt_dir, mnt->mnt_type, mnt->mnt_opts,
- mnt->mnt_freq, mnt->mnt_passno) < 0;
+ FLOCK(f);
+ int error_occured =
+ escape_and_write_string(f, mnt->mnt_fsname)
+ || (0 > putc_unlocked('\t', f))
+ || escape_and_write_string(f, mnt->mnt_dir)
+ || (0 > putc_unlocked('\t', f))
+ || escape_and_write_string(f, mnt->mnt_type)
+ || (0 > putc_unlocked('\t', f))
+ || escape_and_write_string(f, mnt->mnt_opts)
+ || (0 > fprintf(f, "\t%d\t%d\n",
+ mnt->mnt_freq, mnt->mnt_passno));
+ FUNLOCK(f);
+ return error_occured;
}
char *hasmntopt(const struct mntent *mnt, const char *opt)
--
2.40.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [musl] [PATCH] mntent: deal with escaped whitespace in mtab and fstab
@ 2023-03-29 8:46 Matthias Goergens
0 siblings, 0 replies; 9+ messages in thread
From: Matthias Goergens @ 2023-03-29 8:46 UTC (permalink / raw)
To: musl; +Cc: Matthias Goergens
From glibc's documentation:
> Since fields in the mtab and fstab files are separated by whitespace,
> octal escapes are used to represent the characters space (\040),
> tab (\011), newline (\012), and backslash (\\) in those files when they
> occur in one of the four strings in a mntent structure. The
> routines addmntent() and getmntent() will convert from string
> representation to escaped representation and back. When converting
> from escaped representation, the sequence \134 is also converted to a
> backslash.
This fixes the issue reported in https://www.openwall.com/lists/musl/2021/12/14/1
Please pardon the previous broken patch that I tried to send directly
via gmail.
This is my first time contributing to musl. Please point out any ways
to improve. I probably got a few things wrong?
Thanks!
---
src/misc/mntent.c | 71 ++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 64 insertions(+), 7 deletions(-)
diff --git a/src/misc/mntent.c b/src/misc/mntent.c
index d404fbe3..527acc01 100644
--- a/src/misc/mntent.c
+++ b/src/misc/mntent.c
@@ -20,6 +20,34 @@ int endmntent(FILE *f)
return 1;
}
+static inline int decode1(char** in_buf, char** out_buf, const char* from, const char to) {
+ if(strncmp(from, *in_buf, strlen(from)) == 0) {
+ *(*out_buf)++ = to;
+ *in_buf += strlen(from);
+ return 1;
+ }
+ return 0;
+}
+
+static inline char* decode(char* buf) {
+ char* read_cursor = buf;
+ char* write_cursor = buf;
+ while(read_cursor && *read_cursor) {
+ // space
+ decode1(&read_cursor, &write_cursor, "\\040", '\040')
+ // tab
+ || decode1(&read_cursor, &write_cursor, "\\011", '\011')
+ // newline
+ || decode1(&read_cursor, &write_cursor, "\\012", '\012')
+ // backslash
+ || decode1(&read_cursor, &write_cursor, "\\134", '\134')
+ || decode1(&read_cursor, &write_cursor, "\\\\", '\\')
+ // default: copy char as is.
+ || (*write_cursor++ = *read_cursor++);
+ }
+ return buf;
+}
+
struct mntent *getmntent_r(FILE *f, struct mntent *mnt, char *linebuf, int buflen)
{
int n[8], use_internal = (linebuf == SENTINEL);
@@ -55,10 +83,10 @@ struct mntent *getmntent_r(FILE *f, struct mntent *mnt, char *linebuf, int bufle
linebuf[n[5]] = 0;
linebuf[n[7]] = 0;
- mnt->mnt_fsname = linebuf+n[0];
- mnt->mnt_dir = linebuf+n[2];
- mnt->mnt_type = linebuf+n[4];
- mnt->mnt_opts = linebuf+n[6];
+ mnt->mnt_fsname = decode(linebuf+n[0]);
+ mnt->mnt_dir = decode(linebuf+n[2]);
+ mnt->mnt_type = decode(linebuf+n[4]);
+ mnt->mnt_opts = decode(linebuf+n[6]);
return mnt;
}
@@ -69,12 +97,41 @@ struct mntent *getmntent(FILE *f)
return getmntent_r(f, &mnt, SENTINEL, 0);
}
+static inline int write_string(FILE *f, const char* str)
+{
+ char c;
+ int error_occured = 0;
+ while(str && !error_occured && (c = *str++) != 0) {
+ if(c == '\040') // space
+ error_occured = fprintf(f, "%s", "\\040") < 0;
+ else if (c == '\011') // tab
+ error_occured = fprintf(f, "%s", "\\011") < 0;
+ else if (c == '\012') // newline
+ error_occured = fprintf(f, "%s", "\\012") < 0;
+ else if (c == '\\')
+ error_occured = fprintf(f, "%s", "\\\\") < 0;
+ else
+ error_occured = fprintf(f, "%c", c) < 0;
+ }
+ return error_occured;
+}
+
int addmntent(FILE *f, const struct mntent *mnt)
{
if (fseek(f, 0, SEEK_END)) return 1;
- return fprintf(f, "%s\t%s\t%s\t%s\t%d\t%d\n",
- mnt->mnt_fsname, mnt->mnt_dir, mnt->mnt_type, mnt->mnt_opts,
- mnt->mnt_freq, mnt->mnt_passno) < 0;
+ flockfile(f);
+ int result =
+ write_string(f, mnt->mnt_fsname)
+ || (fprintf(f, "\t") < 0)
+ || write_string(f, mnt->mnt_dir)
+ || (fprintf(f, "\t") < 0)
+ || write_string(f, mnt->mnt_type)
+ || (fprintf(f, "\t") < 0)
+ || write_string(f, mnt->mnt_opts)
+ || (fprintf(f, "\t%d\t%d\n",
+ mnt->mnt_freq, mnt->mnt_passno) < 0);
+ funlockfile(f);
+ return result;
}
char *hasmntopt(const struct mntent *mnt, const char *opt)
--
2.40.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [musl] [PATCH] mntent: deal with escaped whitespace in mtab and fstab
@ 2023-03-29 8:38 Matthias Görgens
0 siblings, 0 replies; 9+ messages in thread
From: Matthias Görgens @ 2023-03-29 8:38 UTC (permalink / raw)
To: musl
From ca1001f856e314b0ea9d0f3a5d7c88106f151c2e Mon Sep 17 00:00:00 2001
From: Matthias Goergens <matthias.goergens@gmail.com>
Date: Wed, 29 Mar 2023 16:17:49 +0800
Subject: [PATCH] mntent: deal with escaped whitespace in mtab and fstab
From glibc's documentation:
> Since fields in the mtab and fstab files are separated by whitespace,
> octal escapes are used to represent the characters space (\040),
> tab (\011), newline (\012), and backslash (\\) in those files when they
> occur in one of the four strings in a mntent structure. The
> routines addmntent() and getmntent() will convert from string
> representation to escaped representation and back. When converting
> from escaped representation, the sequence \134 is also converted to a
> backslash.
This fixes the issue reported in
https://www.openwall.com/lists/musl/2021/12/14/1
This is my first time contributing to musl. Please point out any
mistakes you can find.
I am not quite sure I understand everything, and have also probably
gotten the style wrong?
Thanks!
---
src/misc/mntent.c | 71 ++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 64 insertions(+), 7 deletions(-)
diff --git a/src/misc/mntent.c b/src/misc/mntent.c
index d404fbe3..527acc01 100644
--- a/src/misc/mntent.c
+++ b/src/misc/mntent.c
@@ -20,6 +20,34 @@ int endmntent(FILE *f)
return 1;
}
+static inline int decode1(char** in_buf, char** out_buf, const char*
from, const char to) {
+ if(strncmp(from, *in_buf, strlen(from)) == 0) {
+ *(*out_buf)++ = to;
+ *in_buf += strlen(from);
+ return 1;
+ }
+ return 0;
+}
+
+static inline char* decode(char* buf) {
+ char* read_cursor = buf;
+ char* write_cursor = buf;
+ while(read_cursor && *read_cursor) {
+ // space
+ decode1(&read_cursor, &write_cursor, "\\040", '\040')
+ // tab
+ || decode1(&read_cursor, &write_cursor, "\\011", '\011')
+ // newline
+ || decode1(&read_cursor, &write_cursor, "\\012", '\012')
+ // backslash
+ || decode1(&read_cursor, &write_cursor, "\\134", '\134')
+ || decode1(&read_cursor, &write_cursor, "\\\\", '\\')
+ // default: copy char as is.
+ || (*write_cursor++ = *read_cursor++);
+ }
+ return buf;
+}
+
struct mntent *getmntent_r(FILE *f, struct mntent *mnt, char *linebuf,
int buflen)
{
int n[8], use_internal = (linebuf == SENTINEL);
@@ -55,10 +83,10 @@ struct mntent *getmntent_r(FILE *f, struct mntent
*mnt, char *linebuf, int bufle
linebuf[n[5]] = 0;
linebuf[n[7]] = 0;
- mnt->mnt_fsname = linebuf+n[0];
- mnt->mnt_dir = linebuf+n[2];
- mnt->mnt_type = linebuf+n[4];
- mnt->mnt_opts = linebuf+n[6];
+ mnt->mnt_fsname = decode(linebuf+n[0]);
+ mnt->mnt_dir = decode(linebuf+n[2]);
+ mnt->mnt_type = decode(linebuf+n[4]);
+ mnt->mnt_opts = decode(linebuf+n[6]);
return mnt;
}
@@ -69,12 +97,41 @@ struct mntent *getmntent(FILE *f)
return getmntent_r(f, &mnt, SENTINEL, 0);
}
+static inline int write_string(FILE *f, const char* str)
+{
+ char c;
+ int error_occured = 0;
+ while(str && !error_occured && (c = *str++) != 0) {
+ if(c == '\040') // space
+ error_occured = fprintf(f, "%s", "\\040") < 0;
+ else if (c == '\011') // tab
+ error_occured = fprintf(f, "%s", "\\011") < 0;
+ else if (c == '\012') // newline
+ error_occured = fprintf(f, "%s", "\\012") < 0;
+ else if (c == '\\')
+ error_occured = fprintf(f, "%s", "\\\\") < 0;
+ else
+ error_occured = fprintf(f, "%c", c) < 0;
+ }
+ return error_occured;
+}
+
int addmntent(FILE *f, const struct mntent *mnt)
{
if (fseek(f, 0, SEEK_END)) return 1;
- return fprintf(f, "%s\t%s\t%s\t%s\t%d\t%d\n",
- mnt->mnt_fsname, mnt->mnt_dir, mnt->mnt_type, mnt->mnt_opts,
- mnt->mnt_freq, mnt->mnt_passno) < 0;
+ flockfile(f);
+ int result =
+ write_string(f, mnt->mnt_fsname)
+ || (fprintf(f, "\t") < 0)
+ || write_string(f, mnt->mnt_dir)
+ || (fprintf(f, "\t") < 0)
+ || write_string(f, mnt->mnt_type)
+ || (fprintf(f, "\t") < 0)
+ || write_string(f, mnt->mnt_opts)
+ || (fprintf(f, "\t%d\t%d\n",
+ mnt->mnt_freq, mnt->mnt_passno) < 0);
+ funlockfile(f);
+ return result;
}
char *hasmntopt(const struct mntent *mnt, const char *opt)
--
2.40.0
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2023-03-30 13:53 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-29 15:17 [musl] [PATCH] mntent: deal with escaped whitespace in mtab and fstab Matthias Goergens
2023-03-29 17:04 ` Szabolcs Nagy
2023-03-29 17:16 ` Szabolcs Nagy
2023-03-30 8:14 ` Matthias Görgens
2023-03-30 9:29 ` Pascal Cuoq
2023-03-30 13:53 ` Rich Felker
-- strict thread matches above, loose matches on Subject: below --
2023-03-30 9:23 Matthias Goergens
2023-03-29 8:46 Matthias Goergens
2023-03-29 8:38 Matthias Görgens
Code repositories for project(s) associated with this public inbox
https://git.vuxu.org/mirror/musl/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).