mailing list of musl libc
 help / color / mirror / code / Atom feed
* [musl] [PATCH v2] fix: Truncate the too-long mntent in function getmntent_r
@ 2021-10-15 12:20 Kaihang Zhang
  2021-12-01 12:33 ` [musl] " Kaihang Zhang
  2021-12-01 15:24 ` [musl] " Rich Felker
  0 siblings, 2 replies; 3+ messages in thread
From: Kaihang Zhang @ 2021-10-15 12:20 UTC (permalink / raw)
  To: musl, 2010267516; +Cc: Kaihang Zhang

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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* [musl] Re: [PATCH v2] fix: Truncate the too-long mntent in function getmntent_r
  2021-10-15 12:20 [musl] [PATCH v2] fix: Truncate the too-long mntent in function getmntent_r Kaihang Zhang
@ 2021-12-01 12:33 ` Kaihang Zhang
  2021-12-01 15:24 ` [musl] " Rich Felker
  1 sibling, 0 replies; 3+ messages in thread
From: Kaihang Zhang @ 2021-12-01 12:33 UTC (permalink / raw)
  To: musl, care

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@smartx.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@smartx.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)工作邮箱. 如本邮箱发出的邮件与工作无关,该邮件未得到本公司任何的明示或默示的授权.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [musl] [PATCH v2] fix: Truncate the too-long mntent in function getmntent_r
  2021-10-15 12:20 [musl] [PATCH v2] fix: Truncate the too-long mntent in function getmntent_r Kaihang Zhang
  2021-12-01 12:33 ` [musl] " Kaihang Zhang
@ 2021-12-01 15:24 ` Rich Felker
  1 sibling, 0 replies; 3+ messages in thread
From: Rich Felker @ 2021-12-01 15:24 UTC (permalink / raw)
  To: Kaihang Zhang; +Cc: musl, 2010267516

On Fri, Oct 15, 2021 at 08:20:00AM -0400, Kaihang Zhang 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.

Thanks for resending this. I know I've neglected to take a good look
at it so far, and I'll try to get to it right away this time.

Rich

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-12-01 15:24 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-15 12:20 [musl] [PATCH v2] fix: Truncate the too-long mntent in function getmntent_r Kaihang Zhang
2021-12-01 12:33 ` [musl] " Kaihang Zhang
2021-12-01 15:24 ` [musl] " Rich Felker

Code repositories for project(s) associated with this 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).