[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Wed, 1 Dec 2021 20:33:58 +0800
From: Kaihang Zhang <kaihang.zhang@...rtx.com>
To: musl@...ts.openwall.com, care <2010267516@...com>
Subject: Re: [PATCH v2] fix: Truncate the too-long mntent in function getmntent_r
The behavior of getmntent_r in glibc. If the buffer is too samll for a
line, the line will be truncated, glibc uses wrong data for this line.
However, there is no opportunity for the developer to realise a
mistake they made by supplying too small buffer, hence there is no
chance of recovering from it. The user gets the wrong data and doesn't
realize it.
The behavior of getmntent_r in musl libc. If the buffer is too small
for a line, it will set errno to ERANGE and then exit. And the user
can retry with a bigger buffer.
However, the current interface doesn't allow trying again to read the
too-long entry, since it will just have moved to the next one, the
too-long entry data will be lost. Maybe fseek(f, -fgets_result,
SEEK_CUR) could be used to rewind until the start of the line, but the
function would just loop eternally with too short a buffer.
For example, a mount entry has 2000 bytes, the size of buffer is 256
bytes, if errno is ERANGE i will retry use buffer size 512, 768, 1024,
1280, 1536, 1792, 2048... I have to try at least 8 times to get the
next mount entries.
In my opinion, it's ok to truncate the too-long entry (musl libc will
even lose it ! ) instead exiting. And the errno will be set to ERANGE
in order to let uesr realise this mistake, but as i know the user can
only retry from the beginning of the file, that bothers me somehow.
It‘s up to the user to decide to retry or not. If the user doesn't
care about the too-long entry or the discarded contents of it, there
is no need to retry. Othrewise, retry from the beginning of the file.
On Fri, Oct 15, 2021 at 8:20 PM Kaihang Zhang <kaihang.zhang@...rtx.com> wrote:
>
> In function getmntent_r in source misc/mntent.c, entry that is too long
> will be truncated rather than discarded. The caller can tell by errno
> whether the supplied buffer is too small, and retry from the beginning
> of the file.
> ---
> src/misc/mntent.c | 53 +++++++++++++++++++++++++++++------------------
> 1 file changed, 33 insertions(+), 20 deletions(-)
>
> diff --git a/src/misc/mntent.c b/src/misc/mntent.c
> index eabb8200..085ce45d 100644
> --- a/src/misc/mntent.c
> +++ b/src/misc/mntent.c
> @@ -21,12 +21,12 @@ int endmntent(FILE *f)
>
> struct mntent *getmntent_r(FILE *f, struct mntent *mnt, char *linebuf, int buflen)
> {
> - int cnt, n[8], use_internal = (linebuf == SENTINEL);
> -
> - mnt->mnt_freq = 0;
> - mnt->mnt_passno = 0;
> + int use_internal = (linebuf == SENTINEL);
> + char *sub;
>
> do {
> + char *end_ptr;
> +
> if (use_internal) {
> getline(&internal_buf, &internal_bufsize, f);
> linebuf = internal_buf;
> @@ -34,25 +34,38 @@ struct mntent *getmntent_r(FILE *f, struct mntent *mnt, char *linebuf, int bufle
> fgets(linebuf, buflen, f);
> }
> if (feof(f) || ferror(f)) return 0;
> - if (!strchr(linebuf, '\n')) {
> +
> + end_ptr = strchr(linebuf, '\n');
> + if (end_ptr != NULL) {
> + while ((end_ptr[-1] == ' ' || end_ptr[-1] == '\t') && end_ptr != linebuf) end_ptr--;
> + *end_ptr = '\0';
> + } else {
> fscanf(f, "%*[^\n]%*[\n]");
> errno = ERANGE;
> - return 0;
> }
> - cnt = sscanf(linebuf, " %n%*s%n %n%*s%n %n%*s%n %n%*s%n %d %d",
> - n, n+1, n+2, n+3, n+4, n+5, n+6, n+7,
> - &mnt->mnt_freq, &mnt->mnt_passno);
> - } while (cnt < 2 || linebuf[n[0]] == '#');
> -
> - linebuf[n[1]] = 0;
> - linebuf[n[3]] = 0;
> - 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];
> +
> + linebuf += strspn(linebuf, " \t");
> + } while (linebuf[0] == '\0' || linebuf[0] == '#');
> +
> + mnt->mnt_fsname = strsep(&linebuf, " \t");
> +
> + if (linebuf) linebuf += strspn(linebuf, " \t");
> + sub = strsep(&linebuf, " \t");
> + mnt->mnt_dir = sub ? sub : (char *) "";
> +
> + if (linebuf) linebuf += strspn(linebuf, " \t");
> + sub = strsep (&linebuf, " \t");
> + mnt->mnt_type = sub ? sub : (char *) "";
> +
> + if (linebuf) linebuf += strspn(linebuf, " \t");
> + sub = strsep(&linebuf, " \t");
> + mnt->mnt_opts = sub ? sub : (char *) "";
> +
> + switch (linebuf ? sscanf(linebuf, " %d %d", &mnt->mnt_freq, &mnt->mnt_passno) : 0) {
> + case 0: mnt->mnt_freq = 0;
> + case 1: mnt->mnt_passno = 0;
> + case 2: break;
> + }
>
> return mnt;
> }
> --
> 2.25.4
>
--
mail：kaihang.zhang@...rtx.com
tell：15196469611
The SmartX email address is only for business purpose. Any sent
message that is not related to the business is not authorized or
permitted by SmartX.
本邮箱为北京志凌海纳科技有限公司（SmartX）工作邮箱. 如本邮箱发出的邮件与工作无关,该邮件未得到本公司任何的明示或默示的授权.
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
