From: Al Viro <viro@ZenIV.linux.org.uk>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: rth@twiddle.net, linux-kernel@vger.kernel.org
Subject: Re: alpha: potential race around hae_cache in RESTORE_ALL
Date: Sat, 25 Sep 2010 20:18:36 +0100 [thread overview]
Message-ID: <20100925191836.GW19804@ZenIV.linux.org.uk> (raw)
In-Reply-To: <AANLkTi=XURM0hOiitvNSiacd9+7Vx8s7V0KZ+oZQsDDQ@mail.gmail.com>
On Sat, Sep 25, 2010 at 11:42:41AM -0700, Linus Torvalds wrote:
> Ok, so it's been absolutely ages since I touched the HAE stuff, but I
> think the logic was that interrupts will always restore HAE to the
> pre-interrupt state on exit, so reading it was always supposedly
> race-free and didn't need any protection from normal code.
>
> But yes, any actual _changes_ to HAE had better protect against
> interrupts, so that they don't see the half-done state.
>
> So yes, I think you found a bug.
>
> I also don't see why that crazy RESTORE_ALL code does that reload of
> $0 and $1 when it updates HAE. Is that just legacy from it having
> _used_ to do the swipl PAL-call and that trashed those registers?
Looks like... swpipl takes argument in $16 and returns value in $0;
trashes $1, $16, $22--$25. Out of those, RESTORE_ALL deals with $22--$25
later in the sequence and $16 it handled by rti.
> Anyway, I think the fix should be that we really always do have
> interrupts disabled in RESTORE_ALL, rather than re-introduce the swipl
> into the RESTORE_ALL sequence. A lot of the critical sequences already
> do that - notably the normal return-to-user space code sequence that
> is the really critical one.
Agreed. There are only two places that need change (return to kernel path
in ret_from_syscall and kernel_thread essentially jumping to the same path),
so I'll just add
ret_to_kernel:
lda $16, 7
call_pal PAL_swpipl
br restore_all
and make these two places go to ret_to_kernel instead of restore_all.
BTW, another bug on alpha: coredump puts rdusp() result into regsets for
_all_ threads. Should be ti->pcb.usp for everything except current, a-la
KSTK_ESP logics...
I'll send fixes shortly
next prev parent reply other threads:[~2010-09-25 19:18 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-09-25 18:13 alpha: potential race around hae_cache in RESTORE_ALL Al Viro
2010-09-25 18:42 ` Linus Torvalds
2010-09-25 19:18 ` Al Viro [this message]
2010-09-25 19:25 ` Al Viro
[not found] ` <AANLkTikEVr6wA6D_f2Z6OEFu6SCP_-89u0-k-K-wKgb=@mail.gmail.com>
2010-09-25 21:33 ` Linus Torvalds
2010-09-27 7:58 ` Ivan Kokshaysky
2010-09-27 12:12 ` Al Viro
2010-09-27 12:46 ` Al Viro
2010-09-27 16:26 ` Ivan Kokshaysky
2010-09-27 17:10 ` Linus Torvalds
2010-09-27 18:05 ` Richard Henderson
2010-09-27 19:01 ` Al Viro
2010-09-27 21:21 ` Ivan Kokshaysky
2010-09-25 20:07 ` [PATCH] alpha: fix hae_cache race " Al Viro
2010-09-25 20:07 ` [PATCH] alpha: fix usp value in multithreaded coredumps Al Viro
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20100925191836.GW19804@ZenIV.linux.org.uk \
--to=viro@zeniv.linux.org.uk \
--cc=linux-kernel@vger.kernel.org \
--cc=rth@twiddle.net \
--cc=torvalds@linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.