Message ID | 20180621212835.5636-16-willy@infradead.org (mailing list archive) |
---|---|
State | Not Applicable |
Headers | show |
Series | None | expand |
On Thu, Jun 21, 2018 at 02:28:24PM -0700, Matthew Wilcox wrote:
> Removes a custom spinlock and simplifies the code.
I took a closer look at this patch as part of fixing the typo *ahem*.
The original code is buggy at the limit:
- if (winid > VAS_WINDOWS_PER_CHIP) {
- pr_err("Too many (%d) open windows\n", winid);
- vas_release_window_id(ida, winid);
That permits winid to be == VAS_WINDOWS_PER_CHIP, which is 64 << 10.
Since you then go on to store:
int id = window->winid;
vinst->windows[id] = window;
and windows is defined as:
struct vas_window *windows[VAS_WINDOWS_PER_CHIP];
that's a buffer overflow.
Here's the current version of my patch which will be in linux-next tomorrow.
diff --git a/arch/powerpc/platforms/powernv/vas-window.c b/arch/powerpc/platforms/powernv/vas-window.c
index ff9f48812331..e59e0e60e5b5 100644
--- a/arch/powerpc/platforms/powernv/vas-window.c
+++ b/arch/powerpc/platforms/powernv/vas-window.c
@@ -515,35 +515,17 @@ int init_winctx_regs(struct vas_window *window, struct vas_winctx *winctx)
return 0;
}
-static DEFINE_SPINLOCK(vas_ida_lock);
-
static void vas_release_window_id(struct ida *ida, int winid)
{
- spin_lock(&vas_ida_lock);
- ida_remove(ida, winid);
- spin_unlock(&vas_ida_lock);
+ ida_free(ida, winid);
}
static int vas_assign_window_id(struct ida *ida)
{
- int rc, winid;
-
- do {
- rc = ida_pre_get(ida, GFP_KERNEL);
- if (!rc)
- return -EAGAIN;
-
- spin_lock(&vas_ida_lock);
- rc = ida_get_new(ida, &winid);
- spin_unlock(&vas_ida_lock);
- } while (rc == -EAGAIN);
-
- if (rc)
- return rc;
+ int winid = ida_alloc_max(ida, VAS_WINDOWS_PER_CHIP - 1, GFP_KERNEL);
- if (winid > VAS_WINDOWS_PER_CHIP) {
- pr_err("Too many (%d) open windows\n", winid);
- vas_release_window_id(ida, winid);
+ if (winid == -ENOSPC) {
+ pr_err("Too many (%d) open windows\n", VAS_WINDOWS_PER_CHIP);
return -EAGAIN;
}
diff --git a/arch/powerpc/platforms/powernv/vas-window.c b/arch/powerpc/platforms/powernv/vas-window.c index ff9f48812331..2a5e68a2664d 100644 --- a/arch/powerpc/platforms/powernv/vas-window.c +++ b/arch/powerpc/platforms/powernv/vas-window.c @@ -515,35 +515,17 @@ int init_winctx_regs(struct vas_window *window, struct vas_winctx *winctx) return 0; } -static DEFINE_SPINLOCK(vas_ida_lock); - static void vas_release_window_id(struct ida *ida, int winid) { - spin_lock(&vas_ida_lock); - ida_remove(ida, winid); - spin_unlock(&vas_ida_lock); + ida_free(ida, winid); } static int vas_assign_window_id(struct ida *ida) { - int rc, winid; - - do { - rc = ida_pre_get(ida, GFP_KERNEL); - if (!rc) - return -EAGAIN; - - spin_lock(&vas_ida_lock); - rc = ida_get_new(ida, &winid); - spin_unlock(&vas_ida_lock); - } while (rc == -EAGAIN); - - if (rc) - return rc; + int winid = ida_alloc_max(ida, VAX_WINDOWS_PER_CHIP, GFP_KERNEL); - if (winid > VAS_WINDOWS_PER_CHIP) { - pr_err("Too many (%d) open windows\n", winid); - vas_release_window_id(ida, winid); + if (winid == -ENOSPC) { + pr_err("Too many (%d) open windows\n", VAX_WINDOWS_PER_CHIP); return -EAGAIN; }
Removes a custom spinlock and simplifies the code. Signed-off-by: Matthew Wilcox <willy@infradead.org> --- arch/powerpc/platforms/powernv/vas-window.c | 26 ++++----------------- 1 file changed, 4 insertions(+), 22 deletions(-)