Linux-man Archive on lore.kernel.org
 help / color / Atom feed
* [patch] strtok.3 add note about glibc-2.25+ segfault for call like strtok_r(NULL,"|",NULL)
@ 2020-04-25 12:52 Marcin Stolarek
  2020-04-25 16:15 ` Michael Kerrisk (man-pages)
  0 siblings, 1 reply; 3+ messages in thread
From: Marcin Stolarek @ 2020-04-25 12:52 UTC (permalink / raw)
  To: mtk.manpages; +Cc: linux-man, wdijkstr


[-- Attachment #1: Type: text/plain, Size: 1003 bytes --]

The change in glibc commit d58ab810a6e325cc351684d174c48cabce01bcc1
(author in CC):

From commit description:"[...] Also avoid an unnecessary call to
strcspn after the last token by adding an early exit for an empty
string.[...]"

Important code change:
/* Parse S into tokens separated by characters in DELIM.
@@ -45,11 +41,17 @@
 char *
 __strtok_r (char *s, const char *delim, char **save_ptr)
 {
-  char *token;
+  char *end;

   if (s == NULL)
     s = *save_ptr;

+  if (*s == '\0')
+    {
+      *save_ptr = s;
+      return NULL;
+    }
+

may result in the mentioned segmentation fault if the char *str passed
to strtok_r is a NULL (for 1st call).

Checked glibc versions:
~/git-repos/glibc:release/2.25/master$ git tag
--contain=d58ab810a6e325cc351684d174c48cabce01bcc1
changelog-ends-here
glibc-2.25
glibc-2.25.90
glibc-2.26
glibc-2.26.9000
glibc-2.27
glibc-2.27.9000
glibc-2.28
glibc-2.28.9000
glibc-2.29
glibc-2.29.9000
glibc-2.30
glibc-2.30.9000
glibc-2.31
glibc-2.31.9000

cheers,
Marcin

[-- Attachment #2: 0001-Add-note-about-strtok_r-change-in-glibc-2.25.patch --]
[-- Type: text/x-patch, Size: 936 bytes --]

From cdbe9daffdb36400c23a1cb47acd7252d2ad434a Mon Sep 17 00:00:00 2001
From: Marcin Stolarek <stolarek.marcin@gmail.com>
Date: Sat, 25 Apr 2020 14:41:24 +0200
Subject: [PATCH 1/1] Add note about strtok_r() change in glibc-2.25

Calling strtok(NULL,',',NULL) after d58ab810a6e325cc351684d174c48cabce01bcc1
will result in NULL pointer dereference.
---
 man3/strtok.3 | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/man3/strtok.3 b/man3/strtok.3
index 933a7b96c..8754a0216 100644
--- a/man3/strtok.3
+++ b/man3/strtok.3
@@ -197,6 +197,11 @@ is required to be NULL on the first call to
 .BR strtok_r ()
 that is being used to parse
 .IR str .
+.\" glibc-2.25 d58ab810a6e325cc351684d174c48cabce01bcc1
+Since glibc-2.25 using
+.BR strtok_r ()
+with str set to NULL (for the 1st call) is not allowed and may result in
+segmentation fault.
 .SH BUGS
 Be cautious when using these functions.
 If you do use them, note that:
-- 
2.17.1


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

* Re: [patch] strtok.3 add note about glibc-2.25+ segfault for call like strtok_r(NULL,"|",NULL)
  2020-04-25 12:52 [patch] strtok.3 add note about glibc-2.25+ segfault for call like strtok_r(NULL,"|",NULL) Marcin Stolarek
@ 2020-04-25 16:15 ` Michael Kerrisk (man-pages)
       [not found]   ` <CAC8K6BP1TFVjQRxs873FUCBNdOVNAkb+d1exd41Q_CTUvCjnSw@mail.gmail.com>
  0 siblings, 1 reply; 3+ messages in thread
From: Michael Kerrisk (man-pages) @ 2020-04-25 16:15 UTC (permalink / raw)
  To: Marcin Stolarek; +Cc: linux-man, wdijkstr

On Sat, 25 Apr 2020 at 14:52, Marcin Stolarek <stolarek.marcin@gmail.com> wrote:
>
> The change in glibc commit d58ab810a6e325cc351684d174c48cabce01bcc1
> (author in CC):
>
> From commit description:"[...] Also avoid an unnecessary call to
> strcspn after the last token by adding an early exit for an empty
> string.[...]"
>
> Important code change:
> /* Parse S into tokens separated by characters in DELIM.
> @@ -45,11 +41,17 @@
>  char *
>  __strtok_r (char *s, const char *delim, char **save_ptr)
>  {
> -  char *token;
> +  char *end;
>
>    if (s == NULL)
>      s = *save_ptr;
>
> +  if (*s == '\0')
> +    {
> +      *save_ptr = s;
> +      return NULL;
> +    }
> +
>
> may result in the mentioned segmentation fault if the char *str passed
> to strtok_r is a NULL (for 1st call).

I'm a little puzzled here. Getting a segfault if the first argument to
strtok_r() is NULL on the first call seems not so surprising to me.
Why would you expect otherwise? What am I missing?

Thanks,

Michael

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

* Re: [patch] strtok.3 add note about glibc-2.25+ segfault for call like strtok_r(NULL,"|",NULL)
       [not found]   ` <CAC8K6BP1TFVjQRxs873FUCBNdOVNAkb+d1exd41Q_CTUvCjnSw@mail.gmail.com>
@ 2020-04-25 16:54     ` Wilco Dijkstra
  0 siblings, 0 replies; 3+ messages in thread
From: Wilco Dijkstra @ 2020-04-25 16:54 UTC (permalink / raw)
  To: Marcin Stolarek, mtk.manpages; +Cc: linux-man

Hi,

> It worked fine prior to 2.25, since it's a change I thoight it may be nice to document that this set
> of arguments will segfault and has to be avoided by calling app itself.

Did it ever? Even much older versions have always accessed *save_ptr with a NULL input, so a crash
is guaranteed. And *s is also always accessed, so it had better never be NULL. Eg. this is what it did
back in 2007 ([1]):

  if (s == NULL)
    s = *save_ptr;  // crash here

  /* Scan leading delimiters.  */
  s += strspn (s, delim);
  if (*s == '\0')
    {
      *save_ptr = s;
      return NULL;
    }

Cheers,
Wilco

[1] https://github.com/bminor/glibc/blob/0ecb606cb6cf65de1d9fc8a919bceb4be476c602/string/strtok_r.c

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

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

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-25 12:52 [patch] strtok.3 add note about glibc-2.25+ segfault for call like strtok_r(NULL,"|",NULL) Marcin Stolarek
2020-04-25 16:15 ` Michael Kerrisk (man-pages)
     [not found]   ` <CAC8K6BP1TFVjQRxs873FUCBNdOVNAkb+d1exd41Q_CTUvCjnSw@mail.gmail.com>
2020-04-25 16:54     ` Wilco Dijkstra

Linux-man Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-man/0 linux-man/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-man linux-man/ https://lore.kernel.org/linux-man \
		linux-man@vger.kernel.org
	public-inbox-index linux-man

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-man


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git