Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 19 Apr 2022 19:45:16 -0700
From:   Kees Cook <keescook@chromium.org>
To:     joao@overdrivepizza.com
Cc:     linux-kernel@vger.kernel.org, linux-hardening@vger.kernel.org,
        peterz@infradead.org, jpoimboe@redhat.com,
        andrew.cooper3@citrix.com, samitolvanen@google.com,
        mark.rutland@arm.com, hjl.tools@gmail.com,
        alyssa.milburn@linux.intel.com, ndesaulniers@google.com,
        gabriel.gomes@linux.intel.com, rick.p.edgecombe@intel.com
Subject: Re: [RFC PATCH 10/11] linux/interrupt: Fix prototype matching
 property
Message-ID: <202204191942.3C273AB@keescook>
References: <20220420004241.2093-1-joao@overdrivepizza.com>
 <20220420004241.2093-11-joao@overdrivepizza.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220420004241.2093-11-joao@overdrivepizza.com>
Precedence: bulk

On Tue, Apr 19, 2022 at 05:42:40PM -0700, joao@overdrivepizza.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@overdrivepizza.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