free(3) const void *

free(3) const void *

[Alejandro Colomar]

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

Hi,

ISO C and POSIX say that free(3) shall take a `void *`.  But free() does
nothing to the pointee during its lifetime.  Of course, the lifetime of
the object ends right at the boundary of the call to free() itself.  And
even if the lifetime of the object would be extended to the end of
free(), it doesn't really modify the pointee internally; it only uses
the pointer to know which memory it is freeing, but never writes to it.
And even if some implementation would want to write to it for some
reason, it'd be as easy as discarding `const` internally.

It is sometimes (often?) useful to allocate some object, write
immediately to it, and then use it read-only.  The standard definition
of free() forces the programmer to keep a non-const pointer around just
for the sake of freeing, which is unnecessarily dangerous.

Since a `const void *` will accept anything that a `void *` would accept
(and more), how about changing the prototype of free() in glibc as an
extension to the language?

I'd like to refor free(3) to be:

	void free(const void *p);

But this would break function pointers.  I don't know if there's any way
to avoid that.  What was done when const was added to string functions?
Was it considered an acceptable breaking change?

	$ cat fp.c 
	#include <stdlib.h>

	typedef void (*free_t)(void *p);

	extern void free_const(const void *p);

	int
	main(void)
	{
		free_t  fp;

		fp = free;
		fp = free_const;
	}
	$ cc -Wall -Wextra fp.c 
	fp.c: In function ‘main’:
	fp.c:13:12: warning: assignment to ‘free_t’ {aka ‘void (*)(void *)’} from incompatible pointer type ‘void (*)(const void *)’ [-Wincompatible-pointer-types]
	   13 |         fp = free_const;
	      |            ^
	fp.c:10:17: warning: variable ‘fp’ set but not used [-Wunused-but-set-variable]
	   10 |         free_t  fp;
	      |                 ^~
	/usr/bin/ld: /tmp/ccVlhr05.o: in function `main':
	fp.c:(.text+0x12): undefined reference to `free_const'
	collect2: error: ld returned 1 exit status

Maybe we could add a function-like macro so that direct calls with a
`const` pointer use the macro and don't get a warning, but one can still
take the address of the function and it will use the standard prototype:

	$ cat fp.c 
	#include <stdlib.h>
	#include <string.h>

	typedef void (*free_t)(void *p);

	#define free(p)  free((void *) &*p)

	int
	main(void)
	{
		free_t      fp;
		const char  *s = strdup("foo");

		fp = free;
		(void) fp;
		free(s);
	}
	$ cc -Wall -Wextra fp.c 
	$

Does this sound reasonable?

Cheers,
Alex

-- 
<https://www.alejandro-colomar.es/>
Looking for a remote C programming job at the moment.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: free(3) const void *

[Arsen Arsenović]

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

Hi Alex,

Alejandro Colomar <alx@kernel.org> writes:

> Hi,
>
> ISO C and POSIX say that free(3) shall take a `void *`.  But free() does
> nothing to the pointee during its lifetime.  Of course, the lifetime of
> the object ends right at the boundary of the call to free() itself.  And
> even if the lifetime of the object would be extended to the end of
> free(), it doesn't really modify the pointee internally; it only uses
> the pointer to know which memory it is freeing, but never writes to it.
> And even if some implementation would want to write to it for some
> reason, it'd be as easy as discarding `const` internally.
>
> It is sometimes (often?) useful to allocate some object, write
> immediately to it, and then use it read-only.  The standard definition
> of free() forces the programmer to keep a non-const pointer around just
> for the sake of freeing, which is unnecessarily dangerous.
>
> Since a `const void *` will accept anything that a `void *` would accept
> (and more), how about changing the prototype of free() in glibc as an
> extension to the language?

But, free() modifies the object passed to it (even if not its bit
representation) by freeing it.  Freeing const-passed objects would also
violate the constness promise, so I disagree that free should take const
void*.

> I'd like to refor free(3) to be:
>
> 	void free(const void *p);
>
> But this would break function pointers.  I don't know if there's any way
> to avoid that.  What was done when const was added to string functions?
> Was it considered an acceptable breaking change?
>
> 	$ cat fp.c
> 	#include <stdlib.h>
>
> 	typedef void (*free_t)(void *p);
>
> 	extern void free_const(const void *p);
>
> 	int
> 	main(void)
> 	{
> 		free_t  fp;
>
> 		fp = free;
> 		fp = free_const;
> 	}
> 	$ cc -Wall -Wextra fp.c
> 	fp.c: In function ‘main’:
> 	fp.c:13:12: warning: assignment to ‘free_t’ {aka ‘void (*)(void *)’} from incompatible pointer type ‘void (*)(const void *)’ [-Wincompatible-pointer-types]
> 	   13 |         fp = free_const;
> 	      |            ^
> 	fp.c:10:17: warning: variable ‘fp’ set but not used [-Wunused-but-set-variable]
> 	   10 |         free_t  fp;
> 	      |                 ^~
> 	/usr/bin/ld: /tmp/ccVlhr05.o: in function `main':
> 	fp.c:(.text+0x12): undefined reference to `free_const'
> 	collect2: error: ld returned 1 exit status
>
> Maybe we could add a function-like macro so that direct calls with a
> `const` pointer use the macro and don't get a warning, but one can still
> take the address of the function and it will use the standard prototype:
>
> 	$ cat fp.c
> 	#include <stdlib.h>
> 	#include <string.h>
>
> 	typedef void (*free_t)(void *p);
>
> 	#define free(p)  free((void *) &*p)
>
> 	int
> 	main(void)
> 	{
> 		free_t      fp;
> 		const char  *s = strdup("foo");
>
> 		fp = free;
> 		(void) fp;
> 		free(s);
> 	}
> 	$ cc -Wall -Wextra fp.c
> 	$
>
> Does this sound reasonable?
>
> Cheers,
> Alex


--
Arsen Arsenović

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 251 bytes --]

Re: free(3) const void *

[Andreas Schwab]

On Jan 26 2024, Alejandro Colomar wrote:

> It is sometimes (often?) useful to allocate some object, write
> immediately to it, and then use it read-only.  The standard definition
> of free() forces the programmer to keep a non-const pointer around just
> for the sake of freeing, which is unnecessarily dangerous.

It is even more dangerous to pretend that the contents are unmodified,
when in fact they are no longer accessible (the memory can be reused any
time in any way).

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: free(3) const void *

[Alejandro Colomar]

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

Hi Andreas,

On Fri, Jan 26, 2024 at 04:13:55PM +0100, Andreas Schwab wrote:
> On Jan 26 2024, Alejandro Colomar wrote:
> 
> > It is sometimes (often?) useful to allocate some object, write
> > immediately to it, and then use it read-only.  The standard definition
> > of free() forces the programmer to keep a non-const pointer around just
> > for the sake of freeing, which is unnecessarily dangerous.
> 
> It is even more dangerous to pretend that the contents are unmodified,
> when in fact they are no longer accessible (the memory can be reused any
> time in any way).

But that's why [[gnu::malloc(deallocator)]] exists, to helps determine
the lifetime of an object (although it's imperfect, since it doesn't
take into account reallocations).

Have a lovely day,
Alex

-- 
<https://www.alejandro-colomar.es/>
Looking for a remote C programming job at the moment.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: free(3) const void *

[Alejandro Colomar]

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

Hi Arsen,

On Fri, Jan 26, 2024 at 03:24:29PM +0100, Arsen Arsenović wrote:
> But, free() modifies the object passed to it (even if not its bit
> representation) by freeing it.  Freeing const-passed objects would also
> violate the constness promise, so I disagree that free should take const
> void*.

This is an interesting interpretation.  Is expiring the lifetime of an
object a modification of the object?  Possibly.

But, the standard says:

	If an attempt is made to modify an object defined with a
	const-qualified type through use of an lvalue with
	non-const-qualified type, the behavior is undefined.

Even if you consider expiring the lifetime of the object a modification
of the object, the part that says "through use of an lvalue with
non-const-qualified type" is not fulfilled, IMO.  That would reqire
dereferencing the pointer, to actually get the lvalue, which free(3)
never does.

Have a lovely day,
Alex

-- 
<https://www.alejandro-colomar.es/>
Looking for a remote C programming job at the moment.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: free(3) const void *

[Arsen Arsenović]

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


Alejandro Colomar <alx@kernel.org> writes:

> [[PGP Signed Part:No public key for 9E8C1AFBBEFFDB32 created at 2024-01-26T16:35:04+0100 using RSA]]
> Hi Arsen,
>
> On Fri, Jan 26, 2024 at 03:24:29PM +0100, Arsen Arsenović wrote:
>> But, free() modifies the object passed to it (even if not its bit
>> representation) by freeing it.  Freeing const-passed objects would also
>> violate the constness promise, so I disagree that free should take const
>> void*.
>
> This is an interesting interpretation.  Is expiring the lifetime of an
> object a modification of the object?  Possibly.
>
> But, the standard says:
>
> 	If an attempt is made to modify an object defined with a
> 	const-qualified type through use of an lvalue with
> 	non-const-qualified type, the behavior is undefined.
>
> Even if you consider expiring the lifetime of the object a modification
> of the object, the part that says "through use of an lvalue with
> non-const-qualified type" is not fulfilled, IMO.  That would reqire
> dereferencing the pointer, to actually get the lvalue, which free(3)
> never does.

What 'free' precisely does is outside the bounds of the standard,
though.  We can assume it is permitted to so since nothing says
otherwise.

But, besides that, what I mean by 'constness promise' is that an object
must be usable following a const usage of it as if that usage never
happened.  This would certainly not be true of 'free', whether it
dereferences or not.  I am not sure if this is a formalism of the
language definition, but it is something people (and AFAIK compilers)
rely on significantly.

Have a lovely night!
--
Arsen Arsenović

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 251 bytes --]

Re: free(3) const void *

[Andreas Schwab]

On Jan 26 2024, Alejandro Colomar wrote:

> which free(3) never does.

What a library function does is a black box.  It is not restricted by
the standard in any way how it fulfills its duty.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: free(3) const void *

[Xi Ruoyao]

On Fri, 2024-01-26 at 18:22 +0100, Arsen Arsenović wrote:
> 
> Alejandro Colomar <alx@kernel.org> writes:
> 
> > [[PGP Signed Part:No public key for 9E8C1AFBBEFFDB32 created at 2024-01-26T16:35:04+0100 using RSA]]
> > Hi Arsen,
> > 
> > On Fri, Jan 26, 2024 at 03:24:29PM +0100, Arsen Arsenović wrote:
> > > But, free() modifies the object passed to it (even if not its bit
> > > representation) by freeing it.  Freeing const-passed objects would also
> > > violate the constness promise, so I disagree that free should take const
> > > void*.
> > 
> > This is an interesting interpretation.  Is expiring the lifetime of an
> > object a modification of the object?  Possibly.
> > 
> > But, the standard says:
> > 
> > 	If an attempt is made to modify an object defined with a
> > 	const-qualified type through use of an lvalue with
> > 	non-const-qualified type, the behavior is undefined.
> > 
> > Even if you consider expiring the lifetime of the object a modification
> > of the object, the part that says "through use of an lvalue with
> > non-const-qualified type" is not fulfilled, IMO.  That would reqire
> > dereferencing the pointer, to actually get the lvalue, which free(3)
> > never does.
> 
> What 'free' precisely does is outside the bounds of the standard,
> though.  We can assume it is permitted to so since nothing says
> otherwise.
> 
> But, besides that, what I mean by 'constness promise' is that an object
> must be usable following a const usage of it as if that usage never
> happened.  This would certainly not be true of 'free', whether it
> dereferences or not.  I am not sure if this is a formalism of the
> language definition, but it is something people (and AFAIK compilers)
> rely on significantly.

In C we (not sure about the people, but at least the compiler) cannot
rely on it at all.  It's perfectly legal to write something like

void
stupid (const char *c)
{
  strcpy ((char *)c, "some bullshit");
}

int
main (void)
{
  char buf[100];
  stupid (buf);
  puts (buf);
}

Yes it's as stupid as the name of the function.  But it does *not*
invoke any undefined behavior, and so the compiler is not allowed to do
any optimization assuming "stupid" won't change the content in buf.

That's why GCC has invented __attribute__ ((access (read_only, ...))). 
The documentation of this attribute even says we cannot rely on the
const qualifier:

   The read_only access mode specifies that the pointer to which it
   applies is used to read the referenced object but not write to it.
   Unless the argument specifying the size of the access denoted by
   size-index is zero, the referenced object must be initialized. The
   mode implies a stronger guarantee than the const qualifier which,
   when cast away from a pointer, does not prevent the pointed-to object
   from being modified. Examples of the use of the read_only access mode
   is the argument to the puts function, or the second and third
   arguments to the memcpy function.

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

Re: free(3) const void *

[Russ Allbery]

Alejandro Colomar <alx@kernel.org> writes:

> Since a `const void *` will accept anything that a `void *` would accept
> (and more), how about changing the prototype of free() in glibc as an
> extension to the language?

> I'd like to refor free(3) to be:

> 	void free(const void *p);

Maybe this way of explaining the objection will help.  Right now, if you
pass a const pointer into a function, you have some assurance from that
prototype (assisted by compiler diagnostics) that this function will not
modify *or invalidate* that pointer and you can continue using that
pointer after that call.  In other words, while C does not have full
Rust-style lifetime tracking, the const marker on a function approximately
indicates that the caller is not passing ownership of the pointer to that
function and the function call will not affect the pointer.

With this prototype, I believe you will create a situation where someone
could write a function that takes a const pointer and then calls free() at
the end of that function call on this passed parameter, and there would be
no warning from the compiler either when compiling that function or when
compiling a function that calls it and then uses the pointer afterwards
(at least unless there is enough inlining that a compiler can put all the
pieces together).

Unless I'm missing something, the more specific annotation indicating that
free() is a deallocator doesn't help because it doesn't propagate up the
call stack.  It will catch use after free within the same function, but it
won't catch the case where someone passes a const pointer down a whole
chain of functions that take const pointers, and then the function at the
bottom of that call stack frees the pointer, unless someone carefully
annotates every function in that call stack with that annotation.  When
this is done in error, someone obviously wouldn't do that.

The unique functionality of const here is that it *does* propagate up the
call stack via compiler warnings, in that you can't pass a const pointer
to a function with a non-const prototype without a compiler warning or an
explicit cast.  That provides some assurance of those lifetime properties.

In that view of the meaning of const, free() is definitely not const,
since it invalidates the pointer you pass into it and thus by definition
takes ownership of the pointer.  I realize this is not the precise
standard definition of const, but it is definitely what a lot of
real-world C code uses const to mean, and right now it basically works.

-- 
Russ Allbery (eagle@eyrie.org)             <https://www.eyrie.org/~eagle/>

Re: free(3) const void *

[Alejandro Colomar]

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

Hi Xi,

On Sat, Jan 27, 2024 at 01:55:48AM +0800, Xi Ruoyao wrote:
> In C we (not sure about the people, but at least the compiler) cannot
> rely on it at all.  It's perfectly legal to write something like
> 
> void
> stupid (const char *c)
> {
>   strcpy ((char *)c, "some bullshit");
> }
> 
> int
> main (void)
> {
>   char buf[100];
>   stupid (buf);
>   puts (buf);
> }
> 
> Yes it's as stupid as the name of the function.  But it does *not*
> invoke any undefined behavior, and so the compiler is not allowed to do
> any optimization assuming "stupid" won't change the content in buf.
> 
> That's why GCC has invented __attribute__ ((access (read_only, ...))). 
> The documentation of this attribute even says we cannot rely on the
> const qualifier:
> 
>    The read_only access mode specifies that the pointer to which it
>    applies is used to read the referenced object but not write to it.
>    Unless the argument specifying the size of the access denoted by
>    size-index is zero, the referenced object must be initialized. The
>    mode implies a stronger guarantee than the const qualifier which,
>    when cast away from a pointer, does not prevent the pointed-to object
>    from being modified. Examples of the use of the read_only access mode
>    is the argument to the puts function, or the second and third
>    arguments to the memcpy function.

Thanks!

This reminds me of access(none), which is the strongest of the access
modes that [[gnu::access()]] has.  free(3) could perfectly use
[[gnu::access(none)]], since it neither needs to read nor write to the
pointee.  That is stronger than const, so slapping a const there should
be straight-forward.

Regarding Andreas's consideration that libc functions are a black box:
free(3) as a black box means that the lifetime of the object is
terminated at the function boundary, so for the abstract machine,
free(3) cannot possibly access the object (since there's no object
any more).  If it does, for implementation-defined reasons, that
shouldn't be a problem of the caller, and libc should discard const
internally.

Have a lovely night!  =)
Alex

-- 
<https://www.alejandro-colomar.es/>
Looking for a remote C programming job at the moment.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: free(3) const void *

[Alejandro Colomar]

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

Hi Russ,

On Fri, Jan 26, 2024 at 10:09:35AM -0800, Russ Allbery wrote:
> Maybe this way of explaining the objection will help.  Right now, if you
> pass a const pointer into a function, you have some assurance from that
> prototype (assisted by compiler diagnostics) that this function will not
> modify *or invalidate* that pointer and you can continue using that
> pointer after that call.  In other words, while C does not have full
> Rust-style lifetime tracking, the const marker on a function approximately
> indicates that the caller is not passing ownership of the pointer to that
> function and the function call will not affect the pointer.
> 
> With this prototype, I believe you will create a situation where someone
> could write a function that takes a const pointer and then calls free() at
> the end of that function call on this passed parameter, and there would be
> no warning from the compiler either when compiling that function or when
> compiling a function that calls it and then uses the pointer afterwards
> (at least unless there is enough inlining that a compiler can put all the
> pieces together).

Unless I'm missing something, you wouldn't get a warning either if you
use a pointer after free(3), if that free() is hidden in a function
call.

	extern void my_free(void *p);

	my_free(p);
	my_free(p);

`const` doesn't make a difference there, regarding diagnostics about
use-after-free.

> Unless I'm missing something, the more specific annotation indicating that
> free() is a deallocator doesn't help because it doesn't propagate up the
> call stack.  It will catch use after free within the same function, but it
> won't catch the case where someone passes a const pointer down a whole
> chain of functions that take const pointers, and then the function at the
> bottom of that call stack frees the pointer, unless someone carefully
> annotates every function in that call stack with that annotation.  When
> this is done in error, someone obviously wouldn't do that.

You must propagate the annotation yourself (I don't remember from the
top of my head if there's any way to remind you if you firgot to
propagate it.

	[[gnu::access(none)]] extern void my_free_const(const void *p);
	[[gnu::malloc(my_free_const)]] extern void *my_malloc(size_t bytes);

	p = my_malloc(1);
	my_free_const(p);
	my_free_const(p);  // Warning: use-after-free

So, after Xi's reminder, I'll reformulate my suggestion to

	[[gnu::access(none)]]
	void free(const void *p);

(or for compatibility reasons, do the const thing via a macro.)

Have a lovely night,
Alex

-- 
<https://www.alejandro-colomar.es/>
Looking for a remote C programming job at the moment.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: free(3) const void *

[Xi Ruoyao]

On Fri, 2024-01-26 at 19:23 +0100, Alejandro Colomar wrote:
> So, after Xi's reminder, I'll reformulate my suggestion to
> 
> 	[[gnu::access(none)]]
> 	void free(const void *p);

It may defeat glibc.malloc.perturb, which will fill the freed buffer
with some junk bytes to detect UAF more easily.  Without the access
attribute the compiler will (definitely) read these junk bytes from the
freed buffer and hopefully make the program fail in a catastrophic way,
but with the access attribute the compiler may reuse the previously read
value stored in some registers, causing the UAF slip away.

And glibc.malloc.perturb is an environment variable set at runtime, so
we cannot do it via some #if...

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

[PATCH] Use [[gnu::access(none)]] on free(3)

[Alejandro Colomar]

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

The lifetime of the object expires right at the boundary of the call to
free(3), and the function doesn't access (neither read nor write) it, as
far as the abstract machine is concerned.

Cc: Arsen Arsenović <arsen@gentoo.org>
Cc: Andreas Schwab <schwab@linux-m68k.org>
Cc: Xi Ruoyao <xry111@xry111.site>
Cc: Russ Allbery <eagle@eyrie.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
---
 stdlib/stdlib.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/stdlib/stdlib.h b/stdlib/stdlib.h
index 414c49d731..8dfd373bf5 100644
--- a/stdlib/stdlib.h
+++ b/stdlib/stdlib.h
@@ -684,7 +684,8 @@ extern void *realloc (void *__ptr, size_t __size)
      __THROW __attribute_warn_unused_result__ __attribute_alloc_size__ ((2));
 
 /* Free a block allocated by `malloc', `realloc' or `calloc'.  */
-extern void free (void *__ptr) __THROW;
+extern void free (void *__ptr)
+     __THROW __attr_access ((__none__, 1));
 
 #ifdef __USE_MISC
 /* Re-allocate the previously allocated block in PTR, making the new
-- 
2.43.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: free(3) const void *

[Russ Allbery]

Alejandro Colomar <alx@kernel.org> writes:

> Unless I'm missing something, you wouldn't get a warning either if you
> use a pointer after free(3), if that free() is hidden in a function
> call.

> 	extern void my_free(void *p);

> 	my_free(p);
> 	my_free(p);

> `const` doesn't make a difference there, regarding diagnostics about
> use-after-free.

Yes, const can't help you prevent problems where you don't use const.  :)

I'm not saying that using const catches every C programming bug.  But
using const currently does ensure that nothing in the call stack into
which you pass a const pointer frees that pointer, unless it uses an
explicit cast or otherwise breaks its interface contract.

(C in general, and by design, does not protect you from code that doesn't
follow its interface contract, instead erring on the side of providing
escape hatches to do odd things.  Nonetheless, interface contracts such as
const are still helpful in finding bugs within collaborating code, even if
they're not strictly enforced in every circumstance.)

> So, after Xi's reminder, I'll reformulate my suggestion to

> 	[[gnu::access(none)]]
> 	void free(const void *p);

> (or for compatibility reasons, do the const thing via a macro.)

Maybe I'm missing something, but I don't see how this addresses the
concerns that were raised.

-- 
Russ Allbery (eagle@eyrie.org)             <https://www.eyrie.org/~eagle/>

Re: free(3) const void *

[Alejandro Colomar]

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

On Sat, Jan 27, 2024 at 02:36:09AM +0800, Xi Ruoyao wrote:
> On Fri, 2024-01-26 at 19:23 +0100, Alejandro Colomar wrote:
> > So, after Xi's reminder, I'll reformulate my suggestion to
> > 
> > 	[[gnu::access(none)]]
> > 	void free(const void *p);
> 
> It may defeat glibc.malloc.perturb, which will fill the freed buffer
> with some junk bytes to detect UAF more easily.  Without the access
> attribute the compiler will (definitely) read these junk bytes from the
> freed buffer and hopefully make the program fail in a catastrophic way,
> but with the access attribute the compiler may reuse the previously read
> value stored in some registers, causing the UAF slip away.
> 
> And glibc.malloc.perturb is an environment variable set at runtime, so
> we cannot do it via some #if...

Hmmm, interesting.  Then please ignore the patch I sent a moment ago.
:)

Have a lovely night,
Alex

-- 
<https://www.alejandro-colomar.es/>
Looking for a remote C programming job at the moment.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] Use [[gnu::access(none)]] on free(3)

[Alejandro Colomar]

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

On Fri, Jan 26, 2024 at 07:39:06PM +0100, Alejandro Colomar wrote:
> The lifetime of the object expires right at the boundary of the call to
> free(3), and the function doesn't access (neither read nor write) it, as
> far as the abstract machine is concerned.
> 
> Cc: Arsen Arsenović <arsen@gentoo.org>
> Cc: Andreas Schwab <schwab@linux-m68k.org>
> Cc: Xi Ruoyao <xry111@xry111.site>
> Cc: Russ Allbery <eagle@eyrie.org>
> Signed-off-by: Alejandro Colomar <alx@kernel.org>

Please do not apply.  Xi pointed out that GCC uses some magic that would
break with the attribute.

> ---
>  stdlib/stdlib.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/stdlib/stdlib.h b/stdlib/stdlib.h
> index 414c49d731..8dfd373bf5 100644
> --- a/stdlib/stdlib.h
> +++ b/stdlib/stdlib.h
> @@ -684,7 +684,8 @@ extern void *realloc (void *__ptr, size_t __size)
>       __THROW __attribute_warn_unused_result__ __attribute_alloc_size__ ((2));
>  
>  /* Free a block allocated by `malloc', `realloc' or `calloc'.  */
> -extern void free (void *__ptr) __THROW;
> +extern void free (void *__ptr)
> +     __THROW __attr_access ((__none__, 1));
>  
>  #ifdef __USE_MISC
>  /* Re-allocate the previously allocated block in PTR, making the new
> -- 
> 2.43.0
> 



-- 
<https://www.alejandro-colomar.es/>
Looking for a remote C programming job at the moment.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: free(3) const void *

[Alejandro Colomar]

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

On Fri, Jan 26, 2024 at 10:40:23AM -0800, Russ Allbery wrote:
> Yes, const can't help you prevent problems where you don't use const.  :)
> 
> I'm not saying that using const catches every C programming bug.  But
> using const currently does ensure that nothing in the call stack into
> which you pass a const pointer frees that pointer, unless it uses an
> explicit cast or otherwise breaks its interface contract.
> 
> (C in general, and by design, does not protect you from code that doesn't
> follow its interface contract, instead erring on the side of providing
> escape hatches to do odd things.  Nonetheless, interface contracts such as
> const are still helpful in finding bugs within collaborating code, even if
> they're not strictly enforced in every circumstance.)
> 
> > So, after Xi's reminder, I'll reformulate my suggestion to
> 
> > 	[[gnu::access(none)]]
> > 	void free(const void *p);
> 
> > (or for compatibility reasons, do the const thing via a macro.)
> 
> Maybe I'm missing something, but I don't see how this addresses the
> concerns that were raised.

I don't regard `const` as a contract that the pointer is not made
invalid (as you said, ISO C doesn't make such a claim).

I agree that these contracts are good, even if the programmer is allowed
to break them at will (usually, via a cast).  But to me, the only
contract that a pointer is made invalid or not, is the
[[gnu::malloc(f)]] atteribute.  If you use it consistently, you should
have few surprises.  Of course, it may need adaptation.

Have a lovely night,
Alex

-- 
<https://www.alejandro-colomar.es/>
Looking for a remote C programming job at the moment.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: free(3) const void *

[Xi Ruoyao]

On Fri, 2024-01-26 at 19:40 +0100, Alejandro Colomar wrote:
> On Sat, Jan 27, 2024 at 02:36:09AM +0800, Xi Ruoyao wrote:
> > On Fri, 2024-01-26 at 19:23 +0100, Alejandro Colomar wrote:
> > > So, after Xi's reminder, I'll reformulate my suggestion to
> > > 
> > > 	[[gnu::access(none)]]
> > > 	void free(const void *p);
> > 
> > It may defeat glibc.malloc.perturb, which will fill the freed buffer
> > with some junk bytes to detect UAF more easily.  Without the access
> > attribute the compiler will (definitely) read these junk bytes from the
> > freed buffer and hopefully make the program fail in a catastrophic way,
> > but with the access attribute the compiler may reuse the previously read
> > value stored in some registers, causing the UAF slip away.
> > 
> > And glibc.malloc.perturb is an environment variable set at runtime, so
> > we cannot do it via some #if...
> 
> Hmmm, interesting.  Then please ignore the patch I sent a moment ago.
> :)

Back on the topic, IMO you may tell others to create wrappers like

[[gnu::access(none, 1)]] static inline void
aggressive_free (const void *p)
{
  (free) ((void *)p);
}
#define free(x) aggressive_free (x)

with some texts to explain all the caveats (like `free ("string
literal");` will be incorrectly accepted, and the glibc.malloc.perturb
or similar facilities in other malloc implementations, etc) in the man
page...  But for the standard and/or the libc header it's a different
story.  Anyway if such a wrapper is adopted widely it will be easier to
raise it to WG14 and Austin.


-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

Re: free(3) const void *

[Alejandro Colomar]

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

Hi Xi,

On Sat, Jan 27, 2024 at 02:49:42AM +0800, Xi Ruoyao wrote:
> Back on the topic, IMO you may tell others to create wrappers like
> 
> [[gnu::access(none, 1)]] static inline void
> aggressive_free (const void *p)
> {
>   (free) ((void *)p);
> }
> #define free(x) aggressive_free (x)
> 
> with some texts to explain all the caveats (like `free ("string
> literal");` will be incorrectly accepted, and the glibc.malloc.perturb
> or similar facilities in other malloc implementations, etc) in the man
> page...  But for the standard and/or the libc header it's a different
> story.  Anyway if such a wrapper is adopted widely it will be easier to
> raise it to WG14 and Austin.

Sure!  I'll play with that in the following days, and will write
something in the manual page about it.  I raised the topic because for
fixing a const-correctness warning in shadow-utils, I had to make a
pointer non-const for its entire lifetime, and the only call that
required non-const was free(3), which was awkward.  So, I'll definitely
write a free(3) wrapper in shadow utils.  I also had similar problems
about const in Nginx last week.  Let's see if this catches up.  :)

I'll ask you about it if I have any doubts.

Have a lovely night,
Alex

-- 
<https://www.alejandro-colomar.es/>
Looking for a remote C programming job at the moment.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: free(3) const void *

[Florian Weimer]

* Russ Allbery:

> Alejandro Colomar <alx@kernel.org> writes:
>
>> Since a `const void *` will accept anything that a `void *` would accept
>> (and more), how about changing the prototype of free() in glibc as an
>> extension to the language?
>
>> I'd like to refor free(3) to be:
>
>> 	void free(const void *p);
>
> Maybe this way of explaining the objection will help.  Right now, if you
> pass a const pointer into a function, you have some assurance from that
> prototype (assisted by compiler diagnostics) that this function will not
> modify *or invalidate* that pointer and you can continue using that
> pointer after that call.  In other words, while C does not have full
> Rust-style lifetime tracking, the const marker on a function approximately
> indicates that the caller is not passing ownership of the pointer to that
> function and the function call will not affect the pointer.

To reinforce that line of argument: GCC can warn if you pass
uninitialized memory to a function using a const void * argument,
assuming that the only reason the you do that is for the called function
to read the uninitialized object, which is probably not what is
intended.

Thanks,
Florian

Re: free(3) const void *

[Florian Weimer]

* Alejandro Colomar:

> That would reqire dereferencing the pointer, to actually get the
> lvalue, which free(3) never does.

Just to clarify, our implementation in fact dereferences the pointer,
both to obtain heap metadata (outside the previously allocated object),
and to read from the object itself.  For the latter, see this commit and
subsequent ones:

commit bcdaad21d4635931d1bd3b54a7894276925d081d
Author: DJ Delorie <dj@delorie.com>
Date:   Tue Nov 20 13:24:09 2018 -0500

    malloc: tcache double free check
    
Thanks,
Florian

Re: free(3) const void *

[Arsen Arsenović]

Hi Alex,

Alejandro Colomar <alx@kernel.org> writes:

> [[PGP Signed Part:No public key for 9E8C1AFBBEFFDB32 created at 2024-01-26T19:11:27+0100 using RSA]]
> Hi Xi,
>
> On Sat, Jan 27, 2024 at 01:55:48AM +0800, Xi Ruoyao wrote:
>> In C we (not sure about the people, but at least the compiler) cannot
>> rely on it at all.  It's perfectly legal to write something like
>>
>> void
>> stupid (const char *c)
>> {
>>   strcpy ((char *)c, "some bullshit");
>> }
>>
>> int
>> main (void)
>> {
>>   char buf[100];
>>   stupid (buf);
>>   puts (buf);
>> }
>>
>> Yes it's as stupid as the name of the function.  But it does *not*
>> invoke any undefined behavior, and so the compiler is not allowed to do
>> any optimization assuming "stupid" won't change the content in buf.
>>
>> That's why GCC has invented __attribute__ ((access (read_only, ...))).
>> The documentation of this attribute even says we cannot rely on the
>> const qualifier:
>>
>>    The read_only access mode specifies that the pointer to which it
>>    applies is used to read the referenced object but not write to it.
>>    Unless the argument specifying the size of the access denoted by
>>    size-index is zero, the referenced object must be initialized. The
>>    mode implies a stronger guarantee than the const qualifier which,
>>    when cast away from a pointer, does not prevent the pointed-to object
>>    from being modified. Examples of the use of the read_only access mode
>>    is the argument to the puts function, or the second and third
>>    arguments to the memcpy function.
>
> Thanks!
>
> This reminds me of access(none), which is the strongest of the access
> modes that [[gnu::access()]] has.  free(3) could perfectly use
> [[gnu::access(none)]], since it neither needs to read nor write to the
> pointee.  That is stronger than const, so slapping a const there should
> be straight-forward.
>
> Regarding Andreas's consideration that libc functions are a black box:
> free(3) as a black box means that the lifetime of the object is
> terminated at the function boundary, so for the abstract machine,
> free(3) cannot possibly access the object (since there's no object
> any more).  If it does, for implementation-defined reasons, that
> shouldn't be a problem of the caller, and libc should discard const
> internally.

Note that the library (and implementation more broadly) are exempt from
various bits of the standard, as they are expected to be magic.
Notably, the 'lifetime ends at free boundary' rule doesn't mean that
free can't access the object in whichever way it pleases (and, in fact,
free already fails to comply with various other lifetime-related
definitions, to my knowledge).

> Have a lovely night!  =)
> Alex


--
Arsen Arsenović

Re: free(3) const void *

[Arsen Arsenović]

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


Xi Ruoyao <xry111@xry111.site> writes:

> On Fri, 2024-01-26 at 18:22 +0100, Arsen Arsenović wrote:
>>
>> Alejandro Colomar <alx@kernel.org> writes:
>>
>> > [[PGP Signed Part:No public key for 9E8C1AFBBEFFDB32 created at 2024-01-26T16:35:04+0100 using RSA]]
>> > Hi Arsen,
>> >
>> > On Fri, Jan 26, 2024 at 03:24:29PM +0100, Arsen Arsenović wrote:
>> > > But, free() modifies the object passed to it (even if not its bit
>> > > representation) by freeing it.  Freeing const-passed objects would also
>> > > violate the constness promise, so I disagree that free should take const
>> > > void*.
>> >
>> > This is an interesting interpretation.  Is expiring the lifetime of an
>> > object a modification of the object?  Possibly.
>> >
>> > But, the standard says:
>> >
>> > 	If an attempt is made to modify an object defined with a
>> > 	const-qualified type through use of an lvalue with
>> > 	non-const-qualified type, the behavior is undefined.
>> >
>> > Even if you consider expiring the lifetime of the object a modification
>> > of the object, the part that says "through use of an lvalue with
>> > non-const-qualified type" is not fulfilled, IMO.  That would reqire
>> > dereferencing the pointer, to actually get the lvalue, which free(3)
>> > never does.
>>
>> What 'free' precisely does is outside the bounds of the standard,
>> though.  We can assume it is permitted to so since nothing says
>> otherwise.
>>
>> But, besides that, what I mean by 'constness promise' is that an object
>> must be usable following a const usage of it as if that usage never
>> happened.  This would certainly not be true of 'free', whether it
>> dereferences or not.  I am not sure if this is a formalism of the
>> language definition, but it is something people (and AFAIK compilers)
>> rely on significantly.
>
> In C we (not sure about the people, but at least the compiler) cannot
> rely on it at all.  It's perfectly legal to write something like
>
> void
> stupid (const char *c)
> {
>   strcpy ((char *)c, "some bullshit");
> }
>
> int
> main (void)
> {
>   char buf[100];
>   stupid (buf);
>   puts (buf);
> }
>
> Yes it's as stupid as the name of the function.  But it does *not*
> invoke any undefined behavior, and so the compiler is not allowed to do
> any optimization assuming "stupid" won't change the content in buf.
>
> That's why GCC has invented __attribute__ ((access (read_only, ...))).
> The documentation of this attribute even says we cannot rely on the
> const qualifier:
>
>    The read_only access mode specifies that the pointer to which it
>    applies is used to read the referenced object but not write to it.
>    Unless the argument specifying the size of the access denoted by
>    size-index is zero, the referenced object must be initialized. The
>    mode implies a stronger guarantee than the const qualifier which,
>    when cast away from a pointer, does not prevent the pointed-to object
>    from being modified. Examples of the use of the read_only access mode
>    is the argument to the puts function, or the second and third
>    arguments to the memcpy function.

Ah, another regrettable bit of C to remember.  Thanks for sharing - I'll
keep this in mind.

Have a lovely night!
--
Arsen Arsenović

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 251 bytes --]

Re: free(3) const void *

[DJ Delorie]

My only comment on this is thus:

malloc() returns "void *" and that's what free() should be free'ing, so
they should match.

Re: [PATCH] Use [[gnu::access(none)]] on free(3)

[Paul Eggert]

On 2024-01-26 10:41, Alejandro Colomar wrote:
> Please do not apply.  Xi pointed out that GCC uses some magic that would
> break with the attribute.

Plus, I'm not seeing the point of either this patch or the earlier one. 
'free' is built into the language, and GCC therefore can deduce anything 
about 'free' that an attribute would supply. (If glibc made promises 
about 'free' that extend the C standard, that might justify adding an 
attribute, but this does not appear to be the case here.)

If there's some problem with GCC not deducing enough about 'free' in the 
areas discussed on this thread, that can and should be fixed in GCC.

PS. I didn't see the following mentioned so I'll add it here. Putting 
'const void *' into the signature of 'free' would incorrectly reject the 
conforming C code like this:

   void (*f) (void *) = free;

Re: free(3) const void *

[Andreas Schwab]

On Jan 26 2024, DJ Delorie wrote:

> My only comment on this is thus:
>
> malloc() returns "void *" and that's what free() should be free'ing, so
> they should match.

Also, objects in the heap are never read-only.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: free(3) const void *

[DJ Delorie]

Andreas Schwab <schwab@linux-m68k.org> writes:
> Also, objects in the heap are never read-only.

I thought of that, but using "const" in a program can mean something
other than "the object is read-only".  It can also act as a promise to
the user and/or optimizer that an object is not written to, even if
doing such is possible.

So the original problem of "having to keep a non-const pointer around
just to free it" is valid.  My post was just to put me in the "too bad,
deal with it" category ;-)

Re: free(3) const void *

[Andreas Schwab]

On Jan 26 2024, DJ Delorie wrote:

> Andreas Schwab <schwab@linux-m68k.org> writes:
>> Also, objects in the heap are never read-only.
>
> I thought of that, but using "const" in a program can mean something
> other than "the object is read-only".

Right, that is the difference between const-qualified and read-only.

> It can also act as a promise to the user and/or optimizer that an
> object is not written to, even if doing such is possible.

Unfortunately, the C standard does not support that promise.  Only
static or automatic objects can be truely read-only.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: [PATCH] Use [[gnu::access(none)]] on free(3)

[Alejandro Colomar]

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

Hi Paul,

On Fri, Jan 26, 2024 at 01:23:58PM -0800, Paul Eggert wrote:
> PS. I didn't see the following mentioned so I'll add it here. Putting 'const
> void *' into the signature of 'free' would incorrectly reject the conforming
> C code like this:
> 
>   void (*f) (void *) = free;

I mentioned it.  And yeah, it's a big caveat.  :)

Have a lovely night!
Alex

-- 
<https://www.alejandro-colomar.es/>
Looking for a remote C programming job at the moment.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: free(3) const void *

[Alejandro Colomar]

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

Hi DJ, Russ,

On Fri, Jan 26, 2024 at 04:47:38PM -0500, DJ Delorie wrote:
> Andreas Schwab <schwab@linux-m68k.org> writes:
> > Also, objects in the heap are never read-only.
> 
> I thought of that, but using "const" in a program can mean something
> other than "the object is read-only".  It can also act as a promise to
> the user and/or optimizer that an object is not written to, even if
> doing such is possible.
> 
> So the original problem of "having to keep a non-const pointer around
> just to free it" is valid.  My post was just to put me in the "too bad,
> deal with it" category ;-)

After some fresh air, I've been thinking about what Russ said, and it
makes a lot of sense.  While the attribute [[gnu::malloc()]] can tell
where some object is definitely terminated, there's no way to tell via
attributes where an object is definitely not terminated; you can only
guess that an object may or may not be terminated when a function
doesn't use the attribute.  So having `const` imply that an object is
not terminated is a feature.

If that means that we need to live with the fact that we need a writable
pointer to be able to free(3) an object, I guess the answer is indeed
"too bad, deal with it".  :-)

Maybe some day, attributes will give us full certainty about object
lifetime, but that day is not today.

Have a lovely night!
Alex 

-- 
<https://www.alejandro-colomar.es/>
Looking for a remote C programming job at the moment.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] Use [[gnu::access(none)]] on free(3)

[Cristian Rodríguez]

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

On Fri, Jan 26, 2024 at 6:24 PM Paul Eggert <eggert@cs.ucla.edu> wrote:

> On 2024-01-26 10:41, Alejandro Colomar wrote:
> > Please do not apply.  Xi pointed out that GCC uses some magic that would
> > break with the attribute.
>
> Plus, I'm not seeing the point of either this patch or the earlier one.
> 'free' is built into the language, and GCC therefore can deduce anything
> about 'free' that an attribute would supply. (If glibc made promises
> about 'free' that extend the C standard, that might justify adding an
> attribute, but this does not appear to be the case here.)
>
>
As there is __builtin_malloc I guess there could be a __builtin_free which
knows all these details..how useful would that be is a different story.

Re: [PATCH] Use [[gnu::access(none)]] on free(3)

[Gabriel Ravier]

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

On 1/27/24 13:21, Cristian Rodríguez wrote:
>
>
> On Fri, Jan 26, 2024 at 6:24 PM Paul Eggert <eggert@cs.ucla.edu> wrote:
>
>     On 2024-01-26 10:41, Alejandro Colomar wrote:
>     > Please do not apply.  Xi pointed out that GCC uses some magic
>     that would
>     > break with the attribute.
>
>     Plus, I'm not seeing the point of either this patch or the earlier
>     one.
>     'free' is built into the language, and GCC therefore can deduce
>     anything
>     about 'free' that an attribute would supply. (If glibc made promises
>     about 'free' that extend the C standard, that might justify adding an
>     attribute, but this does not appear to be the case here.)
>
>
> As there is __builtin_malloc I guess there could be a __builtin_free 
> which knows all these details..how useful would that be is a different 
> story.

 From what I can see, __builtin_free already exists.

Re: [PATCH] Use [[gnu::access(none)]] on free(3)

[Alejandro Colomar]

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

On Tue, Feb 13, 2024 at 03:19:57PM +0000, Gabriel Ravier wrote:
> On 1/27/24 13:21, Cristian Rodríguez wrote:
> > As there is __builtin_malloc I guess there could be a __builtin_free
> > which knows all these details..how useful would that be is a different
> > story.
> 
> From what I can see, __builtin_free already exists.

Christian probably meant __attribute__((free())).

Have a lovely day!
Alex

-- 
<https://www.alejandro-colomar.es/>
Looking for a remote C programming job at the moment.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]