linux-kernel - Re: [PATCH v6 6/6] locking/lockdep: Reuse freed chain

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: <20200206160334.GV14914@hirez.programming.kicks-ass.net>
Date:   Thu, 6 Feb 2020 17:03:34 +0100
From:   Peter Zijlstra <peterz@...radead.org>
To:     Waiman Long <longman@...hat.com>
Cc:     Ingo Molnar <mingo@...hat.com>, Will Deacon <will.deacon@....com>,
        linux-kernel@...r.kernel.org, Bart Van Assche <bvanassche@....org>
Subject: Re: [PATCH v6 6/6] locking/lockdep: Reuse freed chain_hlocks entries

On Thu, Feb 06, 2020 at 10:24:08AM -0500, Waiman Long wrote:
> +#define for_each_chain_block(bucket, prev, curr)		\
> +	for ((prev) = -1, (curr) = chain_block_buckets[bucket];	\
> +	     (curr) >= 0;					\
> +	     (prev) = (curr), (curr) = chain_block_next(curr))

> +static inline void add_chain_block(int offset, int size)
> +{
> +	int bucket = size_to_bucket(size);
> +	int next = chain_block_buckets[bucket];
> +	int prev, curr;
> +
> +	if (unlikely(size < 2)) {
> +		/*
> +		 * We can't store single entries on the freelist. Leak them.
> +		 *
> +		 * One possible way out would be to uniquely mark them, other
> +		 * than with CHAIN_BLK_FLAG, such that we can recover them when
> +		 * the block before it is re-added.
> +		 */
> +		if (size)
> +			nr_lost_chain_hlocks++;
> +		return;
> +	}
> +
> +	nr_free_chain_hlocks += size;
> +	if (!bucket) {
> +		nr_large_chain_blocks++;
> +
> +		if (unlikely(next >= 0)) {

I was surprised by this condition..

> +			/*
> +			 * Variable sized, sort large to small.
> +			 */
> +			for_each_chain_block(0, prev, curr) {
> +				if (size >= chain_block_size(curr))
> +					break;
> +			}

Because if that is not so, then here:
	@curr == @next
	@prev == -1

> +			init_chain_block(offset, curr, 0, size);

and this will be identical to:

	init_chain_block(offset, next, bucket, size);


> +			if (prev < 0)
> +				chain_block_buckets[0] = offset;
> +			else
> +				init_chain_block(prev, offset, 0, 0);

and this will be:

	chain_block_buckets[bucket] = offset;


Or am I missing something?

> +			return;
> +		}
> +	}
> +	/*
> +	 * Fixed size or bucket[0] empty, add to head.
> +	 */
> +	init_chain_block(offset, next, bucket, size);
> +	chain_block_buckets[bucket] = offset;
> +}