* Correct access attribute on memfrob (bug 28475)
@ 2021-10-19 17:24 Joseph Myers
2021-10-19 19:48 ` Florian Weimer
0 siblings, 1 reply; 4+ messages in thread
From: Joseph Myers @ 2021-10-19 17:24 UTC (permalink / raw)
To: libc-alpha
As noted in bug 28475, the access attribute on memfrob in <string.h>
is incorrect: the function both reads and writes the memory pointed to
by its argument, so it needs to use __read_write__, not
__write_only__. This incorrect attribute results in a build failure
for accessing uninitialized memory for s390x-linux-gnu-O3 with
build-many-glibcs.py using GCC mainline.
Correct the attribute. Fixing this shows up that some calls to
memfrob in elf/ tests are reading uninitialized memory; I'm not
entirely sure of the purpose of those calls, but guess they are about
ensuring that the stack space is indeed allocated at that point in the
function, and so it matters that they are calling a function whose
semantics are unknown to the compiler. Thus, add a memset call before
the memfrob call in those tests to avoid the use of uninitialized
memory.
Tested for x86_64, and with build-many-glibcs.py (GCC mainline) for
s390x-linux-gnu-O3.
diff --git a/elf/tst-execstack-needed.c b/elf/tst-execstack-needed.c
index 8b794a3d47..a42597a4c1 100644
--- a/elf/tst-execstack-needed.c
+++ b/elf/tst-execstack-needed.c
@@ -26,6 +26,7 @@ static void
deeper (void (*f) (void))
{
char stack[1100 * 1024];
+ memset (stack, 123, sizeof stack);
memfrob (stack, sizeof stack);
(*f) ();
memfrob (stack, sizeof stack);
diff --git a/elf/tst-execstack-prog.c b/elf/tst-execstack-prog.c
index 8663153372..40d4d00858 100644
--- a/elf/tst-execstack-prog.c
+++ b/elf/tst-execstack-prog.c
@@ -25,6 +25,7 @@ static void
deeper (void (*f) (void))
{
char stack[1100 * 1024];
+ memset (stack, 123, sizeof stack);
memfrob (stack, sizeof stack);
(*f) ();
memfrob (stack, sizeof stack);
diff --git a/elf/tst-execstack.c b/elf/tst-execstack.c
index 114f341d76..f9a6509be0 100644
--- a/elf/tst-execstack.c
+++ b/elf/tst-execstack.c
@@ -227,6 +227,7 @@ static void
deeper (void (*f) (void))
{
char stack[1100 * 1024];
+ memset (stack, 123, sizeof stack);
memfrob (stack, sizeof stack);
(*f) ();
memfrob (stack, sizeof stack);
diff --git a/string/string.h b/string/string.h
index 04e1b7067d..201d7e31c8 100644
--- a/string/string.h
+++ b/string/string.h
@@ -495,7 +495,7 @@ extern char *strfry (char *__string) __THROW __nonnull ((1));
/* Frobnicate N bytes of S. */
extern void *memfrob (void *__s, size_t __n) __THROW __nonnull ((1))
- __attr_access ((__write_only__, 1, 2));
+ __attr_access ((__read_write__, 1, 2));
# ifndef basename
/* Return the file name within directory of FILENAME. We don't
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: Correct access attribute on memfrob (bug 28475)
2021-10-19 17:24 Correct access attribute on memfrob (bug 28475) Joseph Myers
@ 2021-10-19 19:48 ` Florian Weimer
2021-10-19 20:56 ` [PATCH v2] " Joseph Myers
0 siblings, 1 reply; 4+ messages in thread
From: Florian Weimer @ 2021-10-19 19:48 UTC (permalink / raw)
To: Joseph Myers; +Cc: libc-alpha
* Joseph Myers:
> As noted in bug 28475, the access attribute on memfrob in <string.h>
> is incorrect: the function both reads and writes the memory pointed to
> by its argument, so it needs to use __read_write__, not
> __write_only__. This incorrect attribute results in a build failure
> for accessing uninitialized memory for s390x-linux-gnu-O3 with
> build-many-glibcs.py using GCC mainline.
> Correct the attribute.
That part looks okay.
> Fixing this shows up that some calls to memfrob in elf/ tests are
> reading uninitialized memory; I'm not entirely sure of the purpose of
> those calls, but guess they are about ensuring that the stack space is
> indeed allocated at that point in the function, and so it matters that
> they are calling a function whose semantics are unknown to the
> compiler. Thus, add a memset call before the memfrob call in those
> tests to avoid the use of uninitialized memory.
Using explicit_bzero would be more idomatic, I think.
Florian
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v2] Correct access attribute on memfrob (bug 28475)
2021-10-19 19:48 ` Florian Weimer
@ 2021-10-19 20:56 ` Joseph Myers
2021-10-20 7:33 ` Florian Weimer
0 siblings, 1 reply; 4+ messages in thread
From: Joseph Myers @ 2021-10-19 20:56 UTC (permalink / raw)
To: Florian Weimer; +Cc: libc-alpha
As noted in bug 28475, the access attribute on memfrob in <string.h>
is incorrect: the function both reads and writes the memory pointed to
by its argument, so it needs to use __read_write__, not
__write_only__. This incorrect attribute results in a build failure
for accessing uninitialized memory for s390x-linux-gnu-O3 with
build-many-glibcs.py using GCC mainline.
Correct the attribute. Fixing this shows up that some calls to
memfrob in elf/ tests are reading uninitialized memory; I'm not
entirely sure of the purpose of those calls, but guess they are about
ensuring that the stack space is indeed allocated at that point in the
function, and so it matters that they are calling a function whose
semantics are unknown to the compiler. Thus, change the first memfrob
call in those tests to use explicit_bzero instead, as suggested by
Florian in
<https://sourceware.org/pipermail/libc-alpha/2021-October/132119.html>,
to avoid the use of uninitialized memory.
Tested for x86_64, and with build-many-glibcs.py (GCC mainline) for
s390x-linux-gnu-O3.
---
Changed in version 2 to use explicit_bzero in the elf/ tests as
suggested by Florian.
diff --git a/elf/tst-execstack-needed.c b/elf/tst-execstack-needed.c
index 8b794a3d47..85078e40ef 100644
--- a/elf/tst-execstack-needed.c
+++ b/elf/tst-execstack-needed.c
@@ -26,7 +26,7 @@ static void
deeper (void (*f) (void))
{
char stack[1100 * 1024];
- memfrob (stack, sizeof stack);
+ explicit_bzero (stack, sizeof stack);
(*f) ();
memfrob (stack, sizeof stack);
}
diff --git a/elf/tst-execstack-prog.c b/elf/tst-execstack-prog.c
index 8663153372..1b34bb5597 100644
--- a/elf/tst-execstack-prog.c
+++ b/elf/tst-execstack-prog.c
@@ -25,7 +25,7 @@ static void
deeper (void (*f) (void))
{
char stack[1100 * 1024];
- memfrob (stack, sizeof stack);
+ explicit_bzero (stack, sizeof stack);
(*f) ();
memfrob (stack, sizeof stack);
}
diff --git a/elf/tst-execstack.c b/elf/tst-execstack.c
index 114f341d76..7e898b4f58 100644
--- a/elf/tst-execstack.c
+++ b/elf/tst-execstack.c
@@ -227,7 +227,7 @@ static void
deeper (void (*f) (void))
{
char stack[1100 * 1024];
- memfrob (stack, sizeof stack);
+ explicit_bzero (stack, sizeof stack);
(*f) ();
memfrob (stack, sizeof stack);
}
diff --git a/string/string.h b/string/string.h
index 04e1b7067d..201d7e31c8 100644
--- a/string/string.h
+++ b/string/string.h
@@ -495,7 +495,7 @@ extern char *strfry (char *__string) __THROW __nonnull ((1));
/* Frobnicate N bytes of S. */
extern void *memfrob (void *__s, size_t __n) __THROW __nonnull ((1))
- __attr_access ((__write_only__, 1, 2));
+ __attr_access ((__read_write__, 1, 2));
# ifndef basename
/* Return the file name within directory of FILENAME. We don't
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v2] Correct access attribute on memfrob (bug 28475)
2021-10-19 20:56 ` [PATCH v2] " Joseph Myers
@ 2021-10-20 7:33 ` Florian Weimer
0 siblings, 0 replies; 4+ messages in thread
From: Florian Weimer @ 2021-10-20 7:33 UTC (permalink / raw)
To: Joseph Myers; +Cc: libc-alpha
* Joseph Myers:
> diff --git a/elf/tst-execstack-needed.c b/elf/tst-execstack-needed.c
> index 8b794a3d47..85078e40ef 100644
> --- a/elf/tst-execstack-needed.c
> +++ b/elf/tst-execstack-needed.c
> @@ -26,7 +26,7 @@ static void
> deeper (void (*f) (void))
> {
> char stack[1100 * 1024];
> - memfrob (stack, sizeof stack);
> + explicit_bzero (stack, sizeof stack);
> (*f) ();
> memfrob (stack, sizeof stack);
> }
> diff --git a/elf/tst-execstack-prog.c b/elf/tst-execstack-prog.c
> index 8663153372..1b34bb5597 100644
> --- a/elf/tst-execstack-prog.c
> +++ b/elf/tst-execstack-prog.c
> @@ -25,7 +25,7 @@ static void
> deeper (void (*f) (void))
> {
> char stack[1100 * 1024];
> - memfrob (stack, sizeof stack);
> + explicit_bzero (stack, sizeof stack);
> (*f) ();
> memfrob (stack, sizeof stack);
> }
> diff --git a/elf/tst-execstack.c b/elf/tst-execstack.c
> index 114f341d76..7e898b4f58 100644
> --- a/elf/tst-execstack.c
> +++ b/elf/tst-execstack.c
> @@ -227,7 +227,7 @@ static void
> deeper (void (*f) (void))
> {
> char stack[1100 * 1024];
> - memfrob (stack, sizeof stack);
> + explicit_bzero (stack, sizeof stack);
> (*f) ();
> memfrob (stack, sizeof stack);
> }
> diff --git a/string/string.h b/string/string.h
> index 04e1b7067d..201d7e31c8 100644
> --- a/string/string.h
> +++ b/string/string.h
> @@ -495,7 +495,7 @@ extern char *strfry (char *__string) __THROW __nonnull ((1));
>
> /* Frobnicate N bytes of S. */
> extern void *memfrob (void *__s, size_t __n) __THROW __nonnull ((1))
> - __attr_access ((__write_only__, 1, 2));
> + __attr_access ((__read_write__, 1, 2));
>
> # ifndef basename
> /* Return the file name within directory of FILENAME. We don't
Patch looks good.
Thanks,
Florian
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-10-20 7:33 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-19 17:24 Correct access attribute on memfrob (bug 28475) Joseph Myers
2021-10-19 19:48 ` Florian Weimer
2021-10-19 20:56 ` [PATCH v2] " Joseph Myers
2021-10-20 7:33 ` Florian Weimer
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 read-only IMAP folder(s) and NNTP newsgroup(s).