2011-06-14 23:50:05

by Josh Hunt

Subject: [PATCH] lkdtm: Fix race when crashpoint is hit multiple times before checking count

We observed the crash point count going negative in cases where the crash point
is hit multiple times before the check of "count == 0" is done. Because of this
we never call lkdtm_do_action(). This patch just adds a spinlock to protect
count.

Reported-by: Tapan Dhimant <[email protected]>
Signed-off-by: Josh Hunt <[email protected]>
---
drivers/misc/lkdtm.c | 8 ++++++++
1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/drivers/misc/lkdtm.c b/drivers/misc/lkdtm.c
index 81d7fa4..150cd70 100644
--- a/drivers/misc/lkdtm.c
+++ b/drivers/misc/lkdtm.c
@@ -120,6 +120,7 @@ static int recur_count = REC_NUM_DEFAULT;
static enum cname cpoint = CN_INVALID;
static enum ctype cptype = CT_NONE;
static int count = DEFAULT_COUNT;
+static DEFINE_SPINLOCK(count_lock);

module_param(recur_count, int, 0644);
MODULE_PARM_DESC(recur_count, " Recursion level for the stack overflow test, "\
@@ -230,11 +231,14 @@ static const char *cp_name_to_str(enum cname name)
static int lkdtm_parse_commandline(void)
{
int i;
+ unsigned long flags;

if (cpoint_count < 1 || recur_count < 1)
return -EINVAL;

+ spin_lock_irqsave(&count_lock, flags);
count = cpoint_count;
+ spin_unlock_irqrestore(&count_lock, flags);

/* No special parameters */
if (!cpoint_type && !cpoint_name)
@@ -349,6 +353,9 @@ static void lkdtm_do_action(enum ctype which)

static void lkdtm_handler(void)
{
+ unsigned long flags;
+
+ spin_lock_irqsave(&count_lock, flags);
count--;
printk(KERN_INFO "lkdtm: Crash point %s of type %s hit, trigger in %d rounds\n",
cp_name_to_str(cpoint), cp_type_to_str(cptype), count);
@@ -357,6 +364,7 @@ static void lkdtm_handler(void)
lkdtm_do_action(cptype);
count = cpoint_count;
}
+ spin_unlock_irqrestore(&count_lock, flags);
}

static int lkdtm_register_cpoint(enum cname which)
--
1.7.0.4

2011-06-15 12:58:37

by Ankita Garg

Subject: Re: [PATCH] lkdtm: Fix race when crashpoint is hit multiple times before checking count

Hi,

On Tue, Jun 14, 2011 at 04:40:27PM -0700, Josh Hunt wrote:
> We observed the crash point count going negative in cases where the crash point
> is hit multiple times before the check of "count == 0" is done. Because of this
> we never call lkdtm_do_action(). This patch just adds a spinlock to protect
> count.
>

Good catch! The fix Looks fine.

> Reported-by: Tapan Dhimant <[email protected]>
> Signed-off-by: Josh Hunt <[email protected]>

Acked-by: Ankita Garg <[email protected]>

> ---
> drivers/misc/lkdtm.c | 8 ++++++++
> 1 files changed, 8 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/misc/lkdtm.c b/drivers/misc/lkdtm.c
> index 81d7fa4..150cd70 100644
> --- a/drivers/misc/lkdtm.c
> +++ b/drivers/misc/lkdtm.c
> @@ -120,6 +120,7 @@ static int recur_count = REC_NUM_DEFAULT;
> static enum cname cpoint = CN_INVALID;
> static enum ctype cptype = CT_NONE;
> static int count = DEFAULT_COUNT;
> +static DEFINE_SPINLOCK(count_lock);
>
> module_param(recur_count, int, 0644);
> MODULE_PARM_DESC(recur_count, " Recursion level for the stack overflow test, "\
> @@ -230,11 +231,14 @@ static const char *cp_name_to_str(enum cname name)
> static int lkdtm_parse_commandline(void)
> {
> int i;
> + unsigned long flags;
>
> if (cpoint_count < 1 || recur_count < 1)
> return -EINVAL;
>
> + spin_lock_irqsave(&count_lock, flags);
> count = cpoint_count;
> + spin_unlock_irqrestore(&count_lock, flags);
>
> /* No special parameters */
> if (!cpoint_type && !cpoint_name)
> @@ -349,6 +353,9 @@ static void lkdtm_do_action(enum ctype which)
>
> static void lkdtm_handler(void)
> {
> + unsigned long flags;
> +
> + spin_lock_irqsave(&count_lock, flags);
> count--;
> printk(KERN_INFO "lkdtm: Crash point %s of type %s hit, trigger in %d rounds\n",
> cp_name_to_str(cpoint), cp_type_to_str(cptype), count);
> @@ -357,6 +364,7 @@ static void lkdtm_handler(void)
> lkdtm_do_action(cptype);
> count = cpoint_count;
> }
> + spin_unlock_irqrestore(&count_lock, flags);
> }
>
> static int lkdtm_register_cpoint(enum cname which)

--
Regards,
Ankita Garg ([email protected])
Linux Technology Center
IBM India Systems & Technology Labs,
Bangalore, India