linux-kernel - Re: [PATCH] aio: Add command to wait completion of all requests

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]

Date:   Tue, 13 Jun 2017 11:14:12 +0300
From:   Cyrill Gorcunov <gorcunov@...il.com>
To:     Kirill Tkhai <ktkhai@...tuozzo.com>
Cc:     avagin@...nvz.org, linux-kernel@...r.kernel.org, bcrl@...ck.org,
        viro@...iv.linux.org.uk, akpm@...ux-foundation.org,
        xemul@...tuozzo.com
Subject: Re: [PATCH] aio: Add command to wait completion of all requests

On Fri, Jun 09, 2017 at 12:49:34PM +0300, Kirill Tkhai wrote:
...
> +static int aio_wait_all(struct kioctx *ctx)
> +{
> +	unsigned users, reqs = 0;
> +	struct kioctx_cpu *kcpu;
> +	int cpu, ret;
> +
> +	if (atomic_xchg(&ctx->dead, 1))
> +		return -EBUSY;
> +
> +	users = atomic_read(&current->mm->mm_users);
> +	if (users > 1) {
> +		/*
> +		 * Wait till concurrent threads and aio_complete() see
> +		 * dead flag. Implies full memory barrier on all cpus.
> +		 */
> +		synchronize_sched();
> +	} else {
> +		/*
> +		 * Sync with aio_complete() to be sure it puts reqs_available,
> +		 * when dead flag is already seen.
> +		 */
> +		spin_lock_irq(&ctx->completion_lock);
> +	}
> +
> +	for_each_possible_cpu(cpu) {
> +		kcpu = per_cpu_ptr(ctx->cpu, cpu);
> +		reqs += kcpu->reqs_available;

I'm not that familiar with AIO internals but this snippet worries me:
the reqs_available is unsigned int, reqs is unsigned it as well but
used as an accumulator over ALL cpus, can't it get overflow and
gives modulo result, should not it be unsigned long or something?

> +		kcpu->reqs_available = 0;
> +	}
> +
> +	if (users == 1)
> +		spin_unlock_irq(&ctx->completion_lock);
> +
> +	atomic_add(reqs, &ctx->reqs_available);
> +
> +	ret = wait_event_interruptible(ctx->wait, reqs_completed(ctx));
> +
> +	atomic_set(&ctx->dead, 0);
> +
> +	return ret;
> +}