linux-hardening - Re: [RFC PATCH 10/11] linux/interrupt: Fix prototype matching property

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <202204191942.3C273AB@keescook>
Date:   Tue, 19 Apr 2022 19:45:16 -0700
From:   Kees Cook <keescook@...omium.org>
To:     joao@...rdrivepizza.com
Cc:     linux-kernel@...r.kernel.org, linux-hardening@...r.kernel.org,
        peterz@...radead.org, jpoimboe@...hat.com,
        andrew.cooper3@...rix.com, samitolvanen@...gle.com,
        mark.rutland@....com, hjl.tools@...il.com,
        alyssa.milburn@...ux.intel.com, ndesaulniers@...gle.com,
        gabriel.gomes@...ux.intel.com, rick.p.edgecombe@...el.com
Subject: Re: [RFC PATCH 10/11] linux/interrupt: Fix prototype matching
 property

On Tue, Apr 19, 2022 at 05:42:40PM -0700, joao@...rdrivepizza.com wrote:
> Unions will make function pointers with different prototypes be used through
> the same call. This leads into the same call instruction being used for
> calling functions with different prototypes, making them unsuitable for
> prototype-based fine-grained CFI.

Why? Shouldn't the callers be using different prototypes?

> Fix this CFI policy violation by removing the function pointer union in
> the tasklet struct.

The good news is that tasklet is on the way out the door[1], so this may
quickly become a non-issue, but also to that end, this fix is hardly a
problem for a deprecated API...

-Kees

[1] https://lore.kernel.org/linux-hardening/20220419211658.11403-1-apais@linux.microsoft.com/

> 
> Signed-off-by: Joao Moreira <joao@...rdrivepizza.com>
> ---
>  include/linux/interrupt.h | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
> index f40754caaefa..8d5504b0f20b 100644
> --- a/include/linux/interrupt.h
> +++ b/include/linux/interrupt.h
> @@ -650,10 +650,8 @@ struct tasklet_struct
>  	unsigned long state;
>  	atomic_t count;
>  	bool use_callback;
> -	union {
> -		void (*func)(unsigned long data);
> -		void (*callback)(struct tasklet_struct *t);
> -	};
> +	void (*func)(unsigned long data);
> +	void (*callback)(struct tasklet_struct *t);
>  	unsigned long data;
>  };
>  
> -- 
> 2.35.1
> 

-- 
Kees Cook