x86, PEBS/DS: fix code flow in ds_request()
Ingo Molnar [Tue, 18 Nov 2008 14:23:08 +0000 (15:23 +0100)]
this compiler warning:

  arch/x86/kernel/ds.c: In function 'ds_request':
  arch/x86/kernel/ds.c:368: warning: 'context' may be used uninitialized in this function

Shows that the code flow in ds_request() is buggy - it goes into
the unlock+release-context path even when the context is not allocated
yet.

First allocate the context, then do the other checks.

Also, take care with GFP allocations under the ds_lock spinlock.

Cc: <stable@kernel.org>
Signed-off-by: Ingo Molnar <mingo@elte.hu>

arch/x86/kernel/ds.c

index ac1d5b0..d1a1214 100644 (file)
@@ -236,17 +236,33 @@ static inline struct ds_context *ds_alloc_context(struct task_struct *task)
        struct ds_context *context = *p_context;
 
        if (!context) {
+               spin_unlock(&ds_lock);
+
                context = kzalloc(sizeof(*context), GFP_KERNEL);
 
-               if (!context)
+               if (!context) {
+                       spin_lock(&ds_lock);
                        return NULL;
+               }
 
                context->ds = kzalloc(ds_cfg.sizeof_ds, GFP_KERNEL);
                if (!context->ds) {
                        kfree(context);
+                       spin_lock(&ds_lock);
                        return NULL;
                }
 
+               spin_lock(&ds_lock);
+               /*
+                * Check for race - another CPU could have allocated
+                * it meanwhile:
+                */
+               if (*p_context) {
+                       kfree(context->ds);
+                       kfree(context);
+                       return *p_context;
+               }
+
                *p_context = context;
 
                context->this = p_context;
@@ -384,15 +400,15 @@ static int ds_request(struct task_struct *task, void *base, size_t size,
 
        spin_lock(&ds_lock);
 
-       error = -EPERM;
-       if (!check_tracer(task))
-               goto out_unlock;
-
        error = -ENOMEM;
        context = ds_alloc_context(task);
        if (!context)
                goto out_unlock;
 
+       error = -EPERM;
+       if (!check_tracer(task))
+               goto out_unlock;
+
        error = -EALREADY;
        if (context->owner[qual] == current)
                goto out_unlock;