On Tue, 7 May 2013, akpm@xxxxxxxxxxxxxxxxxxxx wrote:
> From: Jiri Kosina <jkosina@xxxxxxx>
> Subject: random: fix accounting race condition with lockless irq entropy_count update
>
> Commit 902c098a3663 ("random: use lockless techniques in the interrupt
> path") turned IRQ path from being spinlock protected into lockless
> cmpxchg-retry update.
>
> That commit removed r->lock serialization between crediting entropy bits
> from IRQ context and accounting when extracting entropy on userspace read
> path, but didn't turn the r->entropy_count reads/updates in account() to
> use cmpxchg as well.
>
> It has been observed, that under certain circumstances this leads to
> read() on /dev/urandom to return 0 (EOF), as r->entropy_count gets
> corrupted and becomes negative, which in turn results in propagating 0 all
> the way from account() to the actual read() call.
>
> Convert the accounting code to be the proper lockless counterpart of what
> has been partially done by 902c098a3663.
>
> Signed-off-by: Jiri Kosina <jkosina@xxxxxxx>
> Cc: Theodore Ts'o <tytso@xxxxxxx>
> Cc: Greg KH <greg@xxxxxxxxx>
> Cc: <stable@xxxxxxxxxxxxxxx>
> Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
I don't think this patch has been acted upon, has it?
Ted, please?
Thanks.
> ---
>
> drivers/char/random.c | 22 +++++++++++++++-------
> 1 file changed, 15 insertions(+), 7 deletions(-)
>
> diff -puN drivers/char/random.c~random-fix-accounting-race-condition-with-lockless-irq-entropy_count-update drivers/char/random.c
> --- a/drivers/char/random.c~random-fix-accounting-race-condition-with-lockless-irq-entropy_count-update
> +++ a/drivers/char/random.c
> @@ -865,16 +865,24 @@ static size_t account(struct entropy_sto
> if (r->entropy_count / 8 < min + reserved) {
> nbytes = 0;
> } else {
> + int entropy_count, orig;
> +retry:
> + entropy_count = orig = ACCESS_ONCE(r->entropy_count);
> /* If limited, never pull more than available */
> - if (r->limit && nbytes + reserved >= r->entropy_count / 8)
> - nbytes = r->entropy_count/8 - reserved;
> + if (r->limit && nbytes + reserved >= entropy_count / 8)
> + nbytes = entropy_count/8 - reserved;
>
> - if (r->entropy_count / 8 >= nbytes + reserved)
> - r->entropy_count -= nbytes*8;
> - else
> - r->entropy_count = reserved;
> + if (entropy_count / 8 >= nbytes + reserved) {
> + entropy_count -= nbytes*8;
> + if (cmpxchg(&r->entropy_count, orig, entropy_count) != orig)
> + goto retry;
> + } else {
> + entropy_count = reserved;
> + if (cmpxchg(&r->entropy_count, orig, entropy_count) != orig)
> + goto retry;
> + }
>
> - if (r->entropy_count < random_write_wakeup_thresh)
> + if (entropy_count < random_write_wakeup_thresh)
> wakeup_write = 1;
> }
>
> _
>
--
Jiri Kosina
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html