Re: [PATCH v3] staging: r8188eu: Fix return type for implementation of ndo_start_xmit

[Dan Carpenter <error27@xxxxxxxxx>]

[I'm fixing my work email]

On Mon, Sep 5, 2022 at 4:03 PM GUO Zihua <guozihua@xxxxxxxxxx> wrote:
>
> CFI (Control Flow Integrity) is a safety feature allowing the system to
> detect and react should a potential control flow hijacking occurs. In
> particular, the Forward-Edge CFI protects indirect function calls by
> ensuring the prototype of function that is actually called matches the
> definition of the function hook.
>
> Since Linux now supports CFI, it will be a good idea to fix mismatched
> return type for implementation of hooks. Otherwise this would get
> cought out by CFI and cause a panic.

Thanks!  This patch needs a Fixes tag because it fixes a crash.

Fixes: cf68fffb66d6 ("add support for Clang CFI")

Ideally Clang would refuse to build known crashy code instead of us waiting
for users to debug things manually.  I have written a Smatch check which
detects this bug.

drivers/staging/r8188eu/os_dep/os_intfs.c:316 (null)() CLANG_CFI
insists types math exactly: 'rtw_xmit_entry'

I'll post the full results from my build tomorrow or later this week.

regards,
dan carpenter

/*
 * Copyright (C) 2022 Your Name.
 *
 * This program is free software; you can redistribute it and/or
 * modify it under the terms of the GNU General Public License
 * as published by the Free Software Foundation; either version 2
 * of the License, or (at your option) any later version.
 *
 * This program is distributed in the hope that it will be useful,
 * but WITHOUT ANY WARRANTY; without even the implied warranty of
 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 * GNU General Public License for more details.
 *
 * You should have received a copy of the GNU General Public License
 * along with this program; if not, see http://www.gnu.org/copyleft/gpl.txt
 */

#include "smatch.h"
#include "smatch_slist.h"

static int my_id;

static struct symbol *get_enum_type(struct expression *expr)
{
	struct symbol *type;

	type = get_type(expr);
	if (!type || type->type != SYM_PTR)
		return NULL;

	type = get_real_base_type(type);
	if (!type || type->type != SYM_FN)
		return NULL;

	type = get_real_base_type(type);
	if (!type || type->type != SYM_ENUM)
		return NULL;

	return type;
}

static bool enums_equiv(struct symbol *a, struct symbol *b)
{
	if (!a && !b)
		return true;
	if (!a || !b)
		return false;
	if (!a->ident && !b->ident)
		return types_equiv(a, b);
	if (!a->ident || !b->ident)
		return false;
	return strcmp(a->ident->name, b->ident->name) == 0;
}

static void match_assign(struct expression *expr)
{
	struct symbol *left, *right;
	char *name;

	left = get_enum_type(expr->left);
	right = get_enum_type(expr->right);
	if (enums_equiv(left, right))
		return;

	name = expr_to_str(expr->right);
	sm_msg("CLANG_CFI insists types math exactly: '%s'", name);
	free_string(name);
}

void check_enum_cfi(int id)
{
	my_id = id;

	add_hook(&match_assign, ASSIGNMENT_HOOK);
	add_hook(&match_assign, GLOBAL_ASSIGNMENT_HOOK);
}