[RFC] Saner typechecking for closures

[RFC] Saner typechecking for closures

[Matthew Wilcox]

What would you think to this?

+++ b/include/linux/delayed_call.h
@@ -14,13 +14,12 @@ struct delayed_call {

 #define DEFINE_DELAYED_CALL(name) struct delayed_call name = {NULL, NULL}

-/* I really wish we had closures with sane typechecking... */
-static inline void set_delayed_call(struct delayed_call *call,
-               void (*fn)(void *), void *arg)
-{
-       call->fn = fn;
-       call->arg = arg;
-}
+/* Typecheck the arg is appropriate for the function */
+#define set_delayed_call(call, _fn, _arg) do {                         \
+       (void)sizeof(_fn(_arg));                                        \
+       (call)->fn = (void (*)(void *))(_fn);                           \
+       (call)->arg = (_arg);                                           \
+} while (0)

 static inline void do_delayed_call(struct delayed_call *call)
 {


That should give us the possibility of passing any pointer
to the function, but gets us away from void pointers.  I did this as a
followup:

-extern void page_put_link(void *);
+extern void page_put_link(struct folio *);

...

-void page_put_link(void *arg)
+void page_put_link(struct folio *folio)
 {
-       put_page(arg);
+       folio_put(folio);
 }
 EXPORT_SYMBOL(page_put_link);

and similar changes to the three callers.

Or is there something newer and shinier we should be using instead of
delayed_call?

Re: [RFC] Saner typechecking for closures

[Kees Cook]

On Tue, Apr 23, 2024 at 05:38:41AM +0100, Matthew Wilcox wrote:
> What would you think to this?
> 
> +++ b/include/linux/delayed_call.h
> @@ -14,13 +14,12 @@ struct delayed_call {
> 
>  #define DEFINE_DELAYED_CALL(name) struct delayed_call name = {NULL, NULL}
> 
> -/* I really wish we had closures with sane typechecking... */
> -static inline void set_delayed_call(struct delayed_call *call,
> -               void (*fn)(void *), void *arg)
> -{
> -       call->fn = fn;
> -       call->arg = arg;
> -}
> +/* Typecheck the arg is appropriate for the function */
> +#define set_delayed_call(call, _fn, _arg) do {                         \
> +       (void)sizeof(_fn(_arg));                                        \
> +       (call)->fn = (void (*)(void *))(_fn);                           \
> +       (call)->arg = (_arg);                                           \
> +} while (0)
> 
>  static inline void do_delayed_call(struct delayed_call *call)
>  {
> 
> 
> That should give us the possibility of passing any pointer
> to the function, but gets us away from void pointers.  I did this as a

This just means KCFI will freak out now, since it will see a mismatch
between call->fn's type and the target function's type. :(

And instead of args like this, can't we use the regular container_of()
tricks to get at the pointer we want? i.e. make "call" a member of the
strut doing the delayed call?

-Kees

> followup:
> 
> -extern void page_put_link(void *);
> +extern void page_put_link(struct folio *);
> 
> ...
> 
> -void page_put_link(void *arg)
> +void page_put_link(struct folio *folio)
>  {
> -       put_page(arg);
> +       folio_put(folio);
>  }
>  EXPORT_SYMBOL(page_put_link);
> 
> and similar changes to the three callers.
> 
> Or is there something newer and shinier we should be using instead of
> delayed_call?

-- 
Kees Cook

Re: [RFC] Saner typechecking for closures

[Al Viro]

On Wed, Apr 24, 2024 at 04:01:45PM -0700, Kees Cook wrote:

> > That should give us the possibility of passing any pointer
> > to the function, but gets us away from void pointers.  I did this as a
> 
> This just means KCFI will freak out now, since it will see a mismatch
> between call->fn's type and the target function's type. :(
> 
> And instead of args like this, can't we use the regular container_of()
> tricks to get at the pointer we want? i.e. make "call" a member of the
> strut doing the delayed call?

Huh?   A typical situation is (kfree, void *) or (put_page, struct page *).
We are most certainly *not* embedding anything of that sort into struct
page...

What we want is (T -> void) x T : whenever is_pointer(T), but C doesn't
give that kind of polymorphism.  We can do (void * -> void) x void *,
with callbacks converting their arguments to desired types, but that
means that type mismatches are on your head - compiler won't catch
them.

It could be done with lambdas as
	(void)sizeof(f(p)),	// p is a valid argument for f
	call->fn = [](void *v){f((typeof(p))v);},
	call->arg = p

but AFAICS we don't have that implemented sanely - neither in gcc nor in clang.
As the result, we have things like
        set_delayed_call(callback, page_put_link, page);
...
void page_put_link(void *arg)
{
        put_page(arg);
}
and there's nothing to catch you if 'page' above is not struct page *.

It shouldn't need any kind of runtime checks, executable stack, etc. -
everything here can be done statically at compile time.  Oh, well...