linux-kernel - Re: [PATCH 02/13] jump label v9: base patch

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: <20100609223548.GB12752@nowhere>
Date:	Thu, 10 Jun 2010 00:35:49 +0200
From:	Frederic Weisbecker <fweisbec@...il.com>
To:	Jason Baron <jbaron@...hat.com>
Cc:	linux-kernel@...r.kernel.org, mingo@...e.hu,
	mathieu.desnoyers@...ymtl.ca, hpa@...or.com, tglx@...utronix.de,
	rostedt@...dmis.org, andi@...stfloor.org, roland@...hat.com,
	rth@...hat.com, mhiramat@...hat.com, avi@...hat.com,
	davem@...emloft.net, vgoyal@...hat.com, sam@...nborg.org
Subject: Re: [PATCH 02/13] jump label v9: base patch

On Wed, Jun 09, 2010 at 05:38:57PM -0400, Jason Baron wrote:
> +static int build_jump_label_hashtable(struct jump_entry *start, struct jump_entry *stop)
> +{
> +	struct jump_entry *iter, *iter_begin;
> +	struct jump_label_entry *entry;
> +	int count;
> +
> +	sort_jump_label_entries(start, stop);
> +	iter = start;
> +	while (iter < stop) {
> +		entry = get_jump_label_entry((char *)iter->name);
> +		if (!entry) {
> +			iter_begin = iter;
> +			count = 0;
> +			while ((iter < stop) &&
> +				(strcmp((char *)iter->name,
> +					(char *)iter_begin->name) == 0)) {
> +				iter++;
> +				count++;
> +			}




So, you can have multiple entries with the same name? How can that happen
in fact?




> +			entry = add_jump_label_entry((char *)iter_begin->name,
> +							count, iter_begin);
> +			if (IS_ERR(entry))
> +				return PTR_ERR(entry);
> +			continue;
> +		}
> +		WARN(1, KERN_ERR "build_jump_hashtable: unexpected entry!\n");



It seems you are going to endless loop in this fail case.



> +	}
> +	return 0;
> +}
> +
> +/***
> + * jump_label_update - update jump label text
> + * @name - name of the jump label
> + * @type - enum set to JUMP_LABEL_ENABLE or JUMP_LABEL_DISABLE
> + *
> + * Will enable/disable the jump for jump label @name, depending on the
> + * value of @type.
> + *
> + */
> +
> +void jump_label_update(const char *name, enum jump_label_type type)
> +{
> +	struct jump_entry *iter;
> +	struct jump_label_entry *entry;
> +	struct hlist_node *module_node;
> +	struct jump_label_module_entry *e_module;
> +	int count;
> +
> +	mutex_lock(&jump_label_mutex);
> +	entry = get_jump_label_entry(name);
> +	if (entry) {
> +		count = entry->nr_entries;
> +		iter = entry->table;
> +		while (count--) {
> +			if (kernel_text_address(iter->code))
> +				arch_jump_label_transform(iter, type);
> +			iter++;
> +		}



So, this is going to patch multiple times the same value on the
same address in case you have multiple entries for the same name?

That look weird.

BTW, if you can't find the entry, you should perhaps propagate an error.



> +	}
> +	mutex_unlock(&jump_label_mutex);
> +}
> +
> +static int init_jump_label(void)



This can be __init.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/